[powerpc] Avoid debug_smp_processor_id() check in arch_spin_unlock_wait()

Use local_paca directly in arch_spin_unlock_wait(), as all processors have the
same value for the field shared_proc, so we don't need care racy here.

Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
 arch/powerpc/lib/locks.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Thu, 2013-01-10 at 17:00 +0800, Li Zhong wrote:
> Use local_paca directly in arch_spin_unlock_wait(), as all processors have the
> same value for the field shared_proc, so we don't need care racy here.

Of course that won't build if CONFIG_PPC_SPLPAR isn't defined...

Maybe you could change the definition of the SHARED_PROCESSOR
macro itself. The only possible "risk" would be a stale lppaca
if we preempt & hot unplug the CPU at the wrong time (provided
we no longer stop_machine either), I suppose if that's a real
concern we could delay freeing of lppaca's via RCU or such.

Ben.

> Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> ---
>  arch/powerpc/lib/locks.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
> index bb7cfec..850bea6 100644
> --- a/arch/powerpc/lib/locks.c
> +++ b/arch/powerpc/lib/locks.c
> @@ -72,7 +72,7 @@ void arch_spin_unlock_wait(arch_spinlock_t *lock)
>  {
>  	while (lock->slock) {
>  		HMT_low();
> -		if (SHARED_PROCESSOR)
> +		if (local_paca->lppaca_ptr->shared_proc)
>  			__spin_yield(lock);
>  	}
>  	HMT_medium();

On Thu, 2013-01-24 at 14:47 +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2013-01-10 at 17:00 +0800, Li Zhong wrote:
> > Use local_paca directly in arch_spin_unlock_wait(), as all processors have the
> > same value for the field shared_proc, so we don't need care racy here.
> 
> Of course that won't build if CONFIG_PPC_SPLPAR isn't defined...

...ah, I didn't notice that lppaca_ptr is only defined under
CONFIG_PPC_BOOK3S, and the whole paca is only defined under
CONFIG_PPC64; while the function changed seems could be used by any
configuration. Sorry about the carelessness. 

> 
> Maybe you could change the definition of the SHARED_PROCESSOR
> macro itself. The only possible "risk" would be a stale lppaca
> if we preempt & hot unplug the CPU at the wrong time (provided
> we no longer stop_machine either), I suppose if that's a real
> concern we could delay freeing of lppaca's via RCU or such.

I'm not very clear about the "risk" you mentioned above. It seems to me
that the freeing of lppaca only appeared in free_unused_pacas(), called
by early setup code, at which time hotplug seems impossible. 

I must missed something ...

I'll update the SHARED_PROCESSOR directly, it seems better to me now.
(wonder why I gave it up immediately when it first came into my mind
while I was doing the last update...)

Thanks, Zhong

> Ben.
> 
> > Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> > ---
> >  arch/powerpc/lib/locks.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
> > index bb7cfec..850bea6 100644
> > --- a/arch/powerpc/lib/locks.c
> > +++ b/arch/powerpc/lib/locks.c
> > @@ -72,7 +72,7 @@ void arch_spin_unlock_wait(arch_spinlock_t *lock)
> >  {
> >  	while (lock->slock) {
> >  		HMT_low();
> > -		if (SHARED_PROCESSOR)
> > +		if (local_paca->lppaca_ptr->shared_proc)
> >  			__spin_yield(lock);
> >  	}
> >  	HMT_medium();
> 
>

On Thu, 2013-01-24 at 18:13 +0800, Li Zhong wrote:
> I'm not very clear about the "risk" you mentioned above. It seems to me
> that the freeing of lppaca only appeared in free_unused_pacas(), called
> by early setup code, at which time hotplug seems impossible. 
> 
> I must missed something ...

No you are right, we don't free them at the moment.

> I'll update the SHARED_PROCESSOR directly, it seems better to me now.
> (wonder why I gave it up immediately when it first came into my mind
> while I was doing the last update...)

Cheers,
Ben.