On Wed, Jun 29, 2022 at 3:09 PM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Wed, Jun 29, 2022 at 3:34 AM Waiman Long <longman@xxxxxxxxxx> wrote: > > On 6/28/22 21:17, Guo Ren wrote: > > > On Wed, Jun 29, 2022 at 2:13 AM Waiman Long <longman@xxxxxxxxxx> wrote: > > >> On 6/28/22 04:17, guoren@xxxxxxxxxx wrote: > > >> > > >> So the current config setting determines if qspinlock will be used, not > > >> some boot time parameter that user needs to specify. This patch will > > >> just add useless code to lock/unlock sites. I don't see any benefit of > > >> doing that. > > > This is a startup patch for riscv. next, we could let vendors make choices. > > > I'm not sure they like cmdline or vendor-specific errata style. > > > > > > Eventually, we would let one riscv Image support all machines, some > > > use ticket-lock, and some use qspinlock. > > > > OK. Maybe you can postpone this combo spinlock until there is a good use > > case for it. Upstream usually don't accept patches that have no good use > > case yet. > > I think the usecase on risc-v is this: there are cases where the qspinlock > is preferred for performance reasons, but there are also CPU cores on > which it is not safe to use. risc-v like most modern architectures has a > strict rule about being able to build kernels that work on all machines, > so without something like this, it would not be able to use qspinlock at all. > > On the other hand, I don't really like the idea of putting the static-key > wrapper into the asm-generic header. Especially the ticket spinlock > implementation should be simple and not depend on jump labels. If CONFIG_ARCH_USE_QUEUED_SPINLOCKS is not enabled, the patch still will keep the ticket-lock simple without jump labels. > > From looking at the header file dependencies on arm64, I know that > putting jump labels into core infrastructure like the arch_spin_lock() > makes a big mess of indirect includes and measurably slows down > the kernel build. arm64 needn't combo spinlock, it could use pure qspinlock with keeping current header files included. > > I think this can still be done in the riscv asm/spinlock.h header with > minimal impact on the asm-generic file if the riscv maintainers see > a significant enough advantage, but I don't want it in the common code. Yes, it could. I agree with using combo spinlock only with riscv. > > Arnd -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/
