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 2021/12/13 09:38:44 UTC

[GitHub] [incubator-nuttx] zhaoxiu-zeng opened a new pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

zhaoxiu-zeng opened a new pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989


   Don't reset the task's timeslice if no task switch.
   
   Signed-off-by: walker.zeng <zh...@gmail.com>
   


-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r773850276



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       Nice, let's watch the feedback.




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r770295053



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       @zhaoxiu-zeng what's about your thinking?




-- 
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] zhaoxiu-zeng commented on a change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
zhaoxiu-zeng commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r768463775



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       When a task is added to ready queue, the task is at the end of the same priority. When the task runs again, a long time has passed. So we re-assign the longest-timeslice to the task. @xiaoxiang781216




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r769614974



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       Yes, but the standard compliance is the highest priority of NuttX.
   https://github.com/apache/incubator-nuttx/blob/master/INVIOLABLES.md:
   ```
   Strict POSIX compliance
   Strict conformance to the portable standard OS interface as defined at OpenGroup.org.
   A deeply embedded system requires some special support. Special support must be minimized.
   The portable interface must never be compromised only for the sake of expediency.
   Expediency or even improved performance are not justifications for violation of the strict POSIX interface.
   ```




-- 
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] zhaoxiu-zeng commented on a change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
zhaoxiu-zeng commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r776593689



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       We can make such a modification first. The question of whether to reset the timeslice could be left for future.
   
   
   




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r768628877



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       But nxsched_process_roundrobin will reset time slice for this case:
   https://github.com/apache/incubator-nuttx/blob/master/sched/sched/sched_roundrobin.c#L102-L130




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r776598161



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       Ok, please update your PR, so I can merge it once it pass the CI.




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r776592136



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       Do you think we should remove the change in nxsched_add_readytorun?




-- 
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 merged pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989


   


-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r769215307



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       But from https://man7.org/linux/man-pages/man7/sched.7.html:
   A SCHED_RR thread that has been preempted by a higher priority thread and subsequently resumes execution as a running thread will complete the unexpired portion of its round-robin time quantum.
   At least, Linux kernel doesn't reset the time slice after suspend/resume cycle.




-- 
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] zhaoxiu-zeng commented on a change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
zhaoxiu-zeng commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r769473926



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       For the first point, after applying this commit, the behavior is consistent with linux kernel.
   For the second one, the scheduler works too if doesn't reset the task‘s timeslice, of course.
   But I think it would be better to reset the task's timeslice.
   




-- 
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] zhaoxiu-zeng commented on a change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
zhaoxiu-zeng commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r771281716



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       I don't think this commit undermines posix compliance. This is what most OS do, for example ThreadX. I will try to do a commit to linux kernel.
   




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r771573586



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       could you share the email thread after post the patch? So we can know the thinking from Linux community.




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r776592136



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       Do you think we should remove the change in nxsched_add_readytorun @zhaoxiu-zeng?




-- 
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] zhaoxiu-zeng commented on a change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
zhaoxiu-zeng commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r769183551



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       That's after the timeslice runs out, the task will be switch out immediately. Tasks that actively give up the cpu should be rewarded.




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r769614974



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       Yes, but the standard compliance is the highest priority of NuttX.




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r768402901



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       why need reset the time slice in nxsched_add_readytorun, I think it's enough to simply remove the reset in nxsched_resume_scheduler.




-- 
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] zhaoxiu-zeng commented on a change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
zhaoxiu-zeng commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r773733385



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       https://lkml.org/lkml/2021/12/22/188




-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r768402901



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       why need reset the time slice in nxsched_add_readytorun, I think it's enough to simply remove the reset in nxsched_resume_scheduler, @zhaoxiu-zeng.




-- 
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 pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#issuecomment-1002568372


   @zhaoxiu-zeng could you split the unrelated patch to the different PR? So people can get the summary info from title and review the change more smooth.


-- 
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 change in pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#discussion_r775313372



##########
File path: sched/sched/sched_addreadytorun.c
##########
@@ -158,6 +169,17 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
   int cpu;
   int me;
 
+#if CONFIG_RR_INTERVAL > 0
+#ifdef CONFIG_SCHED_SPORADIC
+  if ((btcb->flags & TCB_FLAG_POLICY_MASK) == TCB_FLAG_SCHED_RR)
+#endif
+    {
+      /* Reset the task's timeslice. */
+
+      btcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);

Review comment:
       @zhaoxiu-zeng look like Linux community also think the change doesn't conform to POSIX standard.




-- 
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] zhaoxiu-zeng commented on pull request #4989: sched: fix bug in the function nxsched_resume_scheduler

Posted by GitBox <gi...@apache.org>.
zhaoxiu-zeng commented on pull request #4989:
URL: https://github.com/apache/incubator-nuttx/pull/4989#issuecomment-992413920


   This commit is abandoned, I will do a new one soon.
   
   The BUG is as follow:
   
   1. Suppose there are two ready tasks t1 and t2 have the same priority, and t1 is the running.
   2. A new, higher priority task t0 (such as hpwork) is ready, then switch to t0.
   3. After t0 is suspended, then switch to t1 and reset the t1's timeslice.
   4. goto 2
   
   The t2 will have no chance to run.
   


-- 
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