Re: [RFC PATCH 09/10] POWERPC: smp: remove call toipi_call_lock()/ipi_call_unlock()

From: Yong Zhang
Date: Sun Jun 17 2012 - 22:52:51 EST


On Sat, Jun 16, 2012 at 09:32:19AM -0700, Paul E. McKenney wrote:
> On Tue, May 29, 2012 at 03:16:04PM +0800, Yong Zhang wrote:
> > From: Yong Zhang <yong.zhang@xxxxxxxxxxxxx>
> >
> > 1) call_function.lock used in smp_call_function_many() is just to protect
> > call_function.queue and &data->refs, cpu_online_mask is outside of the
> > lock. And it's not necessary to protect cpu_online_mask,
> > because data->cpumask is pre-calculate and even if a cpu is brougt up
> > when calling arch_send_call_function_ipi_mask(), it's harmless because
> > validation test in generic_smp_call_function_interrupt() will take care
> > of it.
> >
> > 2) For cpu down issue, stop_machine() will guarantee that no concurrent
> > smp_call_fuction() is processing.
>
> However, there is an effort to get rid of stop_machine() from the
> CPU-down path... So something else will be needed.

Yeah. So Thomas changed the commit log like below:
[
ipi_call_lock/unlock() lock resp. unlock call_function.lock. This lock
protects only the call_function data structure itself, but it's
completely unrelated to cpu_online_mask. The mask to which the IPIs
are sent is calculated before call_function.lock is taken in
smp_call_function_many(), so the locking around set_cpu_online() is
pointless and can be removed.

[ tglx: Massaged changelog ]
]

in tip/smp/hotplug.

Thanks,
Yong

>
> Thanx, Paul
>
> > Signed-off-by: Yong Zhang <yong.zhang0@xxxxxxxxx>
> > Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Paul Mackerras <paulus@xxxxxxxxx>
> > Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
> > ---
> > arch/powerpc/kernel/smp.c | 2 --
> > 1 files changed, 0 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index e4cb343..e1417c4 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -571,7 +571,6 @@ void __devinit start_secondary(void *unused)
> > if (system_state == SYSTEM_RUNNING)
> > vdso_data->processorCount++;
> > #endif
> > - ipi_call_lock();
> > notify_cpu_starting(cpu);
> > set_cpu_online(cpu, true);
> > /* Update sibling maps */
> > @@ -601,7 +600,6 @@ void __devinit start_secondary(void *unused)
> > of_node_put(np);
> > }
> > of_node_put(l2_cache);
> > - ipi_call_unlock();
> >
> > local_irq_enable();
> >
> > --
> > 1.7.5.4
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Only stand for myself
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/