From: Guo Ren <guoren@kernel.org>
To: Conor Dooley <conor.dooley@microchip.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,
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 V10 07/19] riscv: qspinlock: errata: Introduce ERRATA_THEAD_QSPINLOCK
Date: Sat, 5 Aug 2023 09:28:56 +0800 [thread overview]
Message-ID: <CAJF2gTSvjbDF9D0wepAHo65zEkMfg2b+KhsVRpSEK+Qg-q=1gw@mail.gmail.com> (raw)
In-Reply-To: <20230804-throwaway-requisite-c73ebe3fee8c@wendy>
On Fri, Aug 4, 2023 at 6:07 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Fri, Aug 04, 2023 at 05:53:35PM +0800, Guo Ren wrote:
> > On Fri, Aug 4, 2023 at 5:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > > On Wed, Aug 02, 2023 at 12:46:49PM -0400, guoren@kernel.org wrote:
> > > > From: Guo Ren <guoren@linux.alibaba.com>
>
> > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > index f8dbbe1bbd34..d9694fe40a9a 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -342,7 +342,8 @@ void __init riscv_fill_hwcap(void)
> > > > * spinlock value, the only way is to change from queued_spinlock to
> > > > * ticket_spinlock, but can not be vice.
> > > > */
> > > > - if (!force_qspinlock) {
> > > > + if (!force_qspinlock &&
> > > > + !riscv_has_errata_thead_qspinlock()) {
> > > > set_bit(RISCV_ISA_EXT_XTICKETLOCK, isainfo->isa);
> > >
> > > Is this a generic vendor extension (lol @ that misnomer) or is it an
> > > erratum? Make your mind up please. As has been said on other series, NAK
> > > to using march/vendor/imp IDs for feature probing.
> >
> > The RISCV_ISA_EXT_XTICKETLOCK is a feature extension number,
>
> No, that is not what "ISA_EXT" means, nor what the X in "XTICKETLOCK"
> would imply.
>
> The comment above these reads:
> These macros represent the logical IDs of each multi-letter RISC-V ISA
> extension and are used in the ISA bitmap.
>
> > and it's
> > set by default for forward-compatible. We also define a vendor
> > extension (riscv_has_errata_thead_qspinlock) to force all our
> > processors to use qspinlock; others still stay on ticket_lock.
>
> No, "riscv_has_errata_thead_qspinlock()" would be an _erratum_, not a
> vendor extension. We need to have a discussion about how to support
> non-standard extensions etc, not abuse errata. That discussion has been
> started on the v0.7.1 vector patches, but has not made progress yet.
You convinced me, yes, I abuse errata here. I would change to Linux
standard static_key mechanism next.
>
> > The only possible changing direction is from qspinlock to ticket_lock
> > because ticket_lock would dirty the lock value, which prevents
> > changing to qspinlock next. So startup with qspinlock and change to
> > ticket_lock before smp up. You also could use cmdline to try qspinlock
> > (force_qspinlock).
>
> I don't see what the relevance of this is, sorry. I am only commenting
> on how you are deciding that the hardware is capable of using qspinlocks,
> I don't intend getting into the detail unless the powers that be deem
> this series worthwhile, as I mentioned:
> > > I've got some thoughts on other parts of this series too, but I'm not
> > > going to spend time on it unless the locking people and Palmer ascent
> > > to this series.
>
--
Best Regards
Guo Ren
next prev parent reply other threads:[~2023-08-05 1:29 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-02 16:46 [PATCH V10 00/19] riscv: Add Native/Paravirt/CNA qspinlock support guoren
2023-08-02 16:46 ` [PATCH V10 01/19] asm-generic: ticket-lock: Reuse arch_spinlock_t of qspinlock guoren
2023-08-02 16:46 ` [PATCH V10 02/19] asm-generic: ticket-lock: Move into ticket_spinlock.h guoren
2023-08-02 16:46 ` [PATCH V10 03/19] riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup guoren
2023-08-02 16:46 ` [PATCH V10 04/19] riscv: qspinlock: Add basic queued_spinlock support guoren
2023-08-11 19:34 ` Waiman Long
2023-08-12 0:18 ` Guo Ren
2023-08-02 16:46 ` [PATCH V10 05/19] riscv: qspinlock: Introduce combo spinlock guoren
2023-08-11 19:51 ` Waiman Long
2023-08-12 0:22 ` Guo Ren
2023-08-02 16:46 ` [PATCH V10 06/19] riscv: qspinlock: Allow force qspinlock from the command line guoren
2023-08-02 16:46 ` [PATCH V10 07/19] riscv: qspinlock: errata: Introduce ERRATA_THEAD_QSPINLOCK guoren
2023-08-04 9:05 ` Conor Dooley
2023-08-04 9:53 ` Guo Ren
2023-08-04 10:06 ` Conor Dooley
2023-08-05 1:28 ` Guo Ren [this message]
2023-08-07 5:23 ` Stefan O'Rear
2023-08-08 2:12 ` Guo Ren
2023-09-13 18:54 ` Palmer Dabbelt
2023-09-13 19:32 ` Waiman Long
2023-09-14 3:31 ` Guo Ren
2023-08-02 16:46 ` [PATCH V10 08/19] riscv: qspinlock: Use new static key for controlling call of virt_spin_lock() guoren
2023-08-02 16:46 ` [PATCH V10 09/19] RISC-V: paravirt: pvqspinlock: Add paravirt qspinlock skeleton guoren
2023-08-02 16:46 ` [PATCH V10 10/19] RISC-V: paravirt: pvqspinlock: KVM: " guoren
2023-08-02 16:46 ` [PATCH V10 11/19] RISC-V: paravirt: pvqspinlock: KVM: Implement kvm_sbi_ext_pvlock_kick_cpu() guoren
2023-08-02 16:46 ` [PATCH V10 12/19] RISC-V: paravirt: pvqspinlock: Add nopvspin kernel parameter guoren
2023-08-02 16:46 ` [PATCH V10 13/19] RISC-V: paravirt: pvqspinlock: Remove unnecessary definitions of cmpxchg & xchg guoren
2023-08-02 16:46 ` [PATCH V10 14/19] RISC-V: paravirt: pvqspinlock: Add xchg8 & cmpxchg_small support guoren
2023-08-02 16:46 ` [PATCH V10 15/19] RISC-V: paravirt: pvqspinlock: Add SBI implementation guoren
2023-08-02 16:46 ` [PATCH V10 16/19] RISC-V: paravirt: pvqspinlock: Add kconfig entry guoren
2023-08-02 16:46 ` [PATCH V10 17/19] RISC-V: paravirt: pvqspinlock: Add trace point for pv_kick/wait guoren
2023-08-02 16:47 ` [PATCH V10 18/19] locking/qspinlock: Move pv_ops into x86 directory guoren
2023-08-11 20:42 ` Waiman Long
2023-08-12 0:24 ` Guo Ren
2023-08-12 0:47 ` Waiman Long
2023-08-02 16:47 ` [PATCH V10 19/19] locking/qspinlock: riscv: Add Compact NUMA-aware lock support guoren
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='CAJF2gTSvjbDF9D0wepAHo65zEkMfg2b+KhsVRpSEK+Qg-q=1gw@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=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).