2008-07-27 11:34:10

by Oleg Nesterov

Subject: [PATCH] wait_task_inactive: don't consider task->nivcsw

If wait_task_inactive() returns success the task was deactivated.
In that case schedule() always increments ->nvcsw which alone can
be used as a "generation counter".

If the next call returns the same number, we can be sure that the
task was unscheduled. Otherwise, because we know that .on_rq == 0
again, ->nvcsw should have been changed in between.

Q: perhaps it is better to do "ncsw = (p->nvcsw << 1) | 1" ? This
decreases the possibility of "was it unscheduled" false positive
when ->nvcsw == 0.

Signed-off-by: Oleg Nesterov <[email protected]>

--- LINUS/kernel/sched.c~ 2008-07-27 14:46:03.000000000 +0400
+++ LINUS/kernel/sched.c 2008-07-27 14:55:12.000000000 +0400
@@ -1922,11 +1922,8 @@ unsigned long wait_task_inactive(struct
running = task_running(rq, p);
on_rq = p->se.on_rq;
ncsw = 0;
- if (!match_state || p->state == match_state) {
- ncsw = p->nivcsw + p->nvcsw;
- if (unlikely(!ncsw))
- ncsw = 1;
- }
+ if (!match_state || p->state == match_state)
+ ncsw = p->nvcsw ?: 1;
task_rq_unlock(rq, &flags);

/*

2008-07-27 19:55:31

by Roland McGrath

Subject: Re: [PATCH] wait_task_inactive: don't consider task->nivcsw

> If wait_task_inactive() returns success the task was deactivated.
> In that case schedule() always increments ->nvcsw which alone can
> be used as a "generation counter".

Thanks, that looks fine to me.

> Q: perhaps it is better to do "ncsw = (p->nvcsw << 1) | 1" ? This
> decreases the possibility of "was it unscheduled" false positive
> when ->nvcsw == 0.

That makes sense.


Thanks,
Roland