You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/21 12:24:25 UTC
[GitHub] [incubator-nuttx] mkeydevelop opened a new pull request, #6660: sched/sched/sched_timerexpiration: add RR option
mkeydevelop opened a new pull request, #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660
Round robin timeslice will not run on idle task if
SCHED_RR_IDLETASK deselected
## Summary
Low Energy device always use SCHED_TICKLESS mode, staying in idle mode as long as possible.
All thread runing on default priority is very popular in real multi thread program. Round robin timeslice is useful in that scenario to make sure all thread have chance to run, especially small RR_INTERVAL.
On the other hand, small RR_INTERVAL will break the idle task frequently. We should not split the idle task timeslice.
## Impact
Idle task will run maximum tick if possible, and it just wakeup by task switch operation.
## Testing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926676885
##########
sched/Kconfig:
##########
@@ -420,6 +420,13 @@ config RR_INTERVAL
Round robin scheduling (SCHED_RR) is enabled by setting this
interval to a positive, non-zero value.
+config SCHED_RR_IDLETASK
Review Comment:
Keep idle task compatible? Maybe someone depend on this behaving?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r927279411
##########
sched/Kconfig:
##########
@@ -420,6 +420,13 @@ config RR_INTERVAL
Round robin scheduling (SCHED_RR) is enabled by setting this
interval to a positive, non-zero value.
+config SCHED_RR_IDLETASK
Review Comment:
But It's a wrong behavior, should be fixed. If we follow this direction, any fix need add an option to keep the old behavior, which just make the thing worse.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: idle task no need RR time slice
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r928378323
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -233,14 +231,23 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
/* Returning zero means that there is no interesting event to be timed */
-#ifdef KEEP_ALIVE_HACK
Review Comment:
But we should remove the hack branch instead? The hack basically disable tickless which isn't power friendly. Or we can keep it as before and fix later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926647739
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
+#ifdef CONFIG_SCHED_RR_IDLETASK
return KEEP_ALIVE_TICKS;
+#else
+
+ /* The idle task is the only task in that sched_priority == 0.
+ */
+
+ if (ntcb->sched_priority != 0)
Review Comment:
let's call sched_idletask instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926687471
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
+#ifdef CONFIG_SCHED_RR_IDLETASK
return KEEP_ALIVE_TICKS;
+#else
+
+ /* The idle task is the only task in that sched_priority == 0.
+ */
+
+ if (ntcb->sched_priority != 0)
+ {
+ return KEEP_ALIVE_TICKS;
+ }
+ else
+ {
+#ifdef CONFIG_SCHED_TICKLESS_LIMIT_MAX_SLEEP
+ return g_oneshot_maxticks - 1;
+#else
+ return UINT32_MAX - 1;
+#endif
Review Comment:
I found up_alarm_start will not call if we return 0 or UINT32_MAX directly. Someone maybe do some statistic job on this call?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop closed pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
mkeydevelop closed pull request #6660: sched/sched/sched_timerexpiration: add RR option
URL: https://github.com/apache/incubator-nuttx/pull/6660
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r927281872
##########
sched/Kconfig:
##########
@@ -420,6 +420,13 @@ config RR_INTERVAL
Round robin scheduling (SCHED_RR) is enabled by setting this
interval to a positive, non-zero value.
+config SCHED_RR_IDLETASK
Review Comment:
Here has some discussion about Kconfig:
https://www.mail-archive.com/dev@nuttx.apache.org/msg07946.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r927278258
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
Review Comment:
It's fine to fix the problem step by step, but I would like we don't add SCHED_RR_IDLETASK to keep the bug behavior. If you and all reviewer think this is problem, why still keep the old behavior? It's complicated the code and frustrate other people when he select the wrong option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: idle task no need RR time slice
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r928629295
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,18 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
- return KEEP_ALIVE_TICKS;
+ /* The idle task always lies at the end of the task list,
+ * idle task no need time slice.
+ */
+
+ if (ntcb->flink != NULL)
+ {
+ return KEEP_ALIVE_TICKS;
+ }
+ else
+ {
+ return UINT32_MAX - 1;
Review Comment:
I think we can remove else clause, the caller could handle zero return value as maximum timeout correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: idle task no need RR time slice
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r933992535
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,18 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
- return KEEP_ALIVE_TICKS;
+ /* The idle task always lies at the end of the task list,
+ * idle task no need time slice.
+ */
+
+ if (ntcb->flink != NULL)
+ {
+ return KEEP_ALIVE_TICKS;
+ }
+ else
+ {
+ return UINT32_MAX - 1;
Review Comment:
@mkeydevelop could you try this patch: https://github.com/apache/incubator-nuttx/pull/6746
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r927268137
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
Review Comment:
Maybe submit new patch to do this much more better? I prefer to do one thing in patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: idle task no need RR time slice
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r928378323
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -233,14 +231,23 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
/* Returning zero means that there is no interesting event to be timed */
-#ifdef KEEP_ALIVE_HACK
Review Comment:
But we should remove the hack branch instead? The hack basically disable tickless which isn't power friendly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r928378323
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -233,14 +231,23 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
/* Returning zero means that there is no interesting event to be timed */
-#ifdef KEEP_ALIVE_HACK
Review Comment:
But we should remove the hack branch instead?
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -233,14 +231,23 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
/* Returning zero means that there is no interesting event to be timed */
-#ifdef KEEP_ALIVE_HACK
if (ret == 0)
{
/* Apply the keep alive hack */
- return KEEP_ALIVE_TICKS;
+ /* The idle task always lies at the end of the task list,
+ * idle task no need time slice.
+ */
+
+ if (ntcb->flink != NULL)
Review Comment:
change to sched_idletask
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: idle task no need RR time slice
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r934138490
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,18 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
- return KEEP_ALIVE_TICKS;
+ /* The idle task always lies at the end of the task list,
+ * idle task no need time slice.
+ */
+
+ if (ntcb->flink != NULL)
+ {
+ return KEEP_ALIVE_TICKS;
+ }
+ else
+ {
+ return UINT32_MAX - 1;
Review Comment:
It work, but up_alarm_start/up_timer_start never call if only idle task is runnable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926693571
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
+#ifdef CONFIG_SCHED_RR_IDLETASK
return KEEP_ALIVE_TICKS;
+#else
+
+ /* The idle task is the only task in that sched_priority == 0.
+ */
+
+ if (ntcb->sched_priority != 0)
Review Comment:
Do same check similar to sched_idletask is good idea, I will fix it later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: idle task no need RR time slice
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r928605718
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -233,14 +231,23 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
/* Returning zero means that there is no interesting event to be timed */
-#ifdef KEEP_ALIVE_HACK
Review Comment:
I remove that patch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r928384088
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -233,14 +231,23 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
/* Returning zero means that there is no interesting event to be timed */
-#ifdef KEEP_ALIVE_HACK
if (ret == 0)
{
/* Apply the keep alive hack */
- return KEEP_ALIVE_TICKS;
+ /* The idle task always lies at the end of the task list,
+ * idle task no need time slice.
+ */
+
+ if (ntcb->flink != NULL)
Review Comment:
sched_idletask just check current cpu running task, nxsched_cpu_scheduler must workable if SMP on. I think @pkarashchenko means to use the similar logic to check idle task?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926615793
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
+#ifdef CONFIG_SCHED_RR_IDLETASK
return KEEP_ALIVE_TICKS;
+#else
+
+ /* The idle task is the only task in that sched_priority == 0.
+ */
+
+ if (ntcb->sched_priority != 0)
Review Comment:
```suggestion
if (ntcb->flink != NULL)
```
I think we should use similar approach to one that is used in `bool sched_idletask(void)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926654244
##########
sched/Kconfig:
##########
@@ -420,6 +420,13 @@ config RR_INTERVAL
Round robin scheduling (SCHED_RR) is enabled by setting this
interval to a positive, non-zero value.
+config SCHED_RR_IDLETASK
Review Comment:
why need this option? it's always good to disable time slice on idle thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r927278258
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
Review Comment:
It's fine to fix the problem step by step, but I would like we don't add SCHED_RR_IDLETASK to keep the buggy behavior. If you and all reviewer think this is problem, why still keep the old behavior? It's complicated the code and frustrate other people when he select the wrong option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r927278258
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
Review Comment:
It's fine to fix the problem step by step, but I would like we don't add SCHED_RR_IDLETASK to keep the bug behavior. If you and all reviewer think this is problem, why still keep the old behavior? It just complicat the code and frustrate other people when he select the wrong option.
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
Review Comment:
It's fine to fix the problem step by step, but I would like we don't add SCHED_RR_IDLETASK to keep the bug behavior. If you and all reviewer think this is problem, why still keep the old behavior? It just complicate the code and frustrate other people when he select the wrong option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mkeydevelop commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
mkeydevelop commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926687471
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
+#ifdef CONFIG_SCHED_RR_IDLETASK
return KEEP_ALIVE_TICKS;
+#else
+
+ /* The idle task is the only task in that sched_priority == 0.
+ */
+
+ if (ntcb->sched_priority != 0)
+ {
+ return KEEP_ALIVE_TICKS;
+ }
+ else
+ {
+#ifdef CONFIG_SCHED_TICKLESS_LIMIT_MAX_SLEEP
+ return g_oneshot_maxticks - 1;
+#else
+ return UINT32_MAX - 1;
+#endif
Review Comment:
I found up_alarm_start/up_timer_start will not call if we return 0 or UINT32_MAX directly. Someone maybe do some statistic job on this call?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: idle task no need RR time slice
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r928485754
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -233,14 +231,23 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
/* Returning zero means that there is no interesting event to be timed */
-#ifdef KEEP_ALIVE_HACK
if (ret == 0)
{
/* Apply the keep alive hack */
- return KEEP_ALIVE_TICKS;
+ /* The idle task always lies at the end of the task list,
+ * idle task no need time slice.
+ */
+
+ if (ntcb->flink != NULL)
Review Comment:
Yes, you are right.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r927281872
##########
sched/Kconfig:
##########
@@ -420,6 +420,13 @@ config RR_INTERVAL
Round robin scheduling (SCHED_RR) is enabled by setting this
interval to a positive, non-zero value.
+config SCHED_RR_IDLETASK
Review Comment:
Here has some discussion about Kconfig:
https://www.mail-archive.com/dev@nuttx.apache.org/msg07946.html
Too many unnecessary option will make the whole system unmaintainable and testable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926655249
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
/* Apply the keep alive hack */
+#ifdef CONFIG_SCHED_RR_IDLETASK
return KEEP_ALIVE_TICKS;
+#else
+
+ /* The idle task is the only task in that sched_priority == 0.
+ */
+
+ if (ntcb->sched_priority != 0)
+ {
+ return KEEP_ALIVE_TICKS;
+ }
+ else
+ {
+#ifdef CONFIG_SCHED_TICKLESS_LIMIT_MAX_SLEEP
+ return g_oneshot_maxticks - 1;
+#else
+ return UINT32_MAX - 1;
+#endif
Review Comment:
can return zero directly, since the caller will treat zero as uINT32_MAX.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6660: sched/sched/sched_timerexpiration: add RR option
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6660:
URL: https://github.com/apache/incubator-nuttx/pull/6660#discussion_r926656167
##########
sched/sched/sched_timerexpiration.c:
##########
@@ -238,7 +238,26 @@ static uint32_t nxsched_cpu_scheduler(int cpu, uint32_t ticks,
{
Review Comment:
why not remove all KEEP_ALIVE_HACK stuff, and return zero directly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org