Christopher M Riedl <c...@informatik.wtf> writes:
>> On July 30, 2019 at 4:31 PM Thiago Jung Bauermann <bauer...@linux.ibm.com>
>> wrote:
>>
>>
>>
>> Christopher M. Riedl <c...@informatik.wtf> writes:
>>
>> > Determining if a processor is in shared processor mode is not a constant
>> > so don't hide it behind a #define.
>> >
>> > Signed-off-by: Christopher M. Riedl <c...@informatik.wtf>
>> > ---
>> > arch/powerpc/include/asm/spinlock.h | 21 +++++++++++++++------
>> > 1 file changed, 15 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/spinlock.h
>> > b/arch/powerpc/include/asm/spinlock.h
>> > index a47f827bc5f1..8631b0b4e109 100644
>> > --- a/arch/powerpc/include/asm/spinlock.h
>> > +++ b/arch/powerpc/include/asm/spinlock.h
>> > @@ -101,15 +101,24 @@ static inline int arch_spin_trylock(arch_spinlock_t
>> > *lock)
>> >
>> > #if defined(CONFIG_PPC_SPLPAR)
>> > /* We only yield to the hypervisor if we are in shared processor mode */
>> > -#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))
>> > extern void __spin_yield(arch_spinlock_t *lock);
>> > extern void __rw_yield(arch_rwlock_t *lock);
>> > #else /* SPLPAR */
>> > #define __spin_yield(x) barrier()
>> > #define __rw_yield(x) barrier()
>> > -#define SHARED_PROCESSOR 0
>> > #endif
>> >
>> > +static inline bool is_shared_processor(void)
>> > +{
>> > +/* Only server processors have an lppaca struct */
>> > +#ifdef CONFIG_PPC_BOOK3S
>> > + return (IS_ENABLED(CONFIG_PPC_SPLPAR) &&
>> > + lppaca_shared_proc(local_paca->lppaca_ptr));
>> > +#else
>> > + return false;
>> > +#endif
>> > +}
>> > +
>>
>> CONFIG_PPC_SPLPAR depends on CONFIG_PPC_PSERIES, which depends on
>> CONFIG_PPC_BOOK3S so the #ifdef above is unnecessary:
>>
>> if CONFIG_PPC_BOOK3S is unset then CONFIG_PPC_SPLPAR will be unset as
>> well and the return expression should short-circuit to false.
>>
>
> Agreed, but the #ifdef is necessary to compile platforms which include
> this header but do not implement lppaca_shared_proc(...) and friends.
> I can reword the comment if that helps.
Ah, indeed. Yes, if you could mention that in the commit I think it
would help. These #ifdefs are becoming démodé so it's good to know why
they're there.
Another alternative is to provide a dummy lppaca_shared_proc() which
always returns false when CONFIG_PPC_BOOK3S isn't set (just mentioning
it, I don't have a preference).
--
Thiago Jung Bauermann
IBM Linux Technology Center
