[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJF2gTTagygiQmNnxFG01ZbVKoHNc0rSbaPAjNFxxf7D7te3uQ@mail.gmail.com>
Date: Thu, 23 Jun 2022 17:17:41 +0800
From: Guo Ren <guoren@...nel.org>
To: Arnd Bergmann <arnd@...db.de>
Cc: Palmer Dabbelt <palmer@...osinc.com>,
Peter Zijlstra <peterz@...radead.org>,
Waiman Long <longman@...hat.com>,
Boqun Feng <boqun.feng@...il.com>,
Conor Dooley <Conor.Dooley@...rochip.com>,
Huacai Chen <chenhuacai@...ngson.cn>,
Xuerui Wang <kernel@...0n.name>, Rui Wang <r@....cc>,
Stafford Horne <shorne@...il.com>,
linux-riscv <linux-riscv@...ts.infradead.org>,
linux-arch <linux-arch@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Guo Ren <guoren@...ux.alibaba.com>
Subject: Re: [PATCH V6 1/2] asm-generic: spinlock: Move qspinlock &
ticket-lock into generic spinlock.h
On Thu, Jun 23, 2022 at 4:33 PM Arnd Bergmann <arnd@...db.de> wrote:
>
> On Tue, Jun 21, 2022 at 4:49 PM <guoren@...nel.org> wrote:
> >
> > From: Guo Ren <guoren@...ux.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@...ux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@...nel.org>
> > Cc: Peter Zijlstra (Intel) <peterz@...radead.org>
> > Cc: Arnd Bergmann <arnd@...db.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.
Okay. ticket_spinlock is also good to me.
>
> > +#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.
Okay
>
> > 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.
Okay, I'll try that.
>
> Arnd
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
Powered by blists - more mailing lists