[PATCH] sched: Fix the comment error in the wait_task_inactive interface.

hupu posted 1 patch 2 months, 1 week ago
kernel/sched/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] sched: Fix the comment error in the wait_task_inactive interface.
Posted by hupu 2 months, 1 week ago
The previous comment was incorrect because "task_on_cpu" only care about
p->on_cpu and does not care whether the runqueue has changed or not,
especially on SMP systems. In addition, task_on_cpu returns true
instead of false when p is running on a CPU.

Signed-off-by: hupu<hupu.gm@gmail.com>
---
  kernel/sched/core.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f3951e4a55e5..05b231a18440 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2129,8 +2129,8 @@ unsigned long wait_task_inactive(struct task_struct *p, unsigned int match_state
  		 * NOTE! Since we don't hold any locks, it's not
  		 * even sure that "rq" stays as the right runqueue!
  		 * But we don't care, since "task_on_cpu()" will
-		 * return false if the runqueue has changed and p
-		 * is actually now running somewhere else!
+		 * return true as long as p is running on a CPU,
+		 * regardless of any changes to the runqueue.
  		 */
  		while (task_on_cpu(rq, p)) {
  			if (!task_state_match(p, match_state))
-- 2.17.1
Re: [RESEND PATCH] sched: Fix the comment error in the wait_task_inactive interface.
Posted by hupu 2 months ago
 > The previous comment was incorrect because "task_on_cpu" only care about
 > p->on_cpu and does not care whether the runqueue has changed or not,
 > especially on SMP systems. In addition, task_on_cpu returns true
 > instead of false when p is running on a CPU.
 >
 > Signed-off-by: hupu<hupu.gm@gmail.com>
 > ---
 >   kernel/sched/core.c | 4 ++--
 >   1 file changed, 2 insertions(+), 2 deletions(-)
 >
 > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
 > index f3951e4a55e5..05b231a18440 100644
 > --- a/kernel/sched/core.c
 > +++ b/kernel/sched/core.c
 > @@ -2129,8 +2129,8 @@ unsigned long wait_task_inactive(struct 
task_struct *p, unsigned int match_state
 >            * NOTE! Since we don't hold any locks, it's not
 >            * even sure that "rq" stays as the right runqueue!
 >            * But we don't care, since "task_on_cpu()" will
 > -         * return false if the runqueue has changed and p
 > -         * is actually now running somewhere else!
 > +         * return true as long as p is running on a CPU,
 > +         * regardless of any changes to the runqueue.
 >            */
 >           while (task_on_cpu(rq, p)) {
 >               if (!task_state_match(p, match_state))
 > -- 2.17.1
 >

Dear Maintainers:
It's been a few days since the patch submission. Is there any progress?

I found that the original comments might be incorrect. If a task is 
running on another CPU,
task_on_cpu() should return true, not false as mentioned in the 
comments, and this is
unrelated to any changes in the runqueue.

Maybe I'm misreading this, but can you help me point it out? I look 
forward to discussing
with you.

Best regards.