Re: [PATCH v2 2/5] Introducing qpw_lock() and per-cpu queue & flush work

Frederic Weisbecker
2026-03-13T21:55:54Z

Le Mon, Mar 02, 2026 at 12:49:47PM -0300, Marcelo Tosatti a écrit :
> 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
> 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
> is undesired and can introduce unexpected deadline misses.
>
> It's interesting, though, that local_lock()s in RT kernels become
> spinlock(). We can make use of those to avoid scheduling work on a remote
> cpu by directly updating another cpu's per_cpu structure, while holding
> it's spinlock().
>
> In order to do that, it's necessary to introduce a new set of functions to
> make it possible to get another cpu's per-cpu "local" lock (qpw_{un,}lock*)
> and also the corresponding queue_percpu_work_on() and flush_percpu_work()
> helpers to run the remote work.
>
> Users of non-RT kernels but with low latency requirements can select
> similar functionality by using the CONFIG_QPW compile time option.
>
> On CONFIG_QPW disabled kernels, no changes are expected, as every
> one of the introduced helpers work the exactly same as the current
> implementation:
> qpw_{un,}lock*() -> local_{un,}lock*() (ignores cpu parameter)

I find this part of the semantic a bit weird. If we eventually queue
the work, why do we care about doing a local_lock() locally ?

> queue_percpu_work_on() -> queue_work_on()
> flush_percpu_work() -> flush_work()
>
> @@ -2840,6 +2840,16 @@ Kernel parameters
>
> The format of <cpu-list> is described above.
>
> + qpw= [KNL,SMP] Select a behavior on per-CPU resource sharing
> + and remote interference mechanism on a kernel built with
> + CONFIG_QPW.
> + Format: { "0" | "1" }
> + 0 - local_lock() + queue_work_on(remote_cpu)
> + 1 - spin_lock() for both local and remote operations
> +
> + Selecting 1 may be interesting for systems that want
> + to avoid interruption & context switches from IPIs.

Like Vlastimil suggested, it would be better to just have it off by default
and turn it on only if nohz_full= is passed. Then we can consider introducing
the parameter later if the need arise.

> +#define qpw_lock_init(lock) \
> + local_lock_init(lock)
> +
> +#define qpw_trylock_init(lock) \
> + local_trylock_init(lock)
> +
> +#define qpw_lock(lock, cpu) \
> + local_lock(lock)
> +
> +#define local_qpw_lock(lock) \
> + local_lock(lock)

It would be easier to grep if all the APIs start with qpw_* prefix.

qpw_local_lock() ?

> +
> +#define qpw_lock_irqsave(lock, flags, cpu) \
> + local_lock_irqsave(lock, flags)
> +
> +#define local_qpw_lock_irqsave(lock, flags) \
> + local_lock_irqsave(lock, flags)

ditto?

> +
> +#define qpw_trylock(lock, cpu) \
> + local_trylock(lock)
> +
> +#define local_qpw_trylock(lock) \
> + local_trylock(lock)

...

> +
> +#define qpw_trylock_irqsave(lock, flags, cpu) \
> + local_trylock_irqsave(lock, flags)
> +
> +#define qpw_unlock(lock, cpu) \
> + local_unlock(lock)
> +
> +#define local_qpw_unlock(lock) \
> + local_unlock(lock)

...

> +
> +#define qpw_unlock_irqrestore(lock, flags, cpu) \
> + local_unlock_irqrestore(lock, flags)
> +
> +#define local_qpw_unlock_irqrestore(lock, flags) \
> + local_unlock_irqrestore(lock, flags)

...

> +
> +#define qpw_lockdep_assert_held(lock) \
> + lockdep_assert_held(lock)
> +
> +#define queue_percpu_work_on(c, wq, qpw) \
> + queue_work_on(c, wq, &(qpw)->work)

qpw_queue_work_on() ?

Perhaps even better would be qpw_queue_work_for(), leaving some room for
mystery about where/how the work will be executed :-)

> +
> +#define flush_percpu_work(qpw) \
> + flush_work(&(qpw)->work)

qpw_flush_work() ?

> +
> +#define qpw_get_cpu(qpw) smp_processor_id()
> +
> +#define qpw_is_cpu_remote(cpu) (false)
> +
> +#define INIT_QPW(qpw, func, c) \
> + INIT_WORK(&(qpw)->work, (func))
> +
> @@ -762,6 +762,41 @@ config CPU_ISOLATION
>
> Say Y if unsure.
>
> +config QPW
> + bool "Queue per-CPU Work"
> + 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
> + 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 QPW_DEFAULT unless boot
> + parameter qpw is passed with a different behavior.
> +
> + If unset, will use the local_lock() + queue_work_on() strategy,
> + regardless of the boot parameter or QPW_DEFAULT.
> +
> + Say N if unsure.

Perhaps that too should just be selected automatically by CONFIG_NO_HZ_FULL and if
the need arise in the future, make it visible to the user?

Thanks.

--
Frederic Weisbecker
SUSE Labs