Re: [PATCH v14 00/15] barrier: Add smp_cond_load_{relaxed,acquire}_timeout()
From: Ankur Arora
Date: Tue Jul 28 2026 - 19:36:39 EST
Hi Andrew
This series cuts across a variety of subsystems and at this point most
patches have been reviewed or acked.
The only ones not are the newly added interface tif_need_resched_relaxed_wait()
in patch-11 "sched: add need-resched timed wait interface" which is a simple
wrapper over the added barrier or, patches 14,15 "barrier: add tests for
smp_cond_load_*_timeout()" and, "barrier: add clock tests for
smp_cond_load_relaxed_timeout()" which add kunit tests.
Would it be possible for you take this in mm-nonmm?
The only things missing are a few issues flagged by sashiko/bpf which
as I mention below aren't worth addressing (or are newly introduced
typos).
Happy to respin to fix those if you think that's better.
Once this is in I would like to send some patches to idle where the
new interface could be used to poll in idle on arm64.
Thanks
Ankur
Ankur Arora <ankur.a.arora@xxxxxxxxxx> writes:
> Ankur Arora <ankur.a.arora@xxxxxxxxxx> writes:
>
>> Hi,
>>
>> Changes in this version:
>>
>> - rename kconfig entry for the barrier kunit test to follow
>> the kunit style guide
>> (s/BARRIER_TIMEOUT_TEST/BARRIER_TIMEOUT_KUNIT_TEST)
>>
>> - make the kunit test be visible only if CONFIG_KUNIT_ALL_TESTS
>> is not enabled.
>>
>> Both comments from Julian Braha.
>>
>> (There were also comments from the sashiko and bpf-bot which I've
>> commented on in prior versions of this series and which don't
>> make sense to address.)
>
> Sashiko/bpf-bot review comments so far are a repeat of earlier
> ones. There are a few new typos that got flagged. Will address
> those if I end up sending another version out because of more
> substantive comments.
>
>> The core kernel often uses smp_cond_load_{relaxed,acquire}() to spin
>> on condition variables with architectural primitives used to avoid
>> hammering the relevant cachelines.
>>
>> (This primitive can vary greatly across architectures: on x86 it's a
>> cpu_relax() to slow down the pipeline. On arm64, this is a __cmpwait()
>> which waits for a cacheline to change state in a time limited fashion.)
>>
>> Regardless of architectural details, typical smp_cond_load*() usage
>> does not allow for termination until the condition change occurs.
>>
>> Beyond the core kernel, there are cases where it is useful to additionally
>> terminate on a timeout. Two cases:
>>
>> - cpuidle poll_idle(): wait for need-resched until the cpuidle polling
>> duration expires.
>>
>> - rqspinlock: nested qspinlock acquisition that terminates on timeout
>> or deadlock.
>>
>> Accordingly add two interfaces (with their generic and arm64 specific
>> implementations):
>>
>> smp_cond_load_relaxed_timeout(ptr, cond_expr, time_expr, timeout)
>> smp_cond_load_acquire_timeout(ptr, cond_expr, time_expr, timeout)
>>
>> Also add tif_need_resched_relaxed_wait() which wraps the polling
>> pattern and its scheduler specific details in poll_idle().
>> In addition add atomic_cond_read_*_timeout(),
>> atomic64_cond_read_*_timeout(), and atomic_long wrappers.
>>
>> Structurally, both the smp_cond_load_*_timeout() interfaces are similar
>> to smp_cond_load*(), with the addition of a rate-limited time-check.
>>
>> Usage
>> ==
>>
>> These interfaces drop straight-forwardly into the rqspinlock logic
>> since qspinlock already uses smp_cond_load*(), and the time-check
>> extension can now be used for timeout and deadlock handling.
>>
>> Using tif_need_resched_relaxed_wait() in poll_idle() removes any
>> architectural details allowing arm64 to straight-forwardly support
>> that path.
>> (However, for efficiency reasons cpuidle/poll_state.c continues to
>> depend on ARCH_HAS_CPU_RELAX since that is defined on architectures
>> with an optimized architectural primitive.)
>>
>>
>> Performance
>> ==
>>
>> Apart from simplifications due to this change, supporting polling in
>> cpuidle on arm64 helps improve wakeup latency (needs a few cpuidle/acpi
>> patches):
>>
>>
>> # perf stat -r 5 --cpu 4,5 -e task-clock,cycles,instructions,sched:sched_wake_idle_without_ipi \
>> perf bench sched pipe -l 1000000 -c 4
>>
>> # No haltpoll (and, no TIF_POLLING_NRFLAG):
>>
>> Performance counter stats for 'CPU(s) 4,5' (5 runs):
>>
>> 25,229.57 msec task-clock # 2.000 CPUs utilized ( +- 7.75% )
>> 45,821,250,284 cycles # 1.816 GHz ( +- 10.07% )
>> 26,557,496,665 instructions # 0.58 insn per cycle ( +- 0.21% )
>> 0 sched:sched_wake_idle_without_ipi # 0.000 /sec
>>
>> 12.615 +- 0.977 seconds time elapsed ( +- 7.75% )
>>
>>
>> # Haltpoll:
>>
>> Performance counter stats for 'CPU(s) 4,5' (5 runs):
>>
>> 15,131.58 msec task-clock # 2.000 CPUs utilized ( +- 10.00% )
>> 34,158,188,839 cycles # 2.257 GHz ( +- 6.91% )
>> 20,824,950,916 instructions # 0.61 insn per cycle ( +- 0.09% )
>> 1,983,822 sched:sched_wake_idle_without_ipi # 131.105 K/sec ( +- 0.78% )
>>
>> 7.566 +- 0.756 seconds time elapsed ( +- 10.00% )
>>
>> We get improved latency because we don't switch in and out of a
>> deeper sleep state or from the hypervisor. This also causes us to
>> execute ~20% fewer instructions.
>>
>>
>> Haris Okanovic also saw improvement in real workloads due to the
>> cpuidle changes: "observed 4-6% improvements in memcahed, cassandra,
>> mysql, and postgresql under certain loads. Other applications likely
>> benefit too." [12]
>>
>>
>> Changelog:
>>
>> v13 [15] (as listed above):
>> - rename kconfig entry for the barrier kunit test to follow
>> the kunit style guide
>> (s/BARRIER_TIMEOUT_TEST/BARRIER_TIMEOUT_KUNIT_TEST)
>>
>> - make the kunit test be visible only if CONFIG_KUNIT_ALL_TESTS
>> is not enabled.
>>
>> Both comments from Julian Braha.
>>
>> v12 [14]:
>> - smp_cond_load_acquire_timeout() now only has acquire semantics in
>> the success (non-timeout) case.
>>
>> - arm64 now does not define ARCH_HAS_CPU_RELAX as without also
>> defining TIF_POLLING_NRFLAG, in some cases we end up with a
>> degenerate version of poll_idle().
>>
>> - kunit: removed the test case for timeout=-1 (not supported)
>> Also add test cases for timeout=0, timeout=1.
>>
>> (All of these address review comments from sashiko/bpf-bot.)
>>
>> v11 [13]:
>> - addressed some review comments from sashiko (see commit notes)
>> - The one notable change is to the implementation of
>> smp_cond_load_acquire_timeout() where there was a missed
>> control dependency in the timeout case.
>> All the others are minor.
>> - fixed a low probability race in the kunit test added in v11.
>> - added a bunch of kunit tests validating the implementation's
>> use of the clock.
>>
>> v10 [10]:
>> - add a comment mentioning that smp_cond_load_relaxed_timeout() might
>> be using architectural primitives that don't support MMIO.
>> (David Laight, Catalin Marinas)
>> - added a kunit test for smp_cond_load_relaxed_timeout() (Andrew
>> Morton.)
>>
>> v9 [9]:
>> - s/@cond/@cond_expr/ (Randy Dunlap)
>> - Clarify that SMP_TIMEOUT_POLL_COUNT is only around memory
>> addresses. (David Laight)
>> - Add the missing config ARCH_HAS_CPU_RELAX in arch/arm64/Kconfig.
>> (Catalin Marinas).
>> - Switch to arch_counter_get_cntvct_stable() (via __delay_cycles())
>> in the cmpwait path instead of using arch_timer_read_counter().
>> (Catalin Marinas)
>>
>> v8 [0]:
>> - Defer evaluation of @time_expr_ns to when we hit the slowpath.
>> (comment from Alexei Starovoitov).
>>
>> - Mention that cpu_poll_relax() is better than raw CPU polling
>> only where ARCH_HAS_CPU_RELAX is defined.
>> - also define ARCH_HAS_CPU_RELAX for arm64.
>> (Came out of a discussion with Will Deacon.)
>>
>> - Split out WFET and WFE handling. I was doing both of these
>> in a common handler.
>> (From Will Deacon and in an earlier revision by Catalin Marinas.)
>>
>> - Add mentions of atomic_cond_read_{relaxed,acquire}(),
>> atomic_cond_read_{relaxed,acquire}_timeout() in
>> Documentation/atomic_t.txt.
>>
>> - Use the BIT() macro to do the checking in tif_bitset_relaxed_wait().
>>
>> - Cleanup unnecessary assignments, casts etc in poll_idle().
>> (From Rafael Wysocki.)
>>
>> - Fixup warnings from kernel build robot
>>
>>
>> v7 [1]:
>> - change the interface to separately provide the timeout. This is
>> useful for supporting WFET and similar primitives which can do
>> timed waiting (suggested by Arnd Bergmann).
>>
>> - Adapting rqspinlock code to this changed interface also
>> necessitated allowing time_expr to fail.
>> - rqspinlock changes to adapt to the new smp_cond_load_acquire_timeout().
>>
>> - add WFET support (suggested by Arnd Bergmann).
>> - add support for atomic-long wrappers.
>> - add a new scheduler interface tif_need_resched_relaxed_wait() which
>> encapsulates the polling logic used by poll_idle().
>> - interface suggested by (Rafael J. Wysocki).
>>
>>
>> v6 [2]:
>> - fixup missing timeout parameters in atomic64_cond_read_*_timeout()
>> - remove a race between setting of TIF_NEED_RESCHED and the call to
>> smp_cond_load_relaxed_timeout(). This would mean that dev->poll_time_limit
>> would be set even if we hadn't spent any time waiting.
>> (The original check compared against local_clock(), which would have been
>> fine, but I was instead using a cheaper check against _TIF_NEED_RESCHED.)
>> (Both from meta-CI bot)
>>
>>
>> v5 [3]:
>> - use cpu_poll_relax() instead of cpu_relax().
>> - instead of defining an arm64 specific
>> smp_cond_load_relaxed_timeout(), just define the appropriate
>> cpu_poll_relax().
>> - re-read the target pointer when we exit due to the time-check.
>> - s/SMP_TIMEOUT_SPIN_COUNT/SMP_TIMEOUT_POLL_COUNT/
>> (Suggested by Will Deacon)
>>
>> - add atomic_cond_read_*_timeout() and atomic64_cond_read_*_timeout()
>> interfaces.
>> - rqspinlock: use atomic_cond_read_acquire_timeout().
>> - cpuidle: use smp_cond_load_relaxed_tiemout() for polling.
>> (Suggested by Catalin Marinas)
>>
>> - rqspinlock: define SMP_TIMEOUT_POLL_COUNT to be 16k for non arm64
>>
>>
>> v4 [4]:
>> - naming change 's/timewait/timeout/'
>> - resilient spinlocks: get rid of res_smp_cond_load_acquire_waiting()
>> and fixup use of RES_CHECK_TIMEOUT().
>> (Both suggested by Catalin Marinas)
>>
>> v3 [5]:
>> - further interface simplifications (suggested by Catalin Marinas)
>>
>> v2 [6]:
>> - simplified the interface (suggested by Catalin Marinas)
>> - get rid of wait_policy, and a multitude of constants
>> - adds a slack parameter
>> This helped remove a fair amount of duplicated code duplication and in
>> hindsight unnecessary constants.
>>
>> v1 [7]:
>> - add wait_policy (coarse and fine)
>> - derive spin-count etc at runtime instead of using arbitrary
>> constants.
>>
>> Haris Okanovic tested v4 of this series with poll_idle()/haltpoll patches. [8]
>>
>> Comments appreciated!
>>
>> Thanks
>> Ankur
>>
>> [0] https://lore.kernel.org/lkml/20251215044919.460086-1-ankur.a.arora@xxxxxxxxxx/
>> [1] https://lore.kernel.org/lkml/20251028053136.692462-1-ankur.a.arora@xxxxxxxxxx/
>> [2] https://lore.kernel.org/lkml/20250911034655.3916002-1-ankur.a.arora@xxxxxxxxxx/
>> [3] https://lore.kernel.org/lkml/20250911034655.3916002-1-ankur.a.arora@xxxxxxxxxx/
>> [4] https://lore.kernel.org/lkml/20250829080735.3598416-1-ankur.a.arora@xxxxxxxxxx/
>> [5] https://lore.kernel.org/lkml/20250627044805.945491-1-ankur.a.arora@xxxxxxxxxx/
>> [6] https://lore.kernel.org/lkml/20250502085223.1316925-1-ankur.a.arora@xxxxxxxxxx/
>> [7] https://lore.kernel.org/lkml/20250203214911.898276-1-ankur.a.arora@xxxxxxxxxx/
>> [8] https://lore.kernel.org/lkml/2cecbf7fb23ee83a4ce027e1be3f46f97efd585c.camel@xxxxxxxxxx/
>> [9] https://lore.kernel.org/lkml/20260209023153.2661784-1-ankur.a.arora@xxxxxxxxxx/
>> [10] https://lore.kernel.org/lkml/20260316013651.3225328-1-ankur.a.arora@xxxxxxxxxx/
>> [11] https://lore.kernel.org/lkml/20230809134837.GM212435@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>> [12] https://lore.kernel.org/lkml/c6f3c8d3f1f2e89a9dc7ae22482973b5a51b08cb.camel@xxxxxxxxxx/
>> [13] https://lore.kernel.org/all/20260408122538.3610871-1-ankur.a.arora@xxxxxxxxxx/#r
>> [14] https://lore.kernel.org/all/20260608080440.127491-1-ankur.a.arora@xxxxxxxxxx/
>> [15] https://lore.kernel.org/all/20260702013334.140905-1-ankur.a.arora@xxxxxxxxxx/
>>
>> Cc: Arnd Bergmann <arnd@xxxxxxxx>
>> Cc: Will Deacon <will@xxxxxxxxxx>
>> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
>> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
>> Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
>> Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
>> Cc: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
>> Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Cc: bpf@xxxxxxxxxxxxxxx
>> Cc: linux-arch@xxxxxxxxxxxxxxx
>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> Cc: linux-pm@xxxxxxxxxxxxxxx
>>
>> Ankur Arora (15):
>> asm-generic: barrier: Add smp_cond_load_relaxed_timeout()
>> arm64: barrier: Support smp_cond_load_relaxed_timeout()
>> arm64/delay: move some constants out to a separate header
>> arm64: support WFET in smp_cond_load_relaxed_timeout()
>> arm64: rqspinlock: Remove private copy of
>> smp_cond_load_acquire_timewait()
>> asm-generic: barrier: Add smp_cond_load_acquire_timeout()
>> atomic: Add atomic_cond_read_*_timeout()
>> locking/atomic: scripts: build atomic_long_cond_read_*_timeout()
>> bpf/rqspinlock: switch check_timeout() to a clock interface
>> bpf/rqspinlock: Use smp_cond_load_acquire_timeout()
>> sched: add need-resched timed wait interface
>> cpuidle/poll_state: Wait for need-resched via
>> tif_need_resched_relaxed_wait()
>> arm64/delay: enable testing smp_cond_load_relaxed_timeout()
>> barrier: add tests for smp_cond_load_*_timeout()
>> barrier: add clock tests for smp_cond_load_relaxed_timeout()
>>
>> Documentation/atomic_t.txt | 14 +-
>> arch/arm64/include/asm/barrier.h | 23 ++++
>> arch/arm64/include/asm/cmpxchg.h | 62 +++++++--
>> arch/arm64/include/asm/delay-const.h | 28 ++++
>> arch/arm64/include/asm/rqspinlock.h | 85 ------------
>> arch/arm64/lib/delay.c | 17 +--
>> drivers/clocksource/arm_arch_timer.c | 2 +
>> drivers/cpuidle/poll_state.c | 21 +--
>> drivers/soc/qcom/rpmh-rsc.c | 8 +-
>> include/asm-generic/barrier.h | 119 +++++++++++++++++
>> include/linux/atomic.h | 10 ++
>> include/linux/atomic/atomic-long.h | 18 ++-
>> include/linux/sched/idle.h | 29 +++++
>> kernel/bpf/rqspinlock.c | 77 +++++++----
>> lib/Kconfig.debug | 10 ++
>> lib/tests/Makefile | 1 +
>> lib/tests/barrier-timeout-test.c | 186 +++++++++++++++++++++++++++
>> scripts/atomic/gen-atomic-long.sh | 16 ++-
>> 18 files changed, 548 insertions(+), 178 deletions(-)
>> create mode 100644 arch/arm64/include/asm/delay-const.h
>> create mode 100644 lib/tests/barrier-timeout-test.c
--
ankur