Prev: first bad commit: 1f36f774 Switch !O_CREAT case to use of do_last()
Next: Loan Offer
From: Oleg Nesterov on 24 Mar 2010 12:10
On 03/24, Peter Zijlstra wrote:
>
> On Mon, 2010-03-15 at 10:10 +0100, Oleg Nesterov wrote:
> > static void move_task_off_dead_cpu(int dead_cpu, struct task_struct *p)
> > {
> > + struct rq *rq = cpu_rq(dead_cpu);
> > + int needs_cpu, dest_cpu;
> > + unsigned long flags;
> > again:
> > + local_irq_save(flags);
> > +
> > + raw_spin_lock(&rq->lock);
> > + needs_cpu = (task_cpu(p) == dead_cpu) && (p->state != TASK_WAKING);
>
> ^
> kernel/sched.c:5445: warning: 'dest_cpu' may be used uninitialized in this function

Hmm. looks like my gcc is more friendly...

OK. certainly I'll send the updated patch, if this series passes
your review otherwise.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Oleg Nesterov on 24 Mar 2010 12:40
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".

And. Without the change above, any owner of TASK_WAKING must disable
preemption and clear irqs.

What do you think?


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?

Of course, this doesn't really matter, but I'd like to understand
what I have missed here.

Thanks,

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: first bad commit: 1f36f774 Switch !O_CREAT case to use of do_last()
Next: Loan Offer