Messages in this thread | | | Date | Mon, 27 Aug 2007 21:37:14 +0100 (BST) | From | Hugh Dickins <> | Subject | Re: [PATCH] fix bogus hotplug cpu warning |
| |
On Mon, 27 Aug 2007, Andrew Morton wrote: > On Mon, 27 Aug 2007 16:06:19 +0100 (BST) > Hugh Dickins <hugh@veritas.com> wrote: > > > Fix bogus DEBUG_PREEMPT warning on x86_64, when cpu brought online after > > bootup: current_is_keventd is right to note its use of smp_processor_id > > is preempt-safe, but should use raw_smp_processor_id to avoid the warning. > > > > Signed-off-by: Hugh Dickins <hugh@veritas.com> > > > > --- 2.6.23-rc3-git10/kernel/workqueue.c 2007-07-26 19:49:58.000000000 +0100 > > +++ linux/kernel/workqueue.c 2007-08-26 18:59:16.000000000 +0100 > > @@ -635,7 +635,7 @@ int keventd_up(void) > > int current_is_keventd(void) > > { > > struct cpu_workqueue_struct *cwq; > > - int cpu = smp_processor_id(); /* preempt-safe: keventd is per-cpu */ > > + int cpu = raw_smp_processor_id(); /* preempt-safe: keventd is per-cpu */ > > int ret = 0; > > > > BUG_ON(!keventd_wq); > > But lib/smp_processor_id.c:debug_smp_processor_id() does > > /* > * Kernel threads bound to a single CPU can safely use > * smp_processor_id(): > */ > this_mask = cpumask_of_cpu(this_cpu); > > if (cpus_equal(current->cpus_allowed, this_mask)) > goto out; > > So I assume that this warning was triggering because some non-keventd, > non-pinned task is calling current_is_keventd()?
That's right, the x86_64 (and the ia64) do_boot_cpu() functions (called when onlining a cpu) do a check on current_is_keventd(): commented thus * During cold boot process, keventd thread is not spun up yet. * When we do cpu hot-add, we create idle threads on the fly, we should * not acquire any attributes from the calling context. Hence the clean * way to create kernel_threads() is to do that from keventd(). * We do the current_is_keventd() due to the fact that ACPI notifier * was also queuing to keventd() and when the caller is already running * in context of keventd(), we would end up with locking up the keventd * thread. though I've not tried to think that through.
> So I agree with the patch, but not with its description.
I don't see which part of the description you disagree with, but please do improve it if you can. The actual trace (when powersaved was saving power by onlining a cpu I'd tried to exclude with maxcpus=1 ;) was this:
BUG: using smp_processor_id() in preemptible [00000001] code: powersaved/3368 caller is current_is_keventd+0x9/0x3d Call Trace: [<ffffffff8031e5f5>] debug_smp_processor_id+0xc1/0xd4 [<ffffffff802453f3>] current_is_keventd+0x9/0x3d [<ffffffff80219c9b>] do_boot_cpu+0x19e/0x3c9 [<ffffffff80219ad4>] do_fork_idle+0x0/0x29 [<ffffffff80219fb7>] __cpu_up+0xd4/0x107 [<ffffffff80252932>] _cpu_up+0xd9/0x17a [<ffffffff80252a09>] cpu_up+0x36/0x4d [<ffffffff8038f7ca>] store_online+0x52/0x81 [<ffffffff8038b575>] sysdev_store+0x3c/0x3e [<ffffffff802cfed4>] flush_write_buffer+0x63/0x81 [<ffffffff802cff5b>] sysfs_write_file+0x69/0x8f [<ffffffff8046e5d9>] _spin_unlock_irqrestore+0x16/0x30 [<ffffffff8028aea3>] vfs_write+0xc5/0x186 [<ffffffff8024c180>] up_write+0xd/0xf [<ffffffff8028b02f>] sys_write+0x51/0x7a [<ffffffff8020bc5e>] system_call+0x7e/0x83
Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |