Hello, There is a problem with this patch on Big Endian machines, see below. On Sat, Mar 19, 2022 at 11:54:53AM +0800, guoren@xxxxxxxxxx wrote: > From: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > > This is a simple, fair spinlock. Specifically it doesn't have all the > subtle memory model dependencies that qspinlock has, which makes it more > suitable for simple systems as it is more likely to be correct. > > [Palmer: commit text] > Signed-off-by: Palmer Dabbelt <palmer@xxxxxxxxxxxx> > > -- > > I have specifically not included Peter's SOB on this, as he sent his > original patch > <https://lore.kernel.org/lkml/YHbBBuVFNnI4kjj3@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/> > without one. > --- > include/asm-generic/spinlock.h | 11 +++- > include/asm-generic/spinlock_types.h | 15 +++++ > include/asm-generic/ticket-lock-types.h | 11 ++++ > include/asm-generic/ticket-lock.h | 86 +++++++++++++++++++++++++ > 4 files changed, 120 insertions(+), 3 deletions(-) > create mode 100644 include/asm-generic/spinlock_types.h > create mode 100644 include/asm-generic/ticket-lock-types.h > create mode 100644 include/asm-generic/ticket-lock.h > > diff --git a/include/asm-generic/ticket-lock.h b/include/asm-generic/ticket-lock.h > new file mode 100644 > index 000000000000..59373de3e32a > --- /dev/null > +++ b/include/asm-generic/ticket-lock.h ... > +static __always_inline void ticket_unlock(arch_spinlock_t *lock) > +{ > + u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN); As mentioned, this patch series breaks SMP on OpenRISC. I traced it to this line. The above `__is_defined(__BIG_ENDIAN)` does not return 1 as expected even on BIG_ENDIAN machines. This works: diff --git a/include/asm-generic/ticket-lock.h b/include/asm-generic/ticket-lock.h index 59373de3e32a..52b5dc9ffdba 100644 --- a/include/asm-generic/ticket-lock.h +++ b/include/asm-generic/ticket-lock.h @@ -26,6 +26,7 @@ #define __ASM_GENERIC_TICKET_LOCK_H #include <linux/atomic.h> +#include <linux/kconfig.h> #include <asm-generic/ticket-lock-types.h> static __always_inline void ticket_lock(arch_spinlock_t *lock) @@ -51,7 +52,7 @@ static __always_inline bool ticket_trylock(arch_spinlock_t *lock) static __always_inline void ticket_unlock(arch_spinlock_t *lock) { - u16 *ptr = (u16 *)lock + __is_defined(__BIG_ENDIAN); + u16 *ptr = (u16 *)lock + IS_ENABLED(CONFIG_CPU_BIG_ENDIAN); u32 val = atomic_read(lock); smp_store_release(ptr, (u16)val + 1); > + u32 val = atomic_read(lock); > + > + smp_store_release(ptr, (u16)val + 1); > +} > +
