Messages in this thread |  | | | Date | Mon, 23 Jun 2008 15:46:32 +0400 | | From | Oleg Nesterov <> | | Subject | Re: v2.6.26-rc7: BUG task_struct: Poison overwritten |
| |
On 06/22, Vegard Nossum wrote: > > 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"?
Yes, the comment is wrong, thanks.
It wasn't updated by
"do CPU_DEAD migrating under read_lock(tasklist) instead of write_lock_irq(tasklist)" http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f7b4cddcc5aca03e80e357360c9424dfba5056c2
Previously move_task_off_dead_cpu() was called under write_lock_irq(tasklist), and we can't take tasklist for writing without disabling irqs.
If you don't see other problems, could you make a patch to fix the comment?
Oleg.
|  |