Messages in this thread | | | From | Arnd Bergmann <> | Date | Thu, 23 Jun 2022 10:33:24 +0200 | Subject | Re: [PATCH V6 1/2] asm-generic: spinlock: Move qspinlock & ticket-lock into generic spinlock.h |
| |
On Tue, Jun 21, 2022 at 4:49 PM <guoren@kernel.org> wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > Separate ticket-lock into tspinlock.h and let generic spinlock support > qspinlock or ticket-lock selected by CONFIG_ARCH_USE_QUEUED_SPINLOCKS > config. > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@kernel.org> > Cc: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: Arnd Bergmann <arnd@arndb.de> > --- > include/asm-generic/spinlock.h | 90 ++------------------------ > include/asm-generic/spinlock_types.h | 14 ++-- > include/asm-generic/tspinlock.h | 92 +++++++++++++++++++++++++++ > include/asm-generic/tspinlock_types.h | 17 +++++
Unless someone has a very good argument for the "tspinlock" name, I would prefer naming the new file ticket_spinlock.h. While the 'qspinlock' name has an established meaning already, this is not the case for 'tspinlock', and the longer name would be less confusing in my opinion.
> +#ifdef CONFIG_ARCH_USE_QUEUED_SPINLOCKS > +#include <asm/qspinlock.h> > #include <asm/qrwlock.h> > +#else > +#include <asm-generic/tspinlock.h> > +#endif
As Huacai Chen suggested in the other thread, the asm/qrwlock.h include should be outside of the #ifdef here.
> diff --git a/include/asm-generic/spinlock_types.h b/include/asm-generic/spinlock_types.h > index 8962bb730945..9875c1d058b3 100644 > --- a/include/asm-generic/spinlock_types.h > +++ b/include/asm-generic/spinlock_types.h > @@ -3,15 +3,11 @@ > #ifndef __ASM_GENERIC_SPINLOCK_TYPES_H > #define __ASM_GENERIC_SPINLOCK_TYPES_H > > -#include <linux/types.h> > -typedef atomic_t arch_spinlock_t; > - > -/* > - * qrwlock_types depends on arch_spinlock_t, so we must typedef that before the > - * include. > - */ > +#ifdef CONFIG_ARCH_USE_QUEUED_SPINLOCKS > +#include <asm-generic/qspinlock_types.h> > #include <asm/qrwlock_types.h> > - > -#define __ARCH_SPIN_LOCK_UNLOCKED ATOMIC_INIT(0) > +#else > +#include <asm-generic/tspinlock_types.h> > +#endif
I don't think this file warrants the extra indirection, since both versions have only a few lines. Just put it all into one file, and change the files that include asm-generic/qspinlock_types.h to use asm-generic/spinlock_types.h instead.
Arnd
| |