Messages in this thread | | | Date | Wed, 25 Jan 2012 18:43:30 +0100 | From | Oleg Nesterov <> | Subject | Re: [BUG] TASK_DEAD task is able to be woken up in special condition |
| |
On 01/25, Peter Zijlstra wrote: > > On Wed, 2012-01-25 at 16:45 +0100, Oleg Nesterov wrote: > > > > > > > > > > for (;;) { > > > > > tsk->state = TASK_DEAD; > > > > > schedule(); > > > > > } > > > > > > > > > > __schedule() can't race with ttwu() once it takes rq->lock. If the > > > > > exiting task is deactivated, finish_task_switch() will see EXIT_DEAD. > > > > > > > > TASK_DEAD, right? > > > > Yes, but... I simply can't understand what I was thinking about. > > And probably I missed something again, but I think this can't work. > > Oh man, total confusion. :-) Every time I look at this bug I see > different shadows on the wall.
Same here ;)
And this time I do not understand your reply.
> > Afaics, this can only help to prevent the race with ttwu_remote() > > doing ttwu_do_wakeup() under rq->lock. > > ttwu_do_wakeup() must always be called with rq->lock held.
Yes sure. I meant the code above can't race with p->on_rq == T case.
> > But we still can race with the !p->on_rq case which sets TASK_WAKING. > > It can do this after finish_task_switch() observes TASK_DEAD and does > > put_task_struct(). > > <random scribbling that got erased> > > No, see below !p->on_rq isn't possible and thus pi_lock is indeed > sufficient.
Which pi_lock? __schedule() doesn't take it. Hmm, see below...
> > > I think Yasunori-San's patch isn't > > > sufficient, note how the p->state = TASK_RUNNING in ttwu_do_wakeup() can > > > happen outside of p->pi_lock when the task gets queued on a remote cpu. > > > > Hmm, really? I am not sure, but I do not trust myself. > > > > To simplify, you mean that > > > > mb(); > > unlock_wait(pi_lock); > > > > tsk->state = TASK_DEAD; > > > > can change ->state from TASK_WAKING to TASK_DEAD, right? Is this really > > possible? ttwu() ensures p->on_rq == F in this case. > > Ahhh.. hold on, p->on_rq must be true, since we disabled preemption > before setting TASK_DEAD, so the thing cannot be scheduled out.
Why? __schedule() checks "preempt_count() & PREEMPT_ACTIVE". And it should be scheduled out, in general this task struct will be freed soon.
> Does this mean that both Yasunori-San's solution and yours work again?
I think that Yasunori-San's solution should work.
But,
> /me goes in search of a fresh mind.. shees!
Yes! I need the fresh head too. Probably just to realize I was completely wrong again.
Oleg.
| |