Re: [patch 1/2] stop_machine: enable __stop_machine() to be called from the cpu online path

[Suresh Siddha <suresh.b.siddha@intel.com>]

On Tue, 2011-06-07 at 11:02 -0700, Ingo Molnar wrote:
> * Suresh Siddha <suresh.b.siddha@intel.com> wrote:
> 
> > Currently stop machine infrastructure can be called only from a cpu 
> > that is online. But for !CONFIG_SMP, we do allow calling 
> > __stop_machine() before the cpu is online.
> > 
> > x86 for example requires stop machine infrastructure in the cpu 
> > online path and currently implements its own stop machine (using 
> > stop_one_cpu_nowait()) for MTRR initialization in the cpu online 
> > path.
> > 
> > Enhance the __stop_machine() so that it can be called before the 
> > cpu is onlined. This will pave the way for code consolidation and 
> > address potential deadlocks caused by multiple mechanisms of doing 
> > system wide rendezvous.
> > 
> > This will also address the behavioral differences of 
> > __stop_machine() between SMP and UP builds.
> > 
> > Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: stable@kernel.org    # v2.6.35+
> > ---
> >  kernel/stop_machine.c |   62 +++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 57 insertions(+), 5 deletions(-)
> 
> This patch is causing problems:
> 
>  [   19.835435] BUG: using smp_processor_id() in preemptible [00000000] code: perf/701
>  [   19.838718] caller is __stop_machine+0x3c/0xbe

This is kind of a false positive. We are just checking if the calling
cpu is online/offline (and there is no possible process migration
between an online and offline cpu).

So I can use either raw_smp_processor_id() or probably wrap it around
get/put_cpu()'s. I would prefer raw_smp_processor_id() instead of the
unnecessary get/put_cpu().

We already have preempt disable/enable down this code path and would
like to keep the preempt disable section to the minimum needed code,
instead of increasing it to address this false positive.

Thoughts?

thanks,
suresh

>  [   19.842079] Pid: 701, comm: perf Not tainted 3.0.0-rc2-tip+ #132432
>  [   19.845378] Call Trace:
>  [   19.847838]  [<c1112431>] ? debug_smp_processor_id+0xbd/0xd0
>  [   19.848712]  [<c1050be6>] ? __stop_machine+0x3c/0xbe
>  [   19.852046]  [<c1005b74>] ? text_poke+0xec/0xec
>  [   19.855379]  [<c1014447>] ? do_page_fault+0xb0/0x361
>  [   19.858711]  [<c1005f00>] ? text_poke_smp+0x48/0x4f
>  [   19.862044]  [<c1014447>] ? do_page_fault+0xb0/0x361
>  [   19.865378]  [<c100519c>] ? arch_jump_label_transform+0x50/0x64
>  [   19.868712]  [<c103d61a>] ? __jump_label_update+0x28/0x39
>  [   19.872044]  [<c103d647>] ? jump_label_update+0x1c/0x49
>  [   19.875377]  [<c103d892>] ? jump_label_inc+0x38/0x40
>  [   19.878710]  [<c105cfaf>] ? perf_swevent_init+0x118/0x129
>  [   19.882044]  [<c10625b6>] ? perf_init_event+0x47/0x7c
>  [   19.885376]  [<c10627f7>] ? perf_event_alloc+0x20c/0x416
>  [   19.888710]  [<c1062fb1>] ? sys_perf_event_open+0x313/0x634
>  [   19.892042]  [<c101469a>] ? do_page_fault+0x303/0x361
>  [   19.895379]  [<c1281e70>] ? sysenter_do_call+0x12/0x26
> 
> --
> 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/