From: Guo Ren <guoren@kernel.org>
To: Leonardo Bras <leobras@redhat.com>
Cc: paul.walmsley@sifive.com, anup@brainfault.org,
peterz@infradead.org, mingo@redhat.com, will@kernel.org,
palmer@rivosinc.com, longman@redhat.com, boqun.feng@gmail.com,
tglx@linutronix.de, paulmck@kernel.org, rostedt@goodmis.org,
rdunlap@infradead.org, catalin.marinas@arm.com,
conor.dooley@microchip.com, xiaoguang.xing@sophgo.com,
bjorn@rivosinc.com, alexghiti@rivosinc.com,
keescook@chromium.org, greentime.hu@sifive.com,
ajones@ventanamicro.com, jszhang@kernel.org, wefu@redhat.com,
wuwei2016@iscas.ac.cn, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-doc@vger.kernel.org,
kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
linux-csky@vger.kernel.org, Guo Ren <guoren@linux.alibaba.com>
Subject: Re: [PATCH V11 06/17] riscv: qspinlock: Introduce combo spinlock
Date: Thu, 14 Sep 2023 12:49:45 +0800 [thread overview]
Message-ID: <CAJF2gTQ=K_2km_AUZj1Vg8q2XgWB49-+nxViAa1mSAfppp6dnA@mail.gmail.com> (raw)
In-Reply-To: <ZQIgTyJ-DETrK8k3@redhat.com>
On Thu, Sep 14, 2023 at 4:49 AM Leonardo Bras <leobras@redhat.com> wrote:
>
> On Wed, Sep 13, 2023 at 05:37:01PM -0300, Leonardo Bras wrote:
> > On Sun, Sep 10, 2023 at 07:06:23AM -0400, Guo Ren wrote:
> > > On Sun, Sep 10, 2023 at 04:29:00AM -0400, guoren@kernel.org wrote:
> > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > >
> > > > Combo spinlock could support queued and ticket in one Linux Image and
> > > > select them during boot time via errata mechanism. Here is the func
> > > > size (Bytes) comparison table below:
> > > >
> > > > TYPE : COMBO | TICKET | QUEUED
> > > > arch_spin_lock : 106 | 60 | 50
> > > > arch_spin_unlock : 54 | 36 | 26
> > > > arch_spin_trylock : 110 | 72 | 54
> > > > arch_spin_is_locked : 48 | 34 | 20
> > > > arch_spin_is_contended : 56 | 40 | 24
> > > > rch_spin_value_unlocked : 48 | 34 | 24
> > > >
> > > > One example of disassemble combo arch_spin_unlock:
> > > > 0xffffffff8000409c <+14>: nop # detour slot
> > > > 0xffffffff800040a0 <+18>: fence rw,w # queued spinlock start
> > > > 0xffffffff800040a4 <+22>: sb zero,0(a4) # queued spinlock end
> > > > 0xffffffff800040a8 <+26>: ld s0,8(sp)
> > > > 0xffffffff800040aa <+28>: addi sp,sp,16
> > > > 0xffffffff800040ac <+30>: ret
> > > > 0xffffffff800040ae <+32>: lw a5,0(a4) # ticket spinlock start
> > > > 0xffffffff800040b0 <+34>: sext.w a5,a5
> > > > 0xffffffff800040b2 <+36>: fence rw,w
> > > > 0xffffffff800040b6 <+40>: addiw a5,a5,1
> > > > 0xffffffff800040b8 <+42>: slli a5,a5,0x30
> > > > 0xffffffff800040ba <+44>: srli a5,a5,0x30
> > > > 0xffffffff800040bc <+46>: sh a5,0(a4) # ticket spinlock end
> > > > 0xffffffff800040c0 <+50>: ld s0,8(sp)
> > > > 0xffffffff800040c2 <+52>: addi sp,sp,16
> > > > 0xffffffff800040c4 <+54>: ret
> > > >
> > > > The qspinlock is smaller and faster than ticket-lock when all are in
> > > > fast-path, and combo spinlock could provide a compatible Linux Image
> > > > for different micro-arch design (weak/strict fwd guarantee LR/SC)
> > > > processors.
> > > >
> > > > Signed-off-by: Guo Ren <guoren@kernel.org>
> > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > ---
> > > > arch/riscv/Kconfig | 9 +++-
> > > > arch/riscv/include/asm/spinlock.h | 78 ++++++++++++++++++++++++++++++-
> > > > arch/riscv/kernel/setup.c | 14 ++++++
> > > > 3 files changed, 98 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index 7f39bfc75744..4bcff2860f48 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -473,7 +473,7 @@ config NODES_SHIFT
> > > >
> > > > choice
> > > > prompt "RISC-V spinlock type"
> > > > - default RISCV_TICKET_SPINLOCKS
> > > > + default RISCV_COMBO_SPINLOCKS
> > > >
> > > > config RISCV_TICKET_SPINLOCKS
> > > > bool "Using ticket spinlock"
> > > > @@ -485,6 +485,13 @@ config RISCV_QUEUED_SPINLOCKS
> > > > help
> > > > Make sure your micro arch LL/SC has a strong forward progress guarantee.
> > > > Otherwise, stay at ticket-lock.
> > > > +
> > > > +config RISCV_COMBO_SPINLOCKS
> > > > + bool "Using combo spinlock"
> > > > + depends on SMP && MMU
> > > > + select ARCH_USE_QUEUED_SPINLOCKS
> > > > + help
> > > > + Select queued spinlock or ticket-lock via errata.
> > > > endchoice
> > > >
> > > > config RISCV_ALTERNATIVE
> > > > diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h
> > > > index c644a92d4548..8ea0fee80652 100644
> > > > --- a/arch/riscv/include/asm/spinlock.h
> > > > +++ b/arch/riscv/include/asm/spinlock.h
> > > > @@ -7,11 +7,85 @@
> > > > #define _Q_PENDING_LOOPS (1 << 9)
> > > > #endif
> > > >
> > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > > +#include <asm-generic/ticket_spinlock.h>
> > > > +
> > > > +#undef arch_spin_is_locked
> > > > +#undef arch_spin_is_contended
> > > > +#undef arch_spin_value_unlocked
> > > > +#undef arch_spin_lock
> > > > +#undef arch_spin_trylock
> > > > +#undef arch_spin_unlock
> > > > +
> > > > +#include <asm-generic/qspinlock.h>
> > > > +#include <linux/jump_label.h>
> > > > +
> > > > +#undef arch_spin_is_locked
> > > > +#undef arch_spin_is_contended
> > > > +#undef arch_spin_value_unlocked
> > > > +#undef arch_spin_lock
> > > > +#undef arch_spin_trylock
> > > > +#undef arch_spin_unlock
> > > Sorry, I forgot __no_arch_spinlock_redefine advice here. I would add it in v12.
> > > https://lore.kernel.org/linux-riscv/4cc7113a-0e4e-763a-cba2-7963bcd26c7a@redhat.com/
> > >
> >
> > Please check a reply to a previous patch I sent earlier: I think these
> > #undef can be avoided.
> >
> > > > +
> > > > +DECLARE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > > +
> > > > +static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > + queued_spin_lock(lock);
> > > > + else
> > > > + ticket_spin_lock(lock);
> > > > +}
> > > > +
> > > > +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > + return queued_spin_trylock(lock);
> > > > + else
> > > > + return ticket_spin_trylock(lock);
> > > > +}
> > > > +
> > > > +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > + queued_spin_unlock(lock);
> > > > + else
> > > > + ticket_spin_unlock(lock);
> > > > +}
> > > > +
> > > > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > + return queued_spin_value_unlocked(lock);
> > > > + else
> > > > + return ticket_spin_value_unlocked(lock);
> > > > +}
> > > > +
> > > > +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > + return queued_spin_is_locked(lock);
> > > > + else
> > > > + return ticket_spin_is_locked(lock);
> > > > +}
> > > > +
> > > > +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > + return queued_spin_is_contended(lock);
> > > > + else
> > > > + return ticket_spin_is_contended(lock);
> > > > +}
> > > > +#else /* CONFIG_RISCV_COMBO_SPINLOCKS */
> > > > +
>
> Also, those functions all reproduce the same behavior, so maybe it would be
> better to keep that behavior in a macro such as:
>
> #define COMBO_SPINLOCK_DECLARE(f) \
> static __always_inline int arch_spin_ ## f(arch_spinlock_t *lock) \
> { \
> if (static_branch_likely(&combo_qspinlock_key)) \
> return queued_spin_ ## f(lock); \
> else \
> return ticket_spin_ ## f(lock); \
> }
>
> COMBO_SPINLOCK_DECLARE(lock)
> COMBO_SPINLOCK_DECLARE(trylock)
> COMBO_SPINLOCK_DECLARE(unlock)
> COMBO_SPINLOCK_DECLARE(value_unlocked)
> COMBO_SPINLOCK_DECLARE(is_locked)
> COMBO_SPINLOCK_DECLARE(is_contended)
>
> Does that make sense?
Yeah, thanks. I would try. You improved my macro skills. :)
>
> Thanks!
> Leo
>
>
> > > > #ifdef CONFIG_QUEUED_SPINLOCKS
> > > > #include <asm/qspinlock.h>
> > > > -#include <asm/qrwlock.h>
> > > > #else
> > > > -#include <asm-generic/spinlock.h>
> > > > +#include <asm-generic/ticket_spinlock.h>
> > > > #endif
> > > >
> > > > +#endif /* CONFIG_RISCV_COMBO_SPINLOCKS */
> > > > +
> > > > +#include <asm/qrwlock.h>
> > > > +
> > > > #endif /* __ASM_RISCV_SPINLOCK_H */
> > > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > > > index 32c2e1eb71bd..a447cf360a18 100644
> > > > --- a/arch/riscv/kernel/setup.c
> > > > +++ b/arch/riscv/kernel/setup.c
> > > > @@ -269,6 +269,18 @@ static void __init parse_dtb(void)
> > > > #endif
> > > > }
> > > >
> > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > > +DEFINE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > > +EXPORT_SYMBOL(combo_qspinlock_key);
> > > > +#endif
> > > > +
> > > > +static void __init riscv_spinlock_init(void)
> > > > +{
> > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > > + static_branch_disable(&combo_qspinlock_key);
> > > > +#endif
> > > > +}
> > > > +
> > > > extern void __init init_rt_signal_env(void);
> > > >
> > > > void __init setup_arch(char **cmdline_p)
> > > > @@ -317,6 +329,8 @@ void __init setup_arch(char **cmdline_p)
> > > > riscv_isa_extension_available(NULL, ZICBOM))
> > > > riscv_noncoherent_supported();
> > > > riscv_set_dma_cache_alignment();
> > > > +
> > > > + riscv_spinlock_init();
> > > > }
> > > >
> > > > static int __init topology_init(void)
> > > > --
> > > > 2.36.1
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > >
> > >
>
--
Best Regards
Guo Ren
next prev parent reply other threads:[~2023-09-14 4:50 UTC|newest]
Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-10 8:28 [PATCH V11 00/17] riscv: Add Native/Paravirt qspinlock support guoren
2023-09-10 8:28 ` [PATCH V11 01/17] asm-generic: ticket-lock: Reuse arch_spinlock_t of qspinlock guoren
2023-09-11 19:05 ` Leonardo Brás
2023-09-13 1:55 ` Guo Ren
2023-09-13 7:59 ` Leonardo Bras
2023-09-10 8:28 ` [PATCH V11 02/17] asm-generic: ticket-lock: Move into ticket_spinlock.h guoren
2023-09-13 8:15 ` Leonardo Bras
2023-09-10 8:28 ` [PATCH V11 03/17] riscv: Use Zicbop in arch_xchg when available guoren
2023-09-13 8:49 ` Leonardo Bras
2023-09-15 12:36 ` Guo Ren
2023-09-16 1:25 ` Leonardo Bras
2023-09-17 14:34 ` Guo Ren
2023-09-19 5:13 ` Leonardo Bras
2023-09-19 7:53 ` Guo Ren
2023-09-19 14:38 ` Leonardo Bras
2023-09-14 13:47 ` Andrew Jones
2023-09-15 8:22 ` Leonardo Bras
2023-09-15 11:07 ` Andrew Jones
2023-09-15 11:26 ` Conor Dooley
2023-09-15 12:22 ` Andrew Jones
2023-09-15 12:42 ` Conor Dooley
2023-09-16 0:05 ` Conor Dooley
2023-09-15 20:32 ` Leonardo Bras
2023-09-14 14:25 ` Andrew Jones
2023-09-14 14:47 ` Andrew Jones
2023-09-15 11:37 ` Conor Dooley
2023-09-15 12:14 ` Andrew Jones
2023-09-15 12:53 ` Conor Dooley
2023-12-31 8:29 ` guoren
2023-09-10 8:28 ` [PATCH V11 04/17] locking/qspinlock: Improve xchg_tail for number of cpus >= 16k guoren
2023-09-11 2:35 ` Waiman Long
2023-09-11 3:09 ` Guo Ren
2023-09-11 13:03 ` Waiman Long
2023-09-12 1:10 ` Guo Ren
2023-09-13 8:55 ` Leonardo Bras
2023-09-13 12:52 ` Guo Ren
2023-09-13 13:06 ` Waiman Long
2023-09-14 3:45 ` Guo Ren
2023-09-10 8:28 ` [PATCH V11 05/17] riscv: qspinlock: Add basic queued_spinlock support guoren
2023-09-13 20:28 ` Leonardo Bras
2023-09-14 4:46 ` Guo Ren
2023-09-14 9:43 ` Leonardo Bras
2023-09-15 2:10 ` Guo Ren
2023-09-15 9:08 ` Leonardo Bras
2023-09-17 15:02 ` Guo Ren
2023-09-19 5:20 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 06/17] riscv: qspinlock: Introduce combo spinlock guoren
2023-09-10 11:06 ` Guo Ren
2023-09-13 20:37 ` Leonardo Bras
2023-09-13 20:49 ` Leonardo Bras
2023-09-14 4:49 ` Guo Ren [this message]
2023-09-14 7:17 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 07/17] riscv: qspinlock: Introduce qspinlock param for command line guoren
2023-09-11 15:22 ` Waiman Long
2023-09-12 1:06 ` Guo Ren
2023-09-11 15:34 ` Waiman Long
2023-09-12 1:08 ` Guo Ren
2023-09-14 7:32 ` Leonardo Bras
2023-09-14 17:23 ` Waiman Long
2023-09-10 8:29 ` [PATCH V11 08/17] riscv: qspinlock: Add virt_spin_lock() support for KVM guest guoren
2023-09-14 8:02 ` Leonardo Bras
2023-09-17 15:12 ` Guo Ren
2023-09-19 5:30 ` Leonardo Bras
2023-09-19 8:04 ` Guo Ren
2023-09-19 14:40 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 09/17] riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup guoren
2023-09-14 8:32 ` Leonardo Bras
2023-09-17 15:15 ` Guo Ren
2023-09-19 5:34 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 10/17] riscv: qspinlock: errata: Enable qspinlock for T-HEAD processors guoren
2023-09-14 9:36 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 11/17] RISC-V: paravirt: pvqspinlock: Add paravirt qspinlock skeleton guoren
2023-09-15 5:42 ` Leonardo Bras
2023-09-17 14:58 ` Guo Ren
2023-09-19 5:43 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 12/17] RISC-V: paravirt: pvqspinlock: Add nopvspin kernel parameter guoren
2023-09-15 6:05 ` Leonardo Bras
2023-09-17 15:03 ` Guo Ren
2023-09-19 5:44 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 13/17] RISC-V: paravirt: pvqspinlock: Add SBI implementation guoren
2023-09-15 6:23 ` Leonardo Bras
2023-09-17 15:06 ` Guo Ren
2023-09-19 5:45 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 14/17] RISC-V: paravirt: pvqspinlock: Add kconfig entry guoren
2023-09-15 6:25 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 15/17] RISC-V: paravirt: pvqspinlock: Add trace point for pv_kick/wait guoren
2023-09-15 6:33 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 16/17] RISC-V: paravirt: pvqspinlock: KVM: Add paravirt qspinlock skeleton guoren
2023-09-15 6:46 ` Leonardo Bras
2023-09-10 8:29 ` [PATCH V11 17/17] RISC-V: paravirt: pvqspinlock: KVM: Implement kvm_sbi_ext_pvlock_kick_cpu() guoren
2023-09-15 6:52 ` Leonardo Bras
2023-09-10 8:58 ` [PATCH V11 00/17] riscv: Add Native/Paravirt qspinlock support Conor Dooley
2023-09-10 9:16 ` Guo Ren
2023-09-10 9:20 ` Guo Ren
2023-09-10 9:31 ` Conor Dooley
2023-09-10 9:49 ` Guo Ren
2023-09-10 19:45 ` Conor Dooley
2023-09-11 3:36 ` Guo Ren
2023-09-11 12:52 ` Conor Dooley
2023-09-12 1:33 ` Guo Ren
2023-09-12 8:07 ` Conor Dooley
2023-09-12 10:58 ` Guo Ren
2023-11-06 20:42 ` Leonardo Bras
2023-11-12 4:23 ` Guo Ren
2023-11-13 10:19 ` Leonardo Bras Soares Passos
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJF2gTQ=K_2km_AUZj1Vg8q2XgWB49-+nxViAa1mSAfppp6dnA@mail.gmail.com' \
--to=guoren@kernel.org \
--cc=ajones@ventanamicro.com \
--cc=alexghiti@rivosinc.com \
--cc=anup@brainfault.org \
--cc=bjorn@rivosinc.com \
--cc=boqun.feng@gmail.com \
--cc=catalin.marinas@arm.com \
--cc=conor.dooley@microchip.com \
--cc=greentime.hu@sifive.com \
--cc=guoren@linux.alibaba.com \
--cc=jszhang@kernel.org \
--cc=keescook@chromium.org \
--cc=kvm@vger.kernel.org \
--cc=leobras@redhat.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-csky@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=longman@redhat.com \
--cc=mingo@redhat.com \
--cc=palmer@rivosinc.com \
--cc=paul.walmsley@sifive.com \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=rdunlap@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=virtualization@lists.linux-foundation.org \
--cc=wefu@redhat.com \
--cc=will@kernel.org \
--cc=wuwei2016@iscas.ac.cn \
--cc=xiaoguang.xing@sophgo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).