Messages in this thread | | | Subject | Re: [PATCH 2/6] move_task_off_dead_cpu: take rq->lock around select_fallback_rq() | From | Peter Zijlstra <> | Date | Fri, 26 Mar 2010 10:06:20 +0100 |
| |
On Wed, 2010-03-24 at 17:33 +0100, Oleg Nesterov wrote: > On 03/24, Peter Zijlstra wrote: > > > > Yeah, you made a few good points in 0/6, am now staring at the code on > > how to close those holes, hope to post something sensible soon. > > Yes, great. > > Speaking of 0/6, I forgot to ask a couple more question... > > try_to_wake_up() does task_rq_lock() which checks TASK_WAKING. Perhaps > it shouldn't ? I mean, perhaps try_to_wake_up() can take rq->lock without > checking task->state. It can never race with the owner of TASK_WAKING, > before anything else we check "p->state & state".
You're right, but creating a special task_rq_lock() for ttwu() went a little far, but now that we can remove all that again, this too should be good again.
> And a stupid question. While doing these changes I was really, really > puzzled by task_rq_lock() which does > > local_irq_save(*flags); > rq = task_rq(p); > raw_spin_lock(&rq->lock); > > to the point, I even tried to read the comment which says: > > Note the ordering: we can safely lookup the task_rq without > explicitly disabling preemption. > > Could you please explain what does this mean? IOW, why can't we do > > rq = task_rq(p); > raw_spin_lock_irqsave(&rq->lock, flags); > > instead?
I'm not sure why that is the case, v2.6.14:kernel/sched.c already has that. Ingo can you remember any reason for this or should we change the code like Oleg suggests?
| |