Re: [PATCH v4 1/4] Introducing pw_lock() and per-cpu queue & flush work
Leonardo Bras <[email protected]>
| Newsgroups | dev.linux.lists.linux-rt-devel,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <alQFuIPKRjQ_vjmg@WindFlash> |
On Wed, May 20, 2026 at 03:06:30PM -0700, Randy Dunlap wrote: > > > On 5/18/26 6:27 PM, Leonardo Bras wrote: > > diff --git a/MAINTAINERS b/MAINTAINERS > > index c2c6d79275c6..7102031207c9 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -21775,20 +21775,27 @@ QORIQ DPAA2 FSL-MC BUS DRIVER > > M: Ioana Ciornei <[email protected]> > > L: [email protected] > > L: [email protected] > > S: Maintained > > F: Documentation/ABI/stable/sysfs-bus-fsl-mc > > F: Documentation/devicetree/bindings/misc/fsl,qoriq-mc.yaml > > F: Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst > > F: drivers/bus/fsl-mc/ > > F: include/uapi/linux/fsl_mc.h > > > > +PW Locks > > +M: Leonardo Bras <[email protected]> > > +S: Supported > > +F: Documentation/locking/pwlocks.rst > > +F: include/linux/pwlocks.h > > +F: kernel/pwlocks.c > > MAINTAINERS entries should be in alphabetical order: PW is not in the > middle of the Q entries. Hi Randy, thank you for reviewing! Argh, sorry, it used to be called QPW. I renamed and forgot to reorder. > > > + > > QT1010 MEDIA DRIVER > > L: [email protected] > > S: Orphan > > W: https://linuxtv.org > > Q: http://patchwork.linuxtv.org/project/linux-media/list/ > > F: drivers/media/tuners/qt1010* > > > > QUALCOMM ATH12K WIRELESS DRIVER > > M: Jeff Johnson <[email protected]> > > L: [email protected] > > > > diff --git a/Documentation/locking/pwlocks.rst b/Documentation/locking/pwlocks.rst > > new file mode 100644 > > index 000000000000..09f4a5417bc1 > > --- /dev/null > > +++ b/Documentation/locking/pwlocks.rst > > @@ -0,0 +1,76 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > +========= > > +PW (Per-CPU Work) locks > > +========= > > Overline and underline should be at least as long as the heading text. > Noted! > > + > > +Some places in the kernel implement a parallel programming strategy > > +consisting on local_locks() for most of the work, and some rare remote > > +operations are scheduled on target cpu. This keeps cache bouncing low since > > on a target CPU. Right > > > +cacheline tends to be mostly local, and avoids the cost of locks in non-RT > > +kernels, even though the very few remote operations will be expensive due > > +to scheduling overhead. > > + > > +On the other hand, for RT workloads this can represent a problem: > > +scheduling work on remote cpu that are executing low latency tasks > > CPUs Will fix > > > +is undesired and can introduce unexpected deadline misses. > > undesirable > ? Yes > > > + > > +PW locks help to convert sites that use local_locks (for cpu local operations) > > +and queue_work_on (for queueing work remotely, to be executed > > +locally on the owner cpu of the lock) to a spinlocks. > > to spinlocks. > Right > > + > > +The lock is declared pw_lock_t type. > > +The lock is initialized with pw_lock_init. > > +The lock is locked with pw_lock (takes a lock and cpu as a parameter). > > +The lock is unlocked with pw_unlock (takes a lock and cpu as a parameter). > > + > > +The pw_lock_irqsave function disables interrupts and saves current interrupt state, > > +cpu as a parameter. > > + > > +For trylock variant, there is the pw_trylock_t type, initialized with > > +pw_trylock_init. Then the corresponding pw_trylock and pw_trylock_irqsave. > > + > > +work_struct should be replaced by pw_struct, which contains a cpu parameter > > +(owner cpu of the lock), initialized by INIT_PW. > > + > > +The queue work related functions (analogous to queue_work_on and flush_work) are: > > +pw_queue_on and pw_flush. > > + > > +The behaviour of the PW lock functions is as follows: > > + > > +* !CONFIG_PWLOCKS (or CONFIG_PWLOCKS and pwlocks=off kernel boot parameter): > > + - pw_lock: local_lock > > + - pw_lock_irqsave: local_lock_irqsave > > + - pw_trylock: local_trylock > > + - pw_trylock_irqsave: local_trylock_irqsave > > + - pw_unlock: local_unlock > > + - pw_lock_local: local_lock > > + - pw_trylock_local: local_trylock > > + - pw_unlock_local: local_unlock > > + - pw_queue_on: queue_work_on > > + - pw_flush: flush_work > > + > > +* CONFIG_PWLOCKS (and CONFIG_PWLOCKS_DEFAULT=y or pwlocks=on kernel boot parameter), > > + - pw_lock: spin_lock > > + - pw_lock_irqsave: spin_lock_irqsave > > + - pw_trylock: spin_trylock > > + - pw_trylock_irqsave: spin_trylock_irqsave > > + - pw_unlock: spin_unlock > > + - pw_lock_local: preempt_disable OR migrate_disable + spin_lock > > + - pw_trylock_local: preempt_disable OR migrate_disable + spin_trylock > > + - pw_unlock_local: preempt_enable OR migrate_enable + spin_unlock > > + - pw_queue_on: executes work function on caller cpu > > + - pw_flush: empty > > + > > +pw_get_cpu(work_struct), to be called from within per-cpu work function, > > +returns the target cpu. > > CPU. > Will fix > > + > > +On the locking functions above, there are the local locking functions > > +(pw_lock_local, pw_trylock_local and pw_unlock_local) that must only > > +be used to access per-CPU data from the CPU that owns that data, > > +and never remotely. They disable preemption/migration and don't require > > +a cpu parameter, making them a replacement for local_lock functions that > > +does not introduce overhead. > > + > > +These should only be used when accessing per-CPU data of the local CPU. > > + > > Running "make htmldocs" with this patch says: > > Documentation/locking/pwlocks.rst: WARNING: document isn't included in any toctree [toc.not_included] > Humm, I will figure out a way of including it in a relevant toctree. > > diff --git a/init/Kconfig b/init/Kconfig > > index 2937c4d308ae..3fb751dc4530 100644 > > --- a/init/Kconfig > > +++ b/init/Kconfig > > @@ -764,20 +764,55 @@ config CPU_ISOLATION > > depends on SMP > > default y > > help > > Make sure that CPUs running critical tasks are not disturbed by > > any source of "noise" such as unbound workqueues, timers, kthreads... > > Unbound jobs get offloaded to housekeeping CPUs. This is driven by > > the "isolcpus=" boot parameter. > > > > Say Y if unsure. > > > > +config PWLOCKS > > + bool "Per-CPU Work locks" > > + depends on SMP || COMPILE_TEST > > + default n > > + help > > + Allow changing the behavior on per-CPU resource sharing with cache, > > + from the regular local_locks() + queue_work_on(remote_cpu) to using > > + per-CPU spinlocks on both local and remote operations. > > + > > + This is useful to give user the option on reducing IPIs to CPUs, and > > to give the user Right > > > + thus reduce interruptions and context switches. On the other hand, it > > + increases generated code and will use atomic operations if spinlocks > > + are selected. > > + > > + If set, will use the default behavior set in PWLOCKS_DEFAULT unless boot > > unless the boot > Correct > > + parameter pwlocks is passed with a different behavior. > > + > > + If unset, will use the local_lock() + queue_work_on() strategy, > > + regardless of the boot parameter or PWLOCKS_DEFAULT. > > + > > + Say N if unsure. > > + > > +config PWLOCKS_DEFAULT > > + bool "Use per-CPU spinlocks by default on PWLOCKS" > > + depends on PWLOCKS > > + default n > > + help > > + If set, will use per-CPU spinlocks as default behavior for per-CPU > > + remote operations. > > + > > + If unset, will use local_lock() + queue_work_on(cpu) as default > > + behavior for remote operations. > > + > > + Say N if unsure > > unsure. > Yeap :) > > + > > source "kernel/rcu/Kconfig" > > > > config IKCONFIG > > tristate "Kernel .config support" > > help > > This option enables the complete Linux kernel ".config" file > > contents to be saved in the kernel. It provides documentation > > of which kernel options are used in a running kernel or in an > > on-disk kernel. This information can be extracted from the kernel > > image file with the script scripts/extract-ikconfig and used as > > -- > ~Randy > Thanks! Leo