Messages in this thread |  | | | Date | Sun, 22 Jun 2008 00:57:00 +0200 | | From | "Vegard Nossum" <> | | Subject | Re: v2.6.26-rc7: BUG task_struct: Poison overwritten | |
On Sat, Jun 21, 2008 at 11:21 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> But it looks like there might be some cpu hotplug race with group
> scheduling - heiko (s390) and avi (x86_64) reported some cpu hotplug
> crashes. We're still looking into those.
Thanks.
I was poking around in kernel/sched.c and noticed something odd: In
migrate_dead(), we have this code:
/*
* Drop lock around migration; if someone else moves it,
* that's OK. No task can be added to this CPU, so iteration is
* fine.
*/
spin_unlock_irq(&rq->lock);
move_task_off_dead_cpu(dead_cpu, p);
spin_lock_irq(&rq->lock);
which is fine in itself, I guess. But spin_unlock_irq() will enable
interrupts. And move_task_off_dead_cpu() has this comment:
/*
* Figure out where task on dead CPU should go, use force if necessary.
* NOTE: interrupts should be disabled by the caller
*/
static void move_task_off_dead_cpu(int dead_cpu, struct task_struct *p)
{
...but here, interrupts will not be disabled. On the other hand
__migrate_task_irq() (called by move_task_off_dead_cpu()) calls
local_irq_disable() itself... What do you think of this? Is the
comment wrong? Or is there a difference between "interrupts" and
"local_irq"?
Vegard
--
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
-- E. W. Dijkstra, EWD1036
|  |