Re: [PATCH] LoongArch: Add qspinlock support

[Guo Ren <guoren@kernel.org>]

On Tue, Jun 21, 2022 at 8:59 AM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi,
>
> On Tue, Jun 21, 2022 at 12:01 AM Guo Ren <guoren@kernel.org> wrote:
> >
> > Hi Huacai & Arnd,
> >
> > Please have a look at riscv qspinlock V5:
> > https://lore.kernel.org/linux-riscv/a498a2ff-2503-b25c-53c9-55f5f2480bf6@microchip.com/T/#t
> From my point of view, we can simply drop RISCV_USE_QUEUED_SPINLOCKS,
> unless ticket spinlock is better than qspinlock in the !NUMA case.
RISC-V ISA has a flexible LR/SC forward guarantee definition, which
means some processors still must stay in ticket-lock.

Loongarch should give out the info about how strong your LR/SC forward
guarantee is. If some versions of the processor do strong guarantee,
but some don't. Maybe you should consider the RISC-V style.

>
> Huacai
>
> >
> >
> >
> > On Mon, Jun 20, 2022 at 5:50 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Jun 20, 2022 at 12:11 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > On Sun, Jun 19, 2022 at 5:48 PM Guo Ren <guoren@kernel.org> wrote:
> > > > >
> > > > > On Sat, Jun 18, 2022 at 1:40 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > >
> > > > > > On Sat, Jun 18, 2022 at 1:19 AM Guo Ren <guoren@kernel.org> wrote:
> > > > > > >
> > > > > > > > static inline u32 arch_xchg32(u32 *ptr, u32 x) {...}
> > > > > > > > static inline u64 arch_xchg64(u64 *ptr, u64 x) {...}
> > > > > > > >
> > > > > > > > #ifdef CONFIG_64BIT
> > > > > > > > #define xchg(ptr, x) (sizeof(*ptr) == 8) ? \
> > > > > > > >             arch_xchg64((u64*)ptr, (uintptr_t)x)  \
> > > > > > > >             arch_xchg32((u32*)ptr, x)
> > > > > > > > #else
> > > > > > > > #define xchg(ptr, x) arch_xchg32((u32*)ptr, (uintptr_t)x)
> > > > > > > > #endif
> > > > > > >
> > > > > > > The above primitive implies only long & int type args are permitted, right?
> > > > > >
> > > > > > The idea is to allow any scalar or pointer type, but not structures or
> > > > > > unions. If we need to deal with those as well, the macro could be extended
> > > > > > accordingly, but I would prefer to limit it as much as possible.
> > > > > >
> > > > > > There is already cmpxchg64(), which is used for types that are fixed to
> > > > > > 64 bit integers even on 32-bit architectures, but it is rarely used except
> > > > > > to implement the atomic64_t helpers.
> > > > > A lot of 32bit arches couldn't provide cmpxchg64 (like arm's ldrexd/strexd).
> > > >
> > > > Most 32-bit architectures also lack SMP support, so they can fall back to
> > > > the generic version from include/asm-generic/cmpxchg-local.h
> > > >
> > > > > Another question: Do you know why arm32 didn't implement
> > > > > HAVE_CMPXCHG_DOUBLE with ldrexd/strexd?
> > > >
> > > > I think it's just fairly obscure, the slub code appears to be the only
> > > > code that would use it.
> > > >
> > > > > >
> > > > > > 80% of the uses of cmpxchg() and xchg() deal with word-sized
> > > > > > quantities like 'unsigned long', or 'void *', but the others are almost
> > > > > > all fixed 32-bit quantities. We could change those to use cmpxchg32()
> > > > > > directly and simplify the cmpxchg() function further to only deal
> > > > > > with word-sized arguments, but I would not do that in the first step.
> > > > > Don't forget cmpxchg_double for this cleanup, when do you want to
> > > > > restart the work?
> > > >
> > > > I have no specific plans at the moment. If you or someone else likes
> > > > to look into it, I can dig out my old patch though.
> > > >
> > > > The cmpxchg_double() call seems to already fit in, since it is an
> > > > inline function and does not expect arbitrary argument types.
> > > Thank all of you. :)
> > >
> > > As Rui and Xuerui said, ll and sc in LoongArch both have implicit full
> > > barriers, so there is no "relaxed" version.
> > >
> > > The __WEAK_LLSC_MB in __cmpxchg_small() have nothing to do with ll and
> > >  sc themselves, we need a barrier at the branch target just because
> > > Loongson-3A5000 has a hardware flaw (and will be fixed in
> > > Loongson-3A6000).
> > >
> > > qspinlock just needs xchg_small(), but cmpxchg_small() is also useful
> > > for percpu operations. So I plan to split this patch to two: the first
> > > add xchg_small() and cmpxchg_small(), the second enable qspinlock.
> > >
> > > Huacai
> > >
> > > >
> > > >        Arnd
> >
> >
> >
> > --
> > Best Regards
> >  Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
> >



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/