[PATCH v16 00/16] barrier: Add smp_cond_load_{relaxed,acquire}_timeout()

[PATCH v16 00/16] barrier: Add smp_cond_load_{relaxed,acquire}_timeout()

[Ankur Arora]

Hi,

Changes in this version address sashiko comments [17]. The major change
is the change in the exit path in __smp_cond_load_relaxed_timeout()
which allows us to simplify providing ACQUIRE semantics in
smp_cond_load_acquire_timeout():

  - patch-1, "asm-generic: barrier: Add smp_cond_load_relaxed_timeout()"
   - unify the exit path for the loop condition and timeout case
   - includes vdso/time64.h
   - document the timeout_ns == 0 case
   - use u64 for internal arithmetic

  - patch-3, "arm64/delay: move, fixup usecs_to_cycles()"
   - While moving USECS_TO_CYCLES() into the driver namespace also
     make it a static inline rpmh_usecs_to_cycles()

  - patch-6, "asm-generic: barrier: Add smp_cond_load_acquire_timeout()",
    - use the change in patch-1 to fixup, simplify the ACQUIRE path
    - thresh out the commit message a bit more

  - patch 14,15,16 "barrier: add tests for smp_cond_load_*_timeout()"
    - enclose the calls to smp_cond_load_{relaxed,acquire}_timeout()
      in preempt_disable(), preempt_enable().
    - WFET makes it difficult to use a synthetic timer. So skip
      timeout tests on configurations with WFET.

Haris Okaovic tested v15 [18] "Tested again on AWS Graviton3/4/5
(ARM Neoverse V1/V2/V3) with haltpoll changes atop latest master
8ab1afb2e: still seeing average 6% improvement across various workloads
(mysql, postgresql, cassandra, memcached) in latency sensitive
configuration.  The new unit tests all pass on both metal and virt
variants of the same machines. -- Haris"

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.


Changelog:

  v15 [17] (as listed above): 
   - patch-1, "asm-generic: barrier: Add smp_cond_load_relaxed_timeout()"
    - unify the exit path for the loop condition and timeout case
    - includes vdso/time64.h
    - document the timeout_ns == 0 case
    - use u64 for internal arithmetic
  
   - patch-3, "arm64/delay: move, fixup usecs_to_cycles()"
    - While moving USECS_TO_CYCLES() into the driver namespace also
      make it a static inline rpmh_usecs_to_cycles()
  
   - patch-6, "asm-generic: barrier: Add smp_cond_load_acquire_timeout()",
     - use the change in patch-1 to fixup, simplify the ACQUIRE path
     - thresh out the commit message a bit more
  
   - patch 14,15,16 "barrier: add tests for smp_cond_load_*_timeout()"
     - enclose the calls to smp_cond_load_{relaxed,acquire}_timeout()
       in preempt_disable(), preempt_enable().
     - WFET makes it difficult to use a synthetic timer. So skip
       timeout tests on configurations with WFET.


  v14 [16]:
    - patch-1, "asm-generic: barrier: Add smp_cond_load_relaxed_timeout()"
     - deferred time-check is now limited to a single usec.

     - smp_cond_load_relaxed_timeout() now enforces that the timeout_ns value
       does fit in s64.

     - SMP_TIMEOUT_POLL_COUNT is explicitly defined to 1 if the
       architecture provides a waiting implementation of cpu_poll_relax()
       (defines CPU_POLL_RELAX_WAITS).

    - patch-3, "arm64/delay: move, fixup usecs_to_cycles()"
      - convert the 32-bit arithmetic to use mul_u64_u64_shr() to avoid
        truncating large delay values.

      - make usecs_to_cycles(), nsecs_to_cycles() static inlines.

    - patch-6, "asm-generic: barrier: Add smp_cond_load_acquire_timeout()"
      - provide full acquire semantics (via smp_load_acquire()) in the
        failure path.

    - patch-16, "barrier: timeout validity checks for smp_cond_load_acquire_timeout()"
      - add some tests for clocks that can fail (rqspinlock like
       semantics)

  v13 [15]:
    - 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@oracle.com/
 [1] https://lore.kernel.org/lkml/20251028053136.692462-1-ankur.a.arora@oracle.com/
 [2] https://lore.kernel.org/lkml/20250911034655.3916002-1-ankur.a.arora@oracle.com/
 [3] https://lore.kernel.org/lkml/20250911034655.3916002-1-ankur.a.arora@oracle.com/
 [4] https://lore.kernel.org/lkml/20250829080735.3598416-1-ankur.a.arora@oracle.com/
 [5] https://lore.kernel.org/lkml/20250627044805.945491-1-ankur.a.arora@oracle.com/
 [6] https://lore.kernel.org/lkml/20250502085223.1316925-1-ankur.a.arora@oracle.com/
 [7] https://lore.kernel.org/lkml/20250203214911.898276-1-ankur.a.arora@oracle.com/
 [8] https://lore.kernel.org/lkml/2cecbf7fb23ee83a4ce027e1be3f46f97efd585c.camel@amazon.com/
 [9] https://lore.kernel.org/lkml/20260209023153.2661784-1-ankur.a.arora@oracle.com/
 [10] https://lore.kernel.org/lkml/20260316013651.3225328-1-ankur.a.arora@oracle.com/
 [11] https://lore.kernel.org/lkml/20230809134837.GM212435@hirez.programming.kicks-ass.net/
 [12] https://lore.kernel.org/lkml/c6f3c8d3f1f2e89a9dc7ae22482973b5a51b08cb.camel@amazon.com/
 [13] https://lore.kernel.org/all/20260408122538.3610871-1-ankur.a.arora@oracle.com/#r
 [14] https://lore.kernel.org/all/20260608080440.127491-1-ankur.a.arora@oracle.com/
 [15] https://lore.kernel.org/all/20260702013334.140905-1-ankur.a.arora@oracle.com/
 [16] https://lore.kernel.org/all/20260714073041.40250-1-ankur.a.arora@oracle.com/
 [17] https://lore.kernel.org/all/20260831202251.305046-1-ankur.a.arora@oracle.com/
 [18] https://lore.kernel.org/all/178908146776.2725577.2613699832033292946.b4-review@b4/

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@kernel.org>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: bpf@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-pm@vger.kernel.org


Ankur Arora (16):
  asm-generic: barrier: Add smp_cond_load_relaxed_timeout()
  arm64: barrier: Support smp_cond_load_relaxed_timeout()
  arm64/delay: move, fixup usecs_to_cycles()
  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: timeout validity checks for smp_cond_load_relaxed_timeout()
  barrier: timeout validity checks for smp_cond_load_acquire_timeout()

 Documentation/atomic_t.txt           |  14 +-
 arch/arm64/include/asm/barrier.h     |  24 +++
 arch/arm64/include/asm/cmpxchg.h     |  62 ++++--
 arch/arm64/include/asm/delay-const.h |  35 ++++
 arch/arm64/include/asm/rqspinlock.h  |  85 --------
 arch/arm64/lib/delay.c               |  19 +-
 drivers/clocksource/arm_arch_timer.c |   2 +
 drivers/cpuidle/poll_state.c         |  21 +-
 drivers/soc/qcom/rpmh-rsc.c          |  12 +-
 include/asm-generic/barrier.h        | 157 ++++++++++++++
 include/linux/atomic.h               |  10 +
 include/linux/atomic/atomic-long.h   |  18 +-
 include/linux/sched/idle.h           |  29 +++
 kernel/bpf/rqspinlock.c              |  78 ++++---
 lib/Kconfig.debug                    |  10 +
 lib/tests/Makefile                   |   1 +
 lib/tests/barrier-timeout-test.c     | 297 +++++++++++++++++++++++++++
 scripts/atomic/gen-atomic-long.sh    |  16 +-
 18 files changed, 710 insertions(+), 180 deletions(-)
 create mode 100644 arch/arm64/include/asm/delay-const.h
 create mode 100644 lib/tests/barrier-timeout-test.c

-- 
2.43.7

[PATCH v16 01/16] asm-generic: barrier: Add smp_cond_load_relaxed_timeout()

[Ankur Arora]

Add smp_cond_load_relaxed_timeout(), which extends
smp_cond_load_relaxed() to allow waiting for a duration.

The interface loops around doing a periodic time-check, waiting for
the condition variable to change. Loop iterations use cpu_poll_relax()
to slow down the busy-wait and this, unless overridden by the
architecture code, amounts to a cpu_relax().

There are two ways for the time-check to fail: the timeout case or,
@time_expr_ns returning an invalid value (negative or zero). The second
failure mode allows for clocks attached to the clock-domain of
@cond_expr --  clocks which might cease to operate meaningfully once
some state internal to @cond_expr has changed -- to fail.

Evaluation of @time_expr_ns: in the fastpath we want to keep the
performance close to smp_cond_load_relaxed(). So defer evaluation
of the potentially costly @time_expr_ns to the slowpath.

This also means that there will always be some hardware dependent
duration that has passed in cpu_poll_relax() iterations at the time
of first evaluation. Additionally cpu_poll_relax() is not guaranteed
to return at timeout boundary. In sum, expect timeout overshoot when
we exit due to expiration of the timeout.

The number of spin iterations before time-check, SMP_TIMEOUT_POLL_COUNT
is chosen to be 200 by default. With a cpu_poll_relax() iteration
taking ~20-30 cycles (measured on a variety of x86 platforms), we
expect a time-check every ~4000-6000 cycles.

If a architecture provides a waiting implementation for cpu_poll_relax()
(and signifies that by defining CPU_POLL_RELAX_WAITS) we define
SMP_TIMEOUT_POLL_COUNT to 1.

Lastly, config option ARCH_HAS_CPU_RELAX indicates availability of a
cpu_poll_relax() that is cheaper than polling. Long timeout values
might not make sense for architectures not having ARCH_HAS_CPU_RELAX.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linux-arch@vger.kernel.org
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
Notes:
  - Unify the exit path out of timeout and hitting the cond_expr.
    This adds an extra (though cheap and easily predicted) check
    for __scl_timed_out in the inner loop, but allows us to simplify
    the ACQUIRE logic in smp_cond_load_acquire_timeout().
  - use u64 for internal arithmetic
  - document the timeout_ns == 0 case
  - includes <linux/time64.h> for NSEC_PER_USEC

 include/asm-generic/barrier.h | 110 ++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index b99cb57dfccc..a03e3b26384a 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -15,6 +15,7 @@
 
 #include <linux/compiler.h>
 #include <linux/kcsan-checks.h>
+#include <vdso/time64.h>
 #include <asm/rwonce.h>
 
 #ifndef nop
@@ -273,6 +274,115 @@ do {									\
 })
 #endif
 
+/*
+ * Number of times we iterate in the loop before doing the time check.
+ */
+#ifndef SMP_TIMEOUT_POLL_COUNT
+#ifdef CPU_POLL_RELAX_WAITS
+#define SMP_TIMEOUT_POLL_COUNT	1 /* Wait mode. No need to poll. */
+#else
+/*
+ * Assume that cpu_poll_relax() provides a small blip in the pipeline.
+ * Combine a reasonable number of cpu_poll_relax() instances before
+ * doing anything substantial like a time-check.
+ * Note that this assumes that the rest of the loop (largely evaluation
+ * of the loop condition is relatively cheap.)
+ */
+#define SMP_TIMEOUT_POLL_COUNT		200
+#endif
+#endif
+
+/*
+ * cpu_poll_relax() stitches up two kinds of primitives: ones that provide
+ * a momentary blip in the pipeline (ex. cpu_relax() on x86), or ones that
+ * support waiting for @ptr value to change, coupled with a precise (or not)
+ * timeout.
+ *
+ * We keep both together because the objective is to minimize expensive
+ * operations while polling on @ptr waiting for it to change. Either
+ * version allows for that.
+ * The arguments (@ptr, @val, @timeout_ns) are only needed for waiting
+ * implementations.
+ *
+ * Note that platforms with a suitable cpu_poll_relax() implementation are
+ * expected to define ARCH_HAS_CPU_RELAX.
+ */
+#ifndef cpu_poll_relax
+#define cpu_poll_relax(ptr, val, timeout_ns)	cpu_relax()
+#endif
+
+/**
+ * smp_cond_load_relaxed_timeout() - (Spin) wait for cond with no ordering
+ * guarantees until a timeout expires.
+ * @ptr: pointer to the variable to wait on.
+ * @cond_expr: boolean expression to wait for.
+ * @time_expr_ns: expression that evaluates to monotonic time (in ns) or,
+ *  on failure, returns zero or a negative value.
+ * @timeout_ns: timeout value in ns. A timeout value of zero is treated
+ *  as the minimum wait so we evaluate cond_expr at least once.
+ * Both of the above are expected to be compatible with s64; the signed
+ * value is used to handle the failure case in @time_expr_ns.
+ *
+ * Equivalent to using READ_ONCE() on the condition variable.
+ *
+ * Callers that expect to wait for prolonged durations might want
+ * to take into account the availability of ARCH_HAS_CPU_RELAX.
+ *
+ * Note that @ptr is expected to point to a memory address. Using this
+ * interface with MMIO will be slower (since SMP_TIMEOUT_POLL_COUNT is
+ * tuned for memory) and might also break in interesting architecture
+ * dependent ways.
+ */
+#ifndef smp_cond_load_relaxed_timeout
+#define __smp_cond_load_relaxed_timeout(ptr, cond_expr,			\
+				      time_expr_ns, timeout_ns)		\
+({									\
+	typeof(ptr) __PTR = (ptr);					\
+	__unqual_scalar_typeof(*(ptr)) VAL;				\
+	u32 __scl_count = 0, __scl_spin = SMP_TIMEOUT_POLL_COUNT;	\
+	u64 __scl_timeout = NSEC_PER_USEC, __scl_time_end = 0;		\
+	s64 __scl_time_now;						\
+	bool __scl_timed_out = false;					\
+									\
+	for (;;) {							\
+		VAL = READ_ONCE(*__PTR);				\
+		if (cond_expr || __scl_timed_out)			\
+			break;						\
+		cpu_poll_relax(__PTR, VAL, (u64)__scl_timeout);		\
+		if (++__scl_count < __scl_spin)				\
+			continue;					\
+		__scl_time_now = (s64)(time_expr_ns);			\
+		if (unlikely(__scl_time_end == 0)) {			\
+			__scl_timeout = (timeout_ns);			\
+			__scl_time_end = __scl_time_now + __scl_timeout;\
+		}							\
+		if (__scl_time_now <= 0 ||				\
+		    __scl_time_end <= __scl_time_now) {			\
+			__scl_timed_out = true;				\
+			continue;					\
+		}							\
+		__scl_count = 0;					\
+		__scl_timeout = __scl_time_end - __scl_time_now;	\
+	}								\
+	(typeof(*(ptr)))VAL;						\
+})
+
+#define smp_cond_load_relaxed_timeout(ptr, cond_expr,			\
+				      time_expr_ns, timeout_ns)		\
+({									\
+	s64 __scl_timeout_ns = (s64)(timeout_ns);			\
+	__unqual_scalar_typeof(*(ptr)) VAL;				\
+									\
+	if (unlikely(__scl_timeout_ns < 0))				\
+		VAL = READ_ONCE(*(ptr));				\
+	else								\
+		VAL = __smp_cond_load_relaxed_timeout(ptr, cond_expr,	\
+						      time_expr_ns,	\
+						      __scl_timeout_ns);\
+	(typeof(*(ptr)))VAL;						\
+})
+#endif
+
 /*
  * pmem_wmb() ensures that all stores for which the modification
  * are written to persistent storage by preceding instructions have
-- 
2.43.7

[PATCH v16 02/16] arm64: barrier: Support smp_cond_load_relaxed_timeout()

[Ankur Arora]

Support waiting in smp_cond_load_relaxed_timeout() via
__cmpwait_relaxed(). To ensure that we wake from waiting in WFE
periodically and don't block forever if there are no stores to
ptr, this path is only used when the event-stream is enabled.

Note that when using __cmpwait_relaxed() we ignore the timeout
value, allowing an overshoot by up to the event-stream period.
And, in the unlikely event that the event-stream is unavailable,
fallback to spin-waiting.

Define CPU_POLL_RELAX_WAITS to state that we have a waiting
implementation. (Non-production environments might not have
arch_timer_evtstrm_available() but we don't care about that
configuration.)

Note that with this we have enough to define ARCH_HAS_CPU_RELAX to
indicate that we support an optimized implementation of
cpu_poll_relax(). However, defer defining ARCH_HAS_CPU_RELAX as that
enables polling based C-state handling, which really needs
TIF_POLLING_NRFLAG.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Suggested-by: Will Deacon <will@kernel.org>
Acked-by: Will Deacon <will@kernel.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 arch/arm64/include/asm/barrier.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 9495c4441a46..e2263c754a55 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -12,6 +12,7 @@
 #include <linux/kasan-checks.h>
 
 #include <asm/alternative-macros.h>
+#include <asm/vdso/processor.h>
 
 #define __nops(n)	".rept	" #n "\nnop\n.endr\n"
 #define nops(n)		asm volatile(__nops(n))
@@ -219,6 +220,27 @@ do {									\
 	(typeof(*ptr))VAL;						\
 })
 
+/* Re-declared here to avoid include dependency. */
+extern bool arch_timer_evtstrm_available(void);
+
+/*
+ * In the common case, cpu_poll_relax() sits waiting in __cmpwait_relaxed()
+ * for @ptr value to change.
+ *
+ * State this by defining CPU_POLL_RELAX_WAITS. This enables a time-check
+ * optimization in smp_cond_load_{relaxed,acquire}_timeout(). Note that this
+ * is not ideal for the cpu_relax() fallback but that's not a production
+ * configuration.
+ */
+#define CPU_POLL_RELAX_WAITS
+
+#define cpu_poll_relax(ptr, val, timeout_ns) do {			\
+	if (arch_timer_evtstrm_available())				\
+		__cmpwait_relaxed(ptr, val);				\
+	else								\
+		cpu_relax();						\
+} while (0)
+
 #include <asm-generic/barrier.h>
 
 #endif	/* __ASSEMBLER__ */
-- 
2.43.7

[PATCH v16 03/16] arm64/delay: move, fixup usecs_to_cycles()

[Ankur Arora]

Update xloops_to_cycles() to use mul_u64_u64_shr() instead of 32-bit fixed point
arithmetic to avoid truncation for delay values larger than ~10s
This adds an extra mul operation but is only used from delay loops so
shouldn't matter too much.

Also move some related constants and functions related to the cycles
computation out to a new header (converting some from macros to static
inlines). And finally make __delay_cycles() available outside of
arch/arm64/lib/delay.c.

Rename some macros in qcom/rpmh-rsc.c which were occupying the same
namespace.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konradybcio@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Reviewed-by: Christoph Lameter <cl@linux.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
Notes:
  - Make USECS_TO_CYCLES() into a static inline rpmh_usecs_to_cycles()

 arch/arm64/include/asm/delay-const.h | 35 ++++++++++++++++++++++++++++
 arch/arm64/lib/delay.c               | 17 ++++----------
 drivers/soc/qcom/rpmh-rsc.c          | 12 ++++++----
 3 files changed, 47 insertions(+), 17 deletions(-)
 create mode 100644 arch/arm64/include/asm/delay-const.h

diff --git a/arch/arm64/include/asm/delay-const.h b/arch/arm64/include/asm/delay-const.h
new file mode 100644
index 000000000000..6ccdfe0a4130
--- /dev/null
+++ b/arch/arm64/include/asm/delay-const.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_DELAY_CONST_H
+#define _ASM_DELAY_CONST_H
+
+#include <linux/math64.h>
+#include <linux/types.h>
+#include <asm/param.h>	/* For HZ */
+
+/* 2**32 / 1000000 (rounded up) */
+#define __usecs_to_xloops_mult	0x10C7UL
+
+/* 2**32 / 1000000000 (rounded up) */
+#define __nsecs_to_xloops_mult	0x5UL
+
+extern unsigned long loops_per_jiffy;
+static inline u64 xloops_to_cycles(u64 xloops)
+{
+	u64 loops_per_sec = (u64)loops_per_jiffy * HZ;
+
+	return mul_u64_u64_shr(xloops, loops_per_sec, 32);
+}
+
+static inline u64 usecs_to_cycles(u64 time_usecs)
+{
+	return xloops_to_cycles(time_usecs * __usecs_to_xloops_mult);
+}
+
+static inline u64 nsecs_to_cycles(u64 time_nsecs)
+{
+	return xloops_to_cycles((time_nsecs) * __nsecs_to_xloops_mult);
+}
+
+u64 notrace __delay_cycles(void);
+
+#endif	/* _ASM_DELAY_CONST_H */
diff --git a/arch/arm64/lib/delay.c b/arch/arm64/lib/delay.c
index e278e060e78a..f08eacef2f0a 100644
--- a/arch/arm64/lib/delay.c
+++ b/arch/arm64/lib/delay.c
@@ -12,17 +12,10 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/timex.h>
+#include <asm/delay-const.h>
 
 #include <clocksource/arm_arch_timer.h>
 
-#define USECS_TO_CYCLES(time_usecs)			\
-	xloops_to_cycles((time_usecs) * 0x10C7UL)
-
-static inline unsigned long xloops_to_cycles(unsigned long xloops)
-{
-	return (xloops * loops_per_jiffy * HZ) >> 32;
-}
-
 /*
  * Force the use of CNTVCT_EL0 in order to have the same base as WFxT.
  * This avoids some annoying issues when CNTVOFF_EL2 is not reset 0 on a
@@ -32,7 +25,7 @@ static inline unsigned long xloops_to_cycles(unsigned long xloops)
  * Note that userspace cannot change the offset behind our back either,
  * as the vcpu mutex is held as long as KVM_RUN is in progress.
  */
-static cycles_t notrace __delay_cycles(void)
+u64 notrace __delay_cycles(void)
 {
 	guard(preempt_notrace)();
 	return __arch_counter_get_cntvct_stable();
@@ -54,7 +47,7 @@ void __delay(unsigned long cycles)
 			wfet(end);
 	} else 	if (arch_timer_evtstrm_available()) {
 		const cycles_t timer_evt_period =
-			USECS_TO_CYCLES(ARCH_TIMER_EVT_STREAM_PERIOD_US);
+			usecs_to_cycles(ARCH_TIMER_EVT_STREAM_PERIOD_US);
 
 		while ((__delay_cycles() - start + timer_evt_period) < cycles)
 			wfe();
@@ -73,12 +66,12 @@ EXPORT_SYMBOL(__const_udelay);
 
 void __udelay(unsigned long usecs)
 {
-	__const_udelay(usecs * 0x10C7UL); /* 2**32 / 1000000 (rounded up) */
+	__const_udelay(usecs * __usecs_to_xloops_mult);
 }
 EXPORT_SYMBOL(__udelay);
 
 void __ndelay(unsigned long nsecs)
 {
-	__const_udelay(nsecs * 0x5UL); /* 2**32 / 1000000000 (rounded up) */
+	__const_udelay(nsecs * __nsecs_to_xloops_mult);
 }
 EXPORT_SYMBOL(__ndelay);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index f881af35ecfc..dabe0ccba370 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -146,14 +146,16 @@ enum {
  *  +---------------------------------------------------+
  */
 
-#define USECS_TO_CYCLES(time_usecs)			\
-	xloops_to_cycles((time_usecs) * 0x10C7UL)
-
-static inline unsigned long xloops_to_cycles(u64 xloops)
+static inline u64 rpmh_xloops_to_cycles(u64 xloops)
 {
 	return (xloops * loops_per_jiffy * HZ) >> 32;
 }
 
+static inline u64 rpmh_usecs_to_cycles(u64 time_usecs)
+{
+	return rpmh_xloops_to_cycles((time_usecs) * 0x10C7UL);
+}
+
 static u32 rpmh_rsc_reg_offset_ver_2_7[] = {
 	[RSC_DRV_TCS_OFFSET]		= 672,
 	[RSC_DRV_CMD_OFFSET]		= 20,
@@ -828,7 +830,7 @@ void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv)
 	wakeup_us = ktime_to_us(wakeup);
 
 	/* Convert the wakeup to arch timer scale */
-	wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
+	wakeup_cycles = rpmh_usecs_to_cycles(wakeup_us);
 	wakeup_cycles += arch_timer_read_counter();
 
 exit:
-- 
2.43.7

[PATCH v16 04/16] arm64: support WFET in smp_cond_load_relaxed_timeout()

[Ankur Arora]

When using WFE we can go beyond the timeout since, lacking an event we
depend on the event-stream for wakeups when there's no event.

So, prefer WFET where available to take advantage of precise wait.

Handling of WFET is via __cmpwait_timeout() which behaves similarly
to __cmpwait() calling out to the __cmpwait_case_timeout_##sz() or
__cmpwait_case_##sz() functions.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
Notes: detail the WFE/WFET choice in the commit message.

 arch/arm64/include/asm/barrier.h |  8 +++--
 arch/arm64/include/asm/cmpxchg.h | 62 +++++++++++++++++++++++++-------
 2 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index e2263c754a55..3b024058cf2f 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -224,8 +224,8 @@ do {									\
 extern bool arch_timer_evtstrm_available(void);
 
 /*
- * In the common case, cpu_poll_relax() sits waiting in __cmpwait_relaxed()
- * for @ptr value to change.
+ * In the common case, cpu_poll_relax() sits waiting in __cmpwait_relaxed()/
+ * __cmpwait_relaxed_timeout() for the ptr value to change.
  *
  * State this by defining CPU_POLL_RELAX_WAITS. This enables a time-check
  * optimization in smp_cond_load_{relaxed,acquire}_timeout(). Note that this
@@ -235,7 +235,9 @@ extern bool arch_timer_evtstrm_available(void);
 #define CPU_POLL_RELAX_WAITS
 
 #define cpu_poll_relax(ptr, val, timeout_ns) do {			\
-	if (arch_timer_evtstrm_available())				\
+	if (alternative_has_cap_unlikely(ARM64_HAS_WFXT))		\
+		__cmpwait_relaxed_timeout(ptr, val, timeout_ns);	\
+	else if (arch_timer_evtstrm_available())			\
 		__cmpwait_relaxed(ptr, val);				\
 	else								\
 		cpu_relax();						\
diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index 6cf3cd6873f5..30dca19c9a7b 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -12,6 +12,7 @@
 
 #include <asm/barrier.h>
 #include <asm/lse.h>
+#include <asm/delay-const.h>
 
 /*
  * We need separate acquire parameters for ll/sc and lse, since the full
@@ -212,7 +213,8 @@ __CMPXCHG_GEN(_mb)
 
 #define __CMPWAIT_CASE(w, sfx, sz)					\
 static inline void __cmpwait_case_##sz(volatile void *ptr,		\
-				       unsigned long val)		\
+				       unsigned long val,		\
+				       u64 __maybe_unused timeout_ns)	\
 {									\
 	unsigned long tmp;						\
 									\
@@ -235,20 +237,52 @@ __CMPWAIT_CASE( ,  , 64);
 
 #undef __CMPWAIT_CASE
 
-#define __CMPWAIT_GEN(sfx)						\
-static __always_inline void __cmpwait##sfx(volatile void *ptr,		\
-				  unsigned long val,			\
-				  int size)				\
+#define __CMPWAIT_TIMEOUT_CASE(w, sfx, sz)				\
+static inline void __cmpwait_case_timeout_##sz(volatile void *ptr,	\
+					       unsigned long val,	\
+					       u64 timeout_ns)		\
+{									\
+	unsigned long tmp;						\
+	u64 ecycles = __delay_cycles() +				\
+			nsecs_to_cycles(timeout_ns);			\
+	asm volatile(							\
+	"	sevl\n"							\
+	"	wfe\n"							\
+	"	ldxr" #sfx "\t%" #w "[tmp], %[v]\n"			\
+	"	eor	%" #w "[tmp], %" #w "[tmp], %" #w "[val]\n"	\
+	"	cbnz	%" #w "[tmp], 1f\n"				\
+	"	msr s0_3_c1_c0_0, %[ecycles]\n"				\
+	"1:"								\
+	: [tmp] "=&r" (tmp), [v] "+Q" (*(u##sz *)ptr)			\
+	: [val] "r" (val), [ecycles] "r" (ecycles));			\
+}
+
+__CMPWAIT_TIMEOUT_CASE(w, b, 8);
+__CMPWAIT_TIMEOUT_CASE(w, h, 16);
+__CMPWAIT_TIMEOUT_CASE(w,  , 32);
+__CMPWAIT_TIMEOUT_CASE( ,  , 64);
+
+#undef __CMPWAIT_TIMEOUT_CASE
+
+#define __CMPWAIT_GEN(timeout, sfx)					\
+static __always_inline void __cmpwait##timeout##sfx(volatile void *ptr,	\
+						    unsigned long val,	\
+						    u64 timeout_ns,	\
+						    int size)		\
 {									\
 	switch (size) {							\
 	case 1:								\
-		return __cmpwait_case##sfx##_8(ptr, (u8)val);		\
+		return __cmpwait_case##timeout##sfx##_8(ptr, (u8)val,	\
+							timeout_ns);	\
 	case 2:								\
-		return __cmpwait_case##sfx##_16(ptr, (u16)val);		\
+		return __cmpwait_case##timeout##sfx##_16(ptr, (u16)val,	\
+							 timeout_ns);	\
 	case 4:								\
-		return __cmpwait_case##sfx##_32(ptr, val);		\
+		return __cmpwait_case##timeout##sfx##_32(ptr, val,	\
+							 timeout_ns);	\
 	case 8:								\
-		return __cmpwait_case##sfx##_64(ptr, val);		\
+		return __cmpwait_case##timeout##sfx##_64(ptr, val,	\
+							 timeout_ns);	\
 	default:							\
 		BUILD_BUG();						\
 	}								\
@@ -256,11 +290,15 @@ static __always_inline void __cmpwait##sfx(volatile void *ptr,		\
 	unreachable();							\
 }
 
-__CMPWAIT_GEN()
+__CMPWAIT_GEN(        , )
+__CMPWAIT_GEN(_timeout, )
 
 #undef __CMPWAIT_GEN
 
-#define __cmpwait_relaxed(ptr, val) \
-	__cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr)))
+#define __cmpwait_relaxed_timeout(ptr, val, timeout_ns)			\
+	__cmpwait_timeout((ptr), (unsigned long)(val), timeout_ns, sizeof(*(ptr)))
+
+#define __cmpwait_relaxed(ptr, val)					\
+	__cmpwait((ptr), (unsigned long)(val), 0, sizeof(*(ptr)))
 
 #endif	/* __ASM_CMPXCHG_H */
-- 
2.43.7

[PATCH v16 05/16] arm64: rqspinlock: Remove private copy of smp_cond_load_acquire_timewait()

[Ankur Arora]

In preparation for defining smp_cond_load_acquire_timeout(), remove
the private copy. Lacking this, the rqspinlock code falls back to using
smp_cond_load_acquire().

Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: bpf@vger.kernel.org
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Haris Okanovic <harisokn@amazon.com>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Reviewed-by: Christoph Lameter (Ampere) <cl@gentwo.org>
Tested-by: Haris Okanovic <harisokn@amazon.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 arch/arm64/include/asm/rqspinlock.h | 85 -----------------------------
 1 file changed, 85 deletions(-)

diff --git a/arch/arm64/include/asm/rqspinlock.h b/arch/arm64/include/asm/rqspinlock.h
index 9ea0a74e5892..a385603436e9 100644
--- a/arch/arm64/include/asm/rqspinlock.h
+++ b/arch/arm64/include/asm/rqspinlock.h
@@ -3,91 +3,6 @@
 #define _ASM_RQSPINLOCK_H
 
 #include <asm/barrier.h>
-
-/*
- * Hardcode res_smp_cond_load_acquire implementations for arm64 to a custom
- * version based on [0]. In rqspinlock code, our conditional expression involves
- * checking the value _and_ additionally a timeout. However, on arm64, the
- * WFE-based implementation may never spin again if no stores occur to the
- * locked byte in the lock word. As such, we may be stuck forever if
- * event-stream based unblocking is not available on the platform for WFE spin
- * loops (arch_timer_evtstrm_available).
- *
- * Once support for smp_cond_load_acquire_timewait [0] lands, we can drop this
- * copy-paste.
- *
- * While we rely on the implementation to amortize the cost of sampling
- * cond_expr for us, it will not happen when event stream support is
- * unavailable, time_expr check is amortized. This is not the common case, and
- * it would be difficult to fit our logic in the time_expr_ns >= time_limit_ns
- * comparison, hence just let it be. In case of event-stream, the loop is woken
- * up at microsecond granularity.
- *
- * [0]: https://lore.kernel.org/lkml/20250203214911.898276-1-ankur.a.arora@oracle.com
- */
-
-#ifndef smp_cond_load_acquire_timewait
-
-#define smp_cond_time_check_count	200
-
-#define __smp_cond_load_relaxed_spinwait(ptr, cond_expr, time_expr_ns,	\
-					 time_limit_ns) ({		\
-	typeof(ptr) __PTR = (ptr);					\
-	__unqual_scalar_typeof(*ptr) VAL;				\
-	unsigned int __count = 0;					\
-	for (;;) {							\
-		VAL = READ_ONCE(*__PTR);				\
-		if (cond_expr)						\
-			break;						\
-		cpu_relax();						\
-		if (__count++ < smp_cond_time_check_count)		\
-			continue;					\
-		if ((time_expr_ns) >= (time_limit_ns))			\
-			break;						\
-		__count = 0;						\
-	}								\
-	(typeof(*ptr))VAL;						\
-})
-
-#define __smp_cond_load_acquire_timewait(ptr, cond_expr,		\
-					 time_expr_ns, time_limit_ns)	\
-({									\
-	typeof(ptr) __PTR = (ptr);					\
-	__unqual_scalar_typeof(*ptr) VAL;				\
-	for (;;) {							\
-		VAL = smp_load_acquire(__PTR);				\
-		if (cond_expr)						\
-			break;						\
-		__cmpwait_relaxed(__PTR, VAL);				\
-		if ((time_expr_ns) >= (time_limit_ns))			\
-			break;						\
-	}								\
-	(typeof(*ptr))VAL;						\
-})
-
-#define smp_cond_load_acquire_timewait(ptr, cond_expr,			\
-				      time_expr_ns, time_limit_ns)	\
-({									\
-	__unqual_scalar_typeof(*ptr) _val;				\
-	int __wfe = arch_timer_evtstrm_available();			\
-									\
-	if (likely(__wfe)) {						\
-		_val = __smp_cond_load_acquire_timewait(ptr, cond_expr,	\
-							time_expr_ns,	\
-							time_limit_ns);	\
-	} else {							\
-		_val = __smp_cond_load_relaxed_spinwait(ptr, cond_expr,	\
-							time_expr_ns,	\
-							time_limit_ns);	\
-		smp_acquire__after_ctrl_dep();				\
-	}								\
-	(typeof(*ptr))_val;						\
-})
-
-#endif
-
-#define res_smp_cond_load_acquire(v, c) smp_cond_load_acquire_timewait(v, c, 0, 1)
-
 #include <asm-generic/rqspinlock.h>
 
 #endif /* _ASM_RQSPINLOCK_H */
-- 
2.43.7

[PATCH v16 06/16] asm-generic: barrier: Add smp_cond_load_acquire_timeout()

[Ankur Arora]

Add the acquire variant of smp_cond_load_relaxed_timeout().

smp_cond_load_acquire_timeout() does its own error checking but
reuses the relaxed logic in __smp_cond_load_relaxed_timeout() for
the actual wait.

Since __smp_cond_load_relaxed_timeout() already provides LOAD->STORE
ordering, we only provide the additional LOAD->LOAD ordering in
the common case.

And providing full ACQUIRE in the error case.

Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linux-arch@vger.kernel.org
Cc: bpf@vger.kernel.org
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Haris Okanovic <harisokn@amazon.com>
Tested-by: Haris Okanovic <harisokn@amazon.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
Notes:
    With a small change in __smp_cond_load_relaxed_timeout(), it
    now provides LOAD->STORE ordering. (Earlier versions provided
    that only for the non-timeout path.)

    This simplifies providing the ACQUIRE ordering because now we
    don't need special handling for the timeout path.

    For the error path (__scl_timeout_ns < 0) where we just use
    smp_load_acquire().

    In the non-error path we use the control dependency to provide the
    LOAD->STORE ordering with smp_acquire__after_ctrl_dep() providing
    the remaining LOAD->LOAD.

    Unfortunately I couldn't see a way of avoiding the goto label 
    because in this more natural variant the control dependency
    on line 6:

  1	if (unlikely(__scl_timeout_ns < 0)) {
  2		VAL = smp_load_acquire(ptr);
  3	} else {
  4		for (;;) {
  5		   VAL = READ_ONCE(*ptr);
  6		   if (cond_expr || __scl_timed_out)
  7 			break;
  8		   ...
  9		}
 10		smp_acquire__after_ctrl_dep();
 11	}
 12    ...
 

    likely runs afoul of this bit in memory-barriers.txt:
     "control dependencies apply only to the stores in the
      then-clause and else-clause of the if-statement in question
      (including functions invoked by those two clauses), not to
      code following that if-statement."

    IIUC if there was a store on line 12, that would not be ordered
    wrt the control dependency on line 6 and a weakly ordered CPU could
    speculatively execute that earlier especially if the timeout was
    extremely short.
    (An earlier version [1] had a similar problem.)

    So this change disentangles the conditional on line 1 executing
    the if and then clauses sequentially.

[1] https://lore.kernel.org/lkml/20260831202251.305046-7-ankur.a.arora@oracle.com/

 include/asm-generic/barrier.h | 47 +++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index a03e3b26384a..7a59bee1e693 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -383,6 +383,53 @@ do {									\
 })
 #endif
 
+/**
+ * smp_cond_load_acquire_timeout() - (Spin) wait for cond with ACQUIRE ordering
+ * until a timeout expires.
+ * @ptr: pointer to the variable to wait on.
+ * @cond_expr: boolean expression to wait for.
+ * @time_expr_ns: monotonic expression that evaluates to time in ns or,
+ *  on failure, returns a negative value.
+ * @timeout_ns: timeout value in ns. A timeout value of zero is treated
+ *  as the minimum wait so we evaluate cond_expr at least once.
+ * (Both of the above are assumed to be compatible with s64.)
+ *
+ * Equivalent to using smp_cond_load_acquire() on the condition variable with
+ * a timeout.
+ */
+#ifndef smp_cond_load_acquire_timeout
+#define smp_cond_load_acquire_timeout(ptr, cond_expr,			\
+				      time_expr_ns, timeout_ns)		\
+({									\
+	__label__ __scl_out;						\
+	s64 __scl_timeout_ns = (s64)(timeout_ns);			\
+	__unqual_scalar_typeof(*(ptr)) VAL;				\
+									\
+	if (unlikely(__scl_timeout_ns < 0)) {				\
+		VAL = smp_load_acquire(ptr);				\
+		goto __scl_out;						\
+	}								\
+									\
+	VAL = __smp_cond_load_relaxed_timeout(ptr,			\
+					      cond_expr,		\
+					      time_expr_ns,		\
+					      __scl_timeout_ns);	\
+	/*								\
+	 * We return having waited on the control dependency,		\
+	 * (cond_expr || __scl_timed_out). Since we break out only	\
+	 * after either of these conditions are true, the conditional	\
+	 * separates the execution stream into two happened-before	\
+	 * and happened-after sequences. And since STOREs are not	\
+	 * speculated, this takes cares of the LOAD->STORE ordering.	\
+	 *								\
+	 * Provide the remaining LOAD->LOAD ordering.			\
+	 */								\
+	smp_acquire__after_ctrl_dep();					\
+__scl_out:								\
+	(typeof(*(ptr)))VAL;						\
+})
+#endif
+
 /*
  * pmem_wmb() ensures that all stores for which the modification
  * are written to persistent storage by preceding instructions have
-- 
2.43.7

[PATCH v16 07/16] atomic: Add atomic_cond_read_*_timeout()

[Ankur Arora]

Add atomic load wrappers, atomic_cond_read_*_timeout() and
atomic64_cond_read_*_timeout() for the cond-load timeout interfaces.

Also add a short description for the atomic_cond_read_{relaxed,acquire}(),
and the atomic_cond_read_{relaxed,acquire}_timeout() interfaces.

Cc: Will Deacon <will@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 Documentation/atomic_t.txt | 14 +++++++++-----
 include/linux/atomic.h     | 10 ++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/atomic_t.txt b/Documentation/atomic_t.txt
index bee3b1bca9a7..0e53f6ccb558 100644
--- a/Documentation/atomic_t.txt
+++ b/Documentation/atomic_t.txt
@@ -16,6 +16,10 @@ Non-RMW ops:
   atomic_read(), atomic_set()
   atomic_read_acquire(), atomic_set_release()
 
+Non-RMW, non-atomic_t ops:
+
+  atomic_cond_read_{relaxed,acquire}()
+  atomic_cond_read_{relaxed,acquire}_timeout()
 
 RMW atomic operations:
 
@@ -79,11 +83,11 @@ SEMANTICS
 
 Non-RMW ops:
 
-The non-RMW ops are (typically) regular LOADs and STOREs and are canonically
-implemented using READ_ONCE(), WRITE_ONCE(), smp_load_acquire() and
-smp_store_release() respectively. Therefore, if you find yourself only using
-the Non-RMW operations of atomic_t, you do not in fact need atomic_t at all
-and are doing it wrong.
+The non-RMW ops are (typically) regular, or conditional LOADs and STOREs and
+are canonically implemented using READ_ONCE(), WRITE_ONCE(),
+smp_load_acquire() and smp_store_release() respectively. Therefore, if you
+find yourself only using the Non-RMW operations of atomic_t, you do not in
+fact need atomic_t at all and are doing it wrong.
 
 A note for the implementation of atomic_set{}() is that it must not break the
 atomicity of the RMW ops. That is:
diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 8dd57c3a99e9..5bcb86e07784 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -31,6 +31,16 @@
 #define atomic64_cond_read_acquire(v, c) smp_cond_load_acquire(&(v)->counter, (c))
 #define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
 
+#define atomic_cond_read_acquire_timeout(v, c, e, t) \
+	smp_cond_load_acquire_timeout(&(v)->counter, (c), (e), (t))
+#define atomic_cond_read_relaxed_timeout(v, c, e, t) \
+	smp_cond_load_relaxed_timeout(&(v)->counter, (c), (e), (t))
+
+#define atomic64_cond_read_acquire_timeout(v, c, e, t) \
+	smp_cond_load_acquire_timeout(&(v)->counter, (c), (e), (t))
+#define atomic64_cond_read_relaxed_timeout(v, c, e, t) \
+	smp_cond_load_relaxed_timeout(&(v)->counter, (c), (e), (t))
+
 /*
  * The idea here is to build acquire/release variants by adding explicit
  * barriers on top of the relaxed variant. In the case where the relaxed
-- 
2.43.7

[PATCH v16 08/16] locking/atomic: scripts: build atomic_long_cond_read_*_timeout()

[Ankur Arora]

Add the atomic long wrappers for the cond-load timeout interfaces.

Cc: Will Deacon <will@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 include/linux/atomic/atomic-long.h | 18 +++++++++++-------
 scripts/atomic/gen-atomic-long.sh  | 16 ++++++++++------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/include/linux/atomic/atomic-long.h b/include/linux/atomic/atomic-long.h
index 6a4e47d2db35..553b6b0e0258 100644
--- a/include/linux/atomic/atomic-long.h
+++ b/include/linux/atomic/atomic-long.h
@@ -11,14 +11,18 @@
 
 #ifdef CONFIG_64BIT
 typedef atomic64_t atomic_long_t;
-#define ATOMIC_LONG_INIT(i)		ATOMIC64_INIT(i)
-#define atomic_long_cond_read_acquire	atomic64_cond_read_acquire
-#define atomic_long_cond_read_relaxed	atomic64_cond_read_relaxed
+#define ATOMIC_LONG_INIT(i)			ATOMIC64_INIT(i)
+#define atomic_long_cond_read_acquire		atomic64_cond_read_acquire
+#define atomic_long_cond_read_relaxed		atomic64_cond_read_relaxed
+#define atomic_long_cond_read_acquire_timeout	atomic64_cond_read_acquire_timeout
+#define atomic_long_cond_read_relaxed_timeout	atomic64_cond_read_relaxed_timeout
 #else
 typedef atomic_t atomic_long_t;
-#define ATOMIC_LONG_INIT(i)		ATOMIC_INIT(i)
-#define atomic_long_cond_read_acquire	atomic_cond_read_acquire
-#define atomic_long_cond_read_relaxed	atomic_cond_read_relaxed
+#define ATOMIC_LONG_INIT(i)			ATOMIC_INIT(i)
+#define atomic_long_cond_read_acquire		atomic_cond_read_acquire
+#define atomic_long_cond_read_relaxed		atomic_cond_read_relaxed
+#define atomic_long_cond_read_acquire_timeout	atomic_cond_read_acquire_timeout
+#define atomic_long_cond_read_relaxed_timeout	atomic_cond_read_relaxed_timeout
 #endif
 
 /**
@@ -1809,4 +1813,4 @@ raw_atomic_long_dec_if_positive(atomic_long_t *v)
 }
 
 #endif /* _LINUX_ATOMIC_LONG_H */
-// 4b882bf19018602c10816c52f8b4ae280adc887b
+// 79c1f4acb5774376ceed559843d5d9ed1348df99
diff --git a/scripts/atomic/gen-atomic-long.sh b/scripts/atomic/gen-atomic-long.sh
index 9826be3ba986..874643dc74bd 100755
--- a/scripts/atomic/gen-atomic-long.sh
+++ b/scripts/atomic/gen-atomic-long.sh
@@ -79,14 +79,18 @@ cat << EOF
 
 #ifdef CONFIG_64BIT
 typedef atomic64_t atomic_long_t;
-#define ATOMIC_LONG_INIT(i)		ATOMIC64_INIT(i)
-#define atomic_long_cond_read_acquire	atomic64_cond_read_acquire
-#define atomic_long_cond_read_relaxed	atomic64_cond_read_relaxed
+#define ATOMIC_LONG_INIT(i)			ATOMIC64_INIT(i)
+#define atomic_long_cond_read_acquire		atomic64_cond_read_acquire
+#define atomic_long_cond_read_relaxed		atomic64_cond_read_relaxed
+#define atomic_long_cond_read_acquire_timeout	atomic64_cond_read_acquire_timeout
+#define atomic_long_cond_read_relaxed_timeout	atomic64_cond_read_relaxed_timeout
 #else
 typedef atomic_t atomic_long_t;
-#define ATOMIC_LONG_INIT(i)		ATOMIC_INIT(i)
-#define atomic_long_cond_read_acquire	atomic_cond_read_acquire
-#define atomic_long_cond_read_relaxed	atomic_cond_read_relaxed
+#define ATOMIC_LONG_INIT(i)			ATOMIC_INIT(i)
+#define atomic_long_cond_read_acquire		atomic_cond_read_acquire
+#define atomic_long_cond_read_relaxed		atomic_cond_read_relaxed
+#define atomic_long_cond_read_acquire_timeout	atomic_cond_read_acquire_timeout
+#define atomic_long_cond_read_relaxed_timeout	atomic_cond_read_relaxed_timeout
 #endif
 
 EOF
-- 
2.43.7

[PATCH v16 09/16] bpf/rqspinlock: switch check_timeout() to a clock interface

[Ankur Arora]

check_timeout() gets the current time value and depending on how
much time has passed, checks for deadlock or times out, returning 0
or -errno on deadlock or timeout.

Switch this out to clock_deadlock(), a clock style interface, which
functions as a clock in the "lock-domain", returning the current time
until a deadlock or timeout occurs. Once a deadlock or timeout has
occurred, it stops functioning as a clock and returns error.

Also adjust the RES_CHECK_TIMEOUT macro to discard the clock value
when updating the explicit return status.

Cc: bpf@vger.kernel.org
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 kernel/bpf/rqspinlock.c | 45 +++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/kernel/bpf/rqspinlock.c b/kernel/bpf/rqspinlock.c
index 111ec80ea958..1b249c6f0674 100644
--- a/kernel/bpf/rqspinlock.c
+++ b/kernel/bpf/rqspinlock.c
@@ -196,8 +196,12 @@ static noinline int check_deadlock_ABBA(rqspinlock_t *lock, u32 mask)
 	return 0;
 }
 
-static noinline int check_timeout(rqspinlock_t *lock, u32 mask,
-				  struct rqspinlock_timeout *ts)
+/*
+ * Returns current monotonic time in ns on success or, negative errno
+ * value on failure due to timeout expiration or detection of deadlock.
+ */
+static noinline s64 clock_deadlock(rqspinlock_t *lock, u32 mask,
+				   struct rqspinlock_timeout *ts)
 {
 	u64 prev = ts->cur;
 	u64 time;
@@ -207,7 +211,7 @@ static noinline int check_timeout(rqspinlock_t *lock, u32 mask,
 			return -EDEADLK;
 		ts->cur = ktime_get_mono_fast_ns();
 		ts->timeout_end = ts->cur + ts->duration;
-		return 0;
+		return (s64)ts->cur;
 	}
 
 	time = ktime_get_mono_fast_ns();
@@ -219,11 +223,15 @@ static noinline int check_timeout(rqspinlock_t *lock, u32 mask,
 	 * checks.
 	 */
 	if (prev + NSEC_PER_MSEC < time) {
+		int ret;
 		ts->cur = time;
-		return check_deadlock_ABBA(lock, mask);
+		ret = check_deadlock_ABBA(lock, mask);
+		if (ret)
+			return ret;
+
 	}
 
-	return 0;
+	return (s64)time;
 }
 
 /*
@@ -231,15 +239,22 @@ static noinline int check_timeout(rqspinlock_t *lock, u32 mask,
  * as the macro does internal amortization for us.
  */
 #ifndef res_smp_cond_load_acquire
-#define RES_CHECK_TIMEOUT(ts, ret, mask)                              \
-	({                                                            \
-		if (!(ts).spin++)                                     \
-			(ret) = check_timeout((lock), (mask), &(ts)); \
-		(ret);                                                \
+#define RES_CHECK_TIMEOUT(ts, ret, mask)					\
+	({									\
+		s64 __timeval_err = 0;						\
+		if (!(ts).spin++)						\
+			__timeval_err = clock_deadlock((lock), (mask), &(ts));	\
+		(ret) = __timeval_err < 0 ? __timeval_err : 0;			\
+		__timeval_err;							\
 	})
 #else
-#define RES_CHECK_TIMEOUT(ts, ret, mask)			      \
-	({ (ret) = check_timeout((lock), (mask), &(ts)); })
+#define RES_CHECK_TIMEOUT(ts, ret, mask)					\
+	({									\
+		s64 __timeval_err;						\
+		__timeval_err = clock_deadlock((lock), (mask), &(ts));		\
+		(ret) = __timeval_err < 0 ? __timeval_err : 0;			\
+		__timeval_err;							\
+	})
 #endif
 
 /*
@@ -281,7 +296,7 @@ int __lockfunc resilient_tas_spin_lock(rqspinlock_t *lock)
 	val = atomic_read(&lock->val);
 
 	if (val || !atomic_try_cmpxchg(&lock->val, &val, 1)) {
-		if (RES_CHECK_TIMEOUT(ts, ret, ~0u))
+		if (RES_CHECK_TIMEOUT(ts, ret, ~0u) < 0)
 			goto out;
 		cpu_relax();
 		goto retry;
@@ -406,7 +421,7 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
 	 */
 	if (val & _Q_LOCKED_MASK) {
 		RES_RESET_TIMEOUT(ts, RES_DEF_TIMEOUT);
-		res_smp_cond_load_acquire(&lock->locked, !VAL || RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_MASK));
+		res_smp_cond_load_acquire(&lock->locked, !VAL || RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_MASK) < 0);
 	}
 
 	if (ret) {
@@ -568,7 +583,7 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
 	 */
 	RES_RESET_TIMEOUT(ts, RES_DEF_TIMEOUT * 2);
 	val = res_atomic_cond_read_acquire(&lock->val, !(VAL & _Q_LOCKED_PENDING_MASK) ||
-					   RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_PENDING_MASK));
+					   RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_PENDING_MASK) < 0);
 
 	/* Disable queue destruction when we detect deadlocks. */
 	if (ret == -EDEADLK) {
-- 
2.43.7

[PATCH v16 10/16] bpf/rqspinlock: Use smp_cond_load_acquire_timeout()

[Ankur Arora]

Switch out the conditional load interfaces used by rqspinlock
to smp_cond_load_acquire_timeout() and its wrapper,
atomic_cond_read_acquire_timeout().

Both these handle the timeout and amortize as needed, so use the
non-amortized RES_CHECK_TIMEOUT.

RES_CHECK_TIMEOUT does double duty here -- presenting the current
clock value, the timeout/deadlock error from clock_deadlock() to
the cond-load and, returning the error value via ret.

For correctness, we need to ensure that the error case of the
cond-load interface always agrees with that in clock_deadlock().

For the most part, this is fine because there's no independent clock,
or double reads from the clock in cond-load -- either of which could
lead to its internal state going out of sync from that of
clock_deadlock().

There is, however, an edge case where clock_deadlock() checks for:

        if (time > ts->timeout_end)
                return -ETIMEDOUT;

while smp_cond_load_acquire_timeout() checks for:

	if (__scl_time_now <= 0 ||
	    __scl_time_end <= __scl_time_now) {
		__scl_timed_out = true;

		...

This runs into a problem when (__scl_time_end == __scl_time_now),
which clock_deadlock() does not treat as a timeout condition but the
second clause in the conditional above does. So, make clock_deadlock()
check for equality as well.

Finally, redefine SMP_TIMEOUT_POLL_COUNT to be 16k to be similar to
the spin-count used in the amortized version. We only do this for
architectures without a waiting implementation of cpu_poll_relax().

Cc: bpf@vger.kernel.org
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
  - minor rewording in commit message.

 kernel/bpf/rqspinlock.c | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/kernel/bpf/rqspinlock.c b/kernel/bpf/rqspinlock.c
index 1b249c6f0674..9e8f19afd7b0 100644
--- a/kernel/bpf/rqspinlock.c
+++ b/kernel/bpf/rqspinlock.c
@@ -215,7 +215,7 @@ static noinline s64 clock_deadlock(rqspinlock_t *lock, u32 mask,
 	}
 
 	time = ktime_get_mono_fast_ns();
-	if (time > ts->timeout_end)
+	if (time >= ts->timeout_end)
 		return -ETIMEDOUT;
 
 	/*
@@ -235,11 +235,10 @@ static noinline s64 clock_deadlock(rqspinlock_t *lock, u32 mask,
 }
 
 /*
- * Do not amortize with spins when res_smp_cond_load_acquire is defined,
- * as the macro does internal amortization for us.
+ * Spin amortized version of RES_CHECK_TIMEOUT. Used when busy-waiting in
+ * atomic_try_cmpxchg().
  */
-#ifndef res_smp_cond_load_acquire
-#define RES_CHECK_TIMEOUT(ts, ret, mask)					\
+#define RES_CHECK_TIMEOUT_AMORTIZED(ts, ret, mask)				\
 	({									\
 		s64 __timeval_err = 0;						\
 		if (!(ts).spin++)						\
@@ -247,7 +246,7 @@ static noinline s64 clock_deadlock(rqspinlock_t *lock, u32 mask,
 		(ret) = __timeval_err < 0 ? __timeval_err : 0;			\
 		__timeval_err;							\
 	})
-#else
+
 #define RES_CHECK_TIMEOUT(ts, ret, mask)					\
 	({									\
 		s64 __timeval_err;						\
@@ -255,7 +254,6 @@ static noinline s64 clock_deadlock(rqspinlock_t *lock, u32 mask,
 		(ret) = __timeval_err < 0 ? __timeval_err : 0;			\
 		__timeval_err;							\
 	})
-#endif
 
 /*
  * Initialize the 'spin' member.
@@ -269,6 +267,18 @@ static noinline s64 clock_deadlock(rqspinlock_t *lock, u32 mask,
  */
 #define RES_RESET_TIMEOUT(ts, _duration) ({ (ts).timeout_end = 0; (ts).duration = _duration; })
 
+/*
+ * Limit how often we invoke clock_deadlock() while spin-waiting in
+ * smp_cond_load_acquire_timeout() or atomic_cond_read_acquire_timeout().
+ *
+ * We only override the default value for architectures without a waiting
+ * implementation.
+ */
+#ifndef CPU_POLL_RELAX_WAITS
+#undef SMP_TIMEOUT_POLL_COUNT
+#define SMP_TIMEOUT_POLL_COUNT	(16*1024)
+#endif
+
 /*
  * Provide a test-and-set fallback for cases when queued spin lock support is
  * absent from the architecture.
@@ -296,7 +306,7 @@ int __lockfunc resilient_tas_spin_lock(rqspinlock_t *lock)
 	val = atomic_read(&lock->val);
 
 	if (val || !atomic_try_cmpxchg(&lock->val, &val, 1)) {
-		if (RES_CHECK_TIMEOUT(ts, ret, ~0u) < 0)
+		if (RES_CHECK_TIMEOUT_AMORTIZED(ts, ret, ~0u) < 0)
 			goto out;
 		cpu_relax();
 		goto retry;
@@ -319,12 +329,6 @@ EXPORT_SYMBOL_GPL(resilient_tas_spin_lock);
  */
 static DEFINE_PER_CPU_ALIGNED(struct qnode, rqnodes[_Q_MAX_NODES]);
 
-#ifndef res_smp_cond_load_acquire
-#define res_smp_cond_load_acquire(v, c) smp_cond_load_acquire(v, c)
-#endif
-
-#define res_atomic_cond_read_acquire(v, c) res_smp_cond_load_acquire(&(v)->counter, (c))
-
 /**
  * resilient_queued_spin_lock_slowpath - acquire the queued spinlock
  * @lock: Pointer to queued spinlock structure
@@ -421,7 +425,9 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
 	 */
 	if (val & _Q_LOCKED_MASK) {
 		RES_RESET_TIMEOUT(ts, RES_DEF_TIMEOUT);
-		res_smp_cond_load_acquire(&lock->locked, !VAL || RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_MASK) < 0);
+		smp_cond_load_acquire_timeout(&lock->locked, !VAL,
+					      RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_MASK),
+					      ts.duration);
 	}
 
 	if (ret) {
@@ -582,8 +588,9 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
 	 * us.
 	 */
 	RES_RESET_TIMEOUT(ts, RES_DEF_TIMEOUT * 2);
-	val = res_atomic_cond_read_acquire(&lock->val, !(VAL & _Q_LOCKED_PENDING_MASK) ||
-					   RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_PENDING_MASK) < 0);
+	val = atomic_cond_read_acquire_timeout(&lock->val, !(VAL & _Q_LOCKED_PENDING_MASK),
+					       RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_PENDING_MASK),
+					       ts.duration);
 
 	/* Disable queue destruction when we detect deadlocks. */
 	if (ret == -EDEADLK) {
-- 
2.43.7

[PATCH v16 11/16] sched: add need-resched timed wait interface

[Ankur Arora]

Add tif_bitset_relaxed_wait() (and tif_need_resched_relaxed_wait()
which wraps it) which takes the thread_info bit and timeout duration
as parameters and waits until the bit is set or for the expiration
of the timeout.

The wait is implemented via smp_cond_load_relaxed_timeout().

smp_cond_load_relaxed_timeout() essentially provides the pattern used
in poll_idle() where we spin in a loop waiting for the flag to change
until a timeout occurs.

tif_need_resched_relaxed_wait() allows us to abstract out the internals
of waiting, scheduler specific details etc.

Placed in linux/sched/idle.h instead of linux/thread_info.h to work
around recursive include hell.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@kernel.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 include/linux/sched/idle.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/include/linux/sched/idle.h b/include/linux/sched/idle.h
index 8465ff1f20d1..ddee9b019895 100644
--- a/include/linux/sched/idle.h
+++ b/include/linux/sched/idle.h
@@ -3,6 +3,7 @@
 #define _LINUX_SCHED_IDLE_H
 
 #include <linux/sched.h>
+#include <linux/sched/clock.h>
 
 enum cpu_idle_type {
 	__CPU_NOT_IDLE = 0,
@@ -113,4 +114,32 @@ static __always_inline void current_clr_polling(void)
 }
 #endif
 
+/*
+ * Caller needs to make sure that the thread context cannot be preempted
+ * or migrated, so current_thread_info() cannot change from under us.
+ *
+ * This also allows us to safely stay in the local_clock domain.
+ */
+static __always_inline bool tif_bitset_relaxed_wait(int tif, u64 timeout_ns)
+{
+	unsigned long flags;
+
+	flags = smp_cond_load_relaxed_timeout(&current_thread_info()->flags,
+					      (VAL & BIT(tif)),
+					      local_clock_noinstr(),
+					      timeout_ns);
+	return flags & BIT(tif);
+}
+
+/**
+ * tif_need_resched_relaxed_wait() - Wait for need-resched being set
+ * with no ordering guarantees until a timeout expires.
+ *
+ * @timeout_ns: timeout value.
+ */
+static __always_inline bool tif_need_resched_relaxed_wait(u64 timeout_ns)
+{
+	return tif_bitset_relaxed_wait(TIF_NEED_RESCHED, timeout_ns);
+}
+
 #endif /* _LINUX_SCHED_IDLE_H */
-- 
2.43.7

[PATCH v16 12/16] cpuidle/poll_state: Wait for need-resched via tif_need_resched_relaxed_wait()

[Ankur Arora]

The inner loop in poll_idle() polls over the thread_info flags,
waiting to see if the thread has TIF_NEED_RESCHED set. The loop
exits once the condition is met, or if the poll time limit has
been exceeded.

To minimize the number of instructions executed in each iteration,
the time check is rate-limited. In addition, each loop iteration
executes cpu_relax() which on certain platforms provides a hint to
the pipeline that the loop busy-waits, allowing the processor to
reduce power consumption.

Switch over to tif_need_resched_relaxed_wait() instead, since that
provides exactly that.

However, since we want to minimize power consumption in idle, building
of cpuidle/poll_state.c continues to depend on CONFIG_ARCH_HAS_CPU_RELAX
as that serves as an indicator that the platform supports an optimized
version of tif_need_resched_relaxed_wait() (via
smp_cond_load_relaxed_timeout()).

Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@kernel.org>
Cc: linux-pm@vger.kernel.org
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
Tested-by: Haris Okanovic <harisokn@amazon.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 drivers/cpuidle/poll_state.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
index c7524e4c522a..7443b3e971ba 100644
--- a/drivers/cpuidle/poll_state.c
+++ b/drivers/cpuidle/poll_state.c
@@ -6,41 +6,22 @@
 #include <linux/cpuidle.h>
 #include <linux/export.h>
 #include <linux/irqflags.h>
-#include <linux/sched.h>
-#include <linux/sched/clock.h>
 #include <linux/sched/idle.h>
 #include <linux/sprintf.h>
 #include <linux/types.h>
 
-#define POLL_IDLE_RELAX_COUNT	200
-
 static int __cpuidle poll_idle(struct cpuidle_device *dev,
 			       struct cpuidle_driver *drv, int index)
 {
-	u64 time_start;
-
-	time_start = local_clock_noinstr();
-
 	dev->poll_time_limit = false;
 
 	raw_local_irq_enable();
 	if (!current_set_polling_and_test()) {
-		unsigned int loop_count = 0;
 		u64 limit;
 
 		limit = cpuidle_poll_time(drv, dev);
 
-		while (!need_resched()) {
-			cpu_relax();
-			if (loop_count++ < POLL_IDLE_RELAX_COUNT)
-				continue;
-
-			loop_count = 0;
-			if (local_clock_noinstr() - time_start > limit) {
-				dev->poll_time_limit = true;
-				break;
-			}
-		}
+		dev->poll_time_limit = !tif_need_resched_relaxed_wait(limit);
 	}
 	raw_local_irq_disable();
 
-- 
2.43.7

[PATCH v16 13/16] arm64/delay: enable testing smp_cond_load_relaxed_timeout()

[Ankur Arora]

This enables the barrier tests to be built as a module.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 arch/arm64/lib/delay.c               | 2 ++
 drivers/clocksource/arm_arch_timer.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm64/lib/delay.c b/arch/arm64/lib/delay.c
index f08eacef2f0a..20bc2894b65b 100644
--- a/arch/arm64/lib/delay.c
+++ b/arch/arm64/lib/delay.c
@@ -12,6 +12,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/timex.h>
+#include <kunit/visibility.h>
 #include <asm/delay-const.h>
 
 #include <clocksource/arm_arch_timer.h>
@@ -30,6 +31,7 @@ u64 notrace __delay_cycles(void)
 	guard(preempt_notrace)();
 	return __arch_counter_get_cntvct_stable();
 }
+EXPORT_SYMBOL_IF_KUNIT(__delay_cycles);
 
 void __delay(unsigned long cycles)
 {
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index cc9a8b399004..227447b19b14 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -28,6 +28,7 @@
 #include <linux/acpi.h>
 #include <linux/arm-smccc.h>
 #include <linux/ptp_kvm.h>
+#include <kunit/visibility.h>
 
 #include <asm/arch_timer.h>
 #include <asm/virt.h>
@@ -893,6 +894,7 @@ bool arch_timer_evtstrm_available(void)
 	 */
 	return cpumask_test_cpu(raw_smp_processor_id(), &evtstrm_available);
 }
+EXPORT_SYMBOL_IF_KUNIT(arch_timer_evtstrm_available);
 
 static struct arch_timer_kvm_info arch_timer_kvm_info;
 
-- 
2.43.7

[PATCH v16 14/16] barrier: add tests for smp_cond_load_*_timeout()

[Ankur Arora]

Add success and failure case tests for smp_cond_load_*_timeout().

Success or failure cases depend on the expected bit being set (or not).
Additionally in failure cases smp_cond_load_*_timeout() cannot return
before timeout.

Cc: Julian Braha <julianbraha@gmail.com>
Tested-by: Haris Okanovic <harisokn@amazon.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 lib/Kconfig.debug                |  10 +++
 lib/tests/Makefile               |   1 +
 lib/tests/barrier-timeout-test.c | 129 +++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+)
 create mode 100644 lib/tests/barrier-timeout-test.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 6b24a2c89b84..672e1e920538 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2509,6 +2509,16 @@ config FFS_KUNIT_TEST
 	  For more information on KUnit and unit tests in general,
 	  please refer to Documentation/dev-tools/kunit/.
 
+config BARRIER_TIMEOUT_KUNIT_TEST
+	tristate "KUnit tests for smp_cond_load_*_timeout()" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Builds KUnit tests that validate wake-up and timeout handling paths in
+	  smp_cond_load_relaxed_timeout() and smp_cond_load_acquire_timeout().
+
+	  If unsure, say N.
+
 config TEST_KSTRTOX
 	tristate "Test kstrto*() family of functions at runtime"
 
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
index 3cac3b63a752..711302dd8ab5 100644
--- a/lib/tests/Makefile
+++ b/lib/tests/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_CHECKSUM_KUNIT) += checksum_kunit.o
 obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
 obj-$(CONFIG_CPUMASK_KUNIT_TEST) += cpumask_kunit.o
 obj-$(CONFIG_FFS_KUNIT_TEST) += ffs_kunit.o
+obj-$(CONFIG_BARRIER_TIMEOUT_KUNIT_TEST) += barrier-timeout-test.o
 CFLAGS_fortify_kunit.o += $(call cc-disable-warning, unsequenced)
 CFLAGS_fortify_kunit.o += $(call cc-disable-warning, stringop-overread)
 CFLAGS_fortify_kunit.o += $(call cc-disable-warning, stringop-truncation)
diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
new file mode 100644
index 000000000000..9c12cd36e7d2
--- /dev/null
+++ b/lib/tests/barrier-timeout-test.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests exercising smp_cond_load_{relaxed,acquire}_timeout().
+ *
+ * Copyright (c) 2026, Oracle Corp.
+ * Author: Ankur Arora <ankur.a.arora@oracle.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+#include <linux/sched/clock.h>
+#include <linux/delay.h>
+#include <asm/barrier.h>
+#include <kunit/test.h>
+#include <kunit/visibility.h>
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+
+struct clock_state {
+	s64	start_time;
+	s64	end_time;
+};
+
+#define TIMEOUT_MSEC	2
+#define TEST_FLAG_VAL	BIT(17)
+static unsigned int scu_flag;
+
+static s64 basic_clock(struct clock_state *clk)
+{
+	clk->end_time = local_clock();
+	return clk->end_time;
+}
+
+static void update_flags(void)
+{
+	WRITE_ONCE(scu_flag, TEST_FLAG_VAL);
+}
+
+static s64 mocked_clock(struct clock_state *clk)
+{
+	s64 clk_mid = clk->start_time + (TIMEOUT_MSEC * NSEC_PER_MSEC)/2;
+
+	clk->end_time = local_clock();
+	if (clk->end_time >= clk_mid)
+		update_flags();
+	return clk->end_time;
+}
+
+typedef s64 (*clkfn_t)(struct clock_state *);
+struct smp_cond_update_params {
+	clkfn_t	clock;
+	bool	acquire;
+	bool	succeeds;
+};
+
+static const struct smp_cond_update_params update_params_list[] = {
+	/* mocked-clock updates flag inline. */
+	{ .clock = &mocked_clock, .succeeds = true, .acquire = false, },
+	{ .clock = &mocked_clock, .succeeds = true, .acquire = true,  },
+
+	/* basic-clock doesn't update flag. */
+	{ .clock = &basic_clock, .succeeds = false,  .acquire = true, },
+	{ .clock = &basic_clock, .succeeds = false,  .acquire = false, },
+};
+
+static void param_to_desc(const struct smp_cond_update_params *p, char *desc)
+{
+	char *clk = NULL, *update = NULL;
+
+	if (p->clock == &mocked_clock) {
+		clk = "mocked";
+		update = "inline";
+	} else if (p->clock == &basic_clock) {
+		clk = "basic";
+		update = "none";
+	}
+
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "smp_cond_%s_timeout: clock=%s, update=%s",
+		p->acquire ? "acquire" : "relaxed", clk, update);
+}
+
+KUNIT_ARRAY_PARAM(smp_cond_update_params, update_params_list, param_to_desc);
+
+static void test_smp_cond_timeout(struct kunit *test)
+{
+	const struct smp_cond_update_params *p = test->param_value;
+	s64 runtime, timeout_ns = TIMEOUT_MSEC * NSEC_PER_MSEC;
+	struct clock_state clk;
+	unsigned int result;
+
+	scu_flag = 0;
+
+	preempt_disable();
+	clk.start_time = clk.end_time = local_clock();
+
+	if (p->acquire) {
+		result = smp_cond_load_acquire_timeout(&scu_flag,
+						       (VAL & TEST_FLAG_VAL),
+						       p->clock(&clk),
+						       timeout_ns);
+	} else {
+		result = smp_cond_load_relaxed_timeout(&scu_flag,
+						       (VAL & TEST_FLAG_VAL),
+						       p->clock(&clk),
+						       timeout_ns);
+	}
+	preempt_enable();
+
+	runtime = clk.end_time - clk.start_time;
+	KUNIT_EXPECT_EQ(test, result, scu_flag);
+	KUNIT_EXPECT_EQ(test, (bool)(result & TEST_FLAG_VAL), p->succeeds);
+	if (!p->succeeds)
+		KUNIT_EXPECT_GE(test, runtime, timeout_ns);
+}
+
+static struct kunit_case barrier_timeout_test_cases[] = {
+	KUNIT_CASE_PARAM(test_smp_cond_timeout, smp_cond_update_params_gen_params),
+	{}
+};
+
+static struct kunit_suite barrier_timeout_test_suite = {
+	.name = "smp-cond-load-*-timeout",
+	.test_cases = barrier_timeout_test_cases,
+};
+
+kunit_test_suite(barrier_timeout_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for smp_cond_load_{relaxed,acquire}_timeout()");
+MODULE_LICENSE("GPL");
-- 
2.43.7

[PATCH v16 15/16] barrier: timeout validity checks for smp_cond_load_relaxed_timeout()

[Ankur Arora]

Add timeout tests for smp_cond_load_relaxed_timeout().  These check
that the implementation returns early on invalid timeout values and
handles edge cases sanely.

Tested-by: Haris Okanovic <harisokn@amazon.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 lib/tests/barrier-timeout-test.c | 112 +++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
index 9c12cd36e7d2..88b374ceb78b 100644
--- a/lib/tests/barrier-timeout-test.c
+++ b/lib/tests/barrier-timeout-test.c
@@ -19,6 +19,8 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
 struct clock_state {
 	s64	start_time;
 	s64	end_time;
+	s64	extra;
+	u32	niters;
 };
 
 #define TIMEOUT_MSEC	2
@@ -113,8 +115,118 @@ static void test_smp_cond_timeout(struct kunit *test)
 		KUNIT_EXPECT_GE(test, runtime, timeout_ns);
 }
 
+static s64 synthetic_clock(struct clock_state *clk)
+{
+	clk->end_time += clk->extra;
+	clk->niters++;
+
+	return clk->end_time;
+}
+
+struct smp_cond_expiry_params {
+	char	*desc;
+	u64	timeout_ns;
+	s64	clk_unit;
+	s64	miniters;
+	s64	maxiters;
+};
+
+static const struct smp_cond_expiry_params expiry_params_list[] = {
+	/* timeout_ns is invalid/out-of-range */
+	{ .clk_unit = 0, .timeout_ns = -1LL,	.miniters = -1, .maxiters = 0, .desc = "invalid (-1LL)", },
+	{ .clk_unit = 0, .timeout_ns = ~0ULL,	.miniters = -1, .maxiters = 0, .desc = "invalid (~0ULL)", },
+	{ .clk_unit = 0, .timeout_ns = S64_MAX+1ULL, .miniters = -1, .maxiters = 0, .desc = "out-of-range (S64_MAX+1)", },
+	{ .clk_unit = 0, .timeout_ns = U64_MAX,	.miniters = -1, .maxiters = 0, .desc = "out-of-range (U64_MAX)", },
+	{ .clk_unit = 0, .timeout_ns = 0,	.miniters = -1, .maxiters = 1, .desc = "degenerate (0)",    },
+
+	/* timeout_ns is valid */
+	{ .clk_unit = (0x1ULL << 28), .timeout_ns = 1,		    .miniters = 1,	      .maxiters = -1, .desc = "1",    },
+	{ .clk_unit = (0x1ULL << 28), .timeout_ns = (0x1ULL << 30), .miniters = 1 << (30-28), .maxiters = -1, .desc = "1<<30",   },
+	{ .clk_unit = (0x1ULL << 28), .timeout_ns = S32_MAX,	    .miniters = 1 << (31-28), .maxiters = -1, .desc = "S32_MAX", },
+	{ .clk_unit = (0x1ULL << 28), .timeout_ns = U32_MAX,	    .miniters = 1 << (32-28), .maxiters = -1, .desc = "U32_MAX", },
+	{ .clk_unit = (0x1ULL << 28), .timeout_ns = (0x1ULL << 33), .miniters = 1 << (33-28), .maxiters = -1, .desc = "1<<33",   },
+};
+
+static void expiry_param_to_desc(const struct smp_cond_expiry_params *p, char *desc)
+{
+	char iters[32] = "";
+
+	if (p->miniters != -1)
+		snprintf(iters, 32, ">= %llx", (u64)p->miniters);
+	else if (p->maxiters != -1)
+		snprintf(iters, 32, "%s %llx", p->maxiters == 0 ? "==" : "<=", (u64)p->maxiters);
+
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE,
+		 "smp_cond_*_timeout: clock=%s,  timeout=%s, iterations %s",
+		 "synthetic", p->desc, iters);
+}
+
+static char *expiry_skip_test(const struct smp_cond_expiry_params *p)
+{
+#ifdef CONFIG_ARM64
+
+	/*
+	 * WFXT waits for the full p->timeout_ns so limit it to a reasonable
+	 * duration.
+	 */
+	if (alternative_has_cap_unlikely(ARM64_HAS_WFXT) &&
+	    p->timeout_ns >= NSEC_PER_SEC) {
+
+		return "runtime duration too long";
+	}
+#endif
+	return NULL;
+}
+
+static void test_smp_cond_relaxed(struct kunit *test)
+{
+	const struct smp_cond_expiry_params *p = test->param_value;
+	struct clock_state clk = {
+		.start_time = 0,
+		.end_time = 0,
+		.extra = p->clk_unit,
+		.niters = 0,
+	};
+	unsigned int result;
+	s64 runtime;
+	char *reason;
+
+	reason = expiry_skip_test(p);
+	if (reason)
+		kunit_skip(test, "%s", reason);
+
+	scu_flag = 0;
+
+	preempt_disable();
+	result = smp_cond_load_relaxed_timeout(&scu_flag,
+						0,
+						synthetic_clock(&clk),
+						p->timeout_ns);
+	preempt_enable();
+
+	runtime = (u64)clk.end_time - (u64)clk.start_time;
+
+	KUNIT_EXPECT_EQ(test, result, scu_flag);
+	/*
+	 * Check if we do the expected number of iterations.
+	 */
+	if (p->miniters != -1)
+		KUNIT_EXPECT_GE(test, clk.niters, p->miniters);
+	if (p->maxiters != -1)
+		KUNIT_EXPECT_LE(test, clk.niters, p->maxiters);
+
+	/*
+	 * maxiters == 0 means that the timeout is invalid/out-of-range.
+	 * When not, we cannot return with runtime < timeout_ns.
+	 */
+	if (p->maxiters != 0)
+		KUNIT_EXPECT_GE(test, runtime, p->timeout_ns);
+}
+
+KUNIT_ARRAY_PARAM(smp_cond_expiry_params, expiry_params_list, expiry_param_to_desc);
 static struct kunit_case barrier_timeout_test_cases[] = {
 	KUNIT_CASE_PARAM(test_smp_cond_timeout, smp_cond_update_params_gen_params),
+	KUNIT_CASE_PARAM(test_smp_cond_relaxed, smp_cond_expiry_params_gen_params),
 	{}
 };
 
-- 
2.43.7

[PATCH v16 16/16] barrier: timeout validity checks for smp_cond_load_acquire_timeout()

[Ankur Arora]

Extend timeout tests in smp_cond_expiry_params to also test
smp_cond_load_acquire_timeout().

The interface supports clocks that can return error on timeout.
These are used by rqspinlock. Test that by using a similar clock
that starts returning error past timeout.

Tested-by: Haris Okanovic <harisokn@amazon.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
 lib/tests/barrier-timeout-test.c | 58 +++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
index 88b374ceb78b..addb83ceca2d 100644
--- a/lib/tests/barrier-timeout-test.c
+++ b/lib/tests/barrier-timeout-test.c
@@ -158,7 +158,7 @@ static void expiry_param_to_desc(const struct smp_cond_expiry_params *p, char *d
 
 	snprintf(desc, KUNIT_PARAM_DESC_SIZE,
 		 "smp_cond_*_timeout: clock=%s,  timeout=%s, iterations %s",
-		 "synthetic", p->desc, iters);
+		 "synthetic/failing", p->desc, iters);
 }
 
 static char *expiry_skip_test(const struct smp_cond_expiry_params *p)
@@ -223,10 +223,66 @@ static void test_smp_cond_relaxed(struct kunit *test)
 		KUNIT_EXPECT_GE(test, runtime, p->timeout_ns);
 }
 
+/*
+ * Use a clock similar to the one used in rqspinlock where it start
+ * returning failure value once it goes beyond the timeout.
+ */
+static s64 failing_clock(const struct smp_cond_expiry_params *p,
+			 struct clock_state *clk)
+{
+	clk->end_time += clk->extra;
+	clk->niters++;
+
+	if ((u64)clk->end_time >= (u64)p->timeout_ns)
+		return -ETIMEDOUT;
+
+	return clk->end_time;
+}
+
+static void test_smp_cond_acquire_error(struct kunit *test)
+{
+	const struct smp_cond_expiry_params *p = test->param_value;
+	struct clock_state clk = {
+		.start_time = 0,
+		.end_time = 0,
+		.extra = p->clk_unit,
+		.niters = 0,
+	};
+	unsigned int result;
+	s64 ret = -ETIMEDOUT;
+	char *reason;
+
+	reason = expiry_skip_test(p);
+	if (reason)
+		kunit_skip(test, "%s", reason);
+
+	scu_flag = 0;
+
+	preempt_disable();
+	result = smp_cond_load_acquire_timeout(&scu_flag,
+						0,
+						(ret = failing_clock(p, &clk)),
+						p->timeout_ns);
+	preempt_enable();
+
+	KUNIT_EXPECT_EQ(test, result, scu_flag);
+	if (p->miniters != -1)
+		KUNIT_EXPECT_GE(test, clk.niters, p->miniters);
+	if (p->maxiters != -1)
+		KUNIT_EXPECT_LE(test, clk.niters, p->maxiters);
+
+	/*
+	 * scu_flag is never updated so the clock should eventually
+	 * hit the timeout and error out.
+	 */
+	KUNIT_EXPECT_EQ(test, ret, -ETIMEDOUT);
+}
+
 KUNIT_ARRAY_PARAM(smp_cond_expiry_params, expiry_params_list, expiry_param_to_desc);
 static struct kunit_case barrier_timeout_test_cases[] = {
 	KUNIT_CASE_PARAM(test_smp_cond_timeout, smp_cond_update_params_gen_params),
 	KUNIT_CASE_PARAM(test_smp_cond_relaxed, smp_cond_expiry_params_gen_params),
+	KUNIT_CASE_PARAM(test_smp_cond_acquire_error, smp_cond_expiry_params_gen_params),
 	{}
 };
 
-- 
2.43.7

Re: [PATCH v16 03/16] arm64/delay: move, fixup usecs_to_cycles()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] qcom/rpmh: integer overflow in rpmh_xloops_to_cycles() causes premature wakeups
--

commit cfe9540cf38fefd87b6b20f22814052cba488f5d
Author: Ankur Arora <ankur.a.arora@oracle.com>

arm64/delay: move, fixup usecs_to_cycles()

Updates xloops_to_cycles() to use mul_u64_u64_shr() instead of 32-bit fixed
point arithmetic to avoid truncation for delay values larger than ~10s. Also
moves delay constants and functions to a new header and renames colliding
macros in qcom/rpmh-rsc.c.

> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index f881af35ecfc5..dabe0ccba3706 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c

[ ... ]

> @@ -146,14 +146,16 @@ enum {
>   *  +---------------------------------------------------+
>   */
>  
> -#define USECS_TO_CYCLES(time_usecs)			\
> -	xloops_to_cycles((time_usecs) * 0x10C7UL)
> -
> -static inline unsigned long xloops_to_cycles(u64 xloops)
> +static inline u64 rpmh_xloops_to_cycles(u64 xloops)
>  {
>  	return (xloops * loops_per_jiffy * HZ) >> 32;
>  }

[Severity: High]
Does this code overflow the 64-bit bounds for sleep delays longer than ~224
seconds?

The core arm64 delay code in this patch was updated to use 128-bit arithmetic
(mul_u64_u64_shr) to avoid overflow for delays larger than ~10s. However, it
looks like the old fixed-point math was preserved here during the rename to
rpmh_xloops_to_cycles().

>  
> +static inline u64 rpmh_usecs_to_cycles(u64 time_usecs)
> +{
> +	return rpmh_xloops_to_cycles((time_usecs) * 0x10C7UL);
> +}
> +

[ ... ]

> @@ -828,7 +830,7 @@ void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv)
>  	wakeup_us = ktime_to_us(wakeup);
>  
>  	/* Convert the wakeup to arch timer scale */
> -	wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
> +	wakeup_cycles = rpmh_usecs_to_cycles(wakeup_us);

[Severity: High]
If wakeup_us is derived from the time to the next hrtimer, which could be
hours or days away during deep system idle/suspend, will passing this into
rpmh_usecs_to_cycles() program a prematurely small, corrupted cycle count
into the hardware due to the truncation mentioned above?

>  	wakeup_cycles += arch_timer_read_counter();
>  
>  exit:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=3

Re: [PATCH v16 15/16] barrier: timeout validity checks for smp_cond_load_relaxed_timeout()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] kselftest: barrier: incorrectly skips invalid timeout tests on ARM64
--

commit bda3f35055835da4a210dfc722306737806214d9
Author: Ankur Arora <ankur.a.arora@oracle.com>

barrier: timeout validity checks for smp_cond_load_relaxed_timeout()

This commit adds timeout tests for smp_cond_load_relaxed_timeout() to verify
that the implementation returns early on invalid timeout values and handles
edge cases sanely.

> diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
> index 9c12cd36e7d29..88b374ceb78b4 100644
> --- a/lib/tests/barrier-timeout-test.c
> +++ b/lib/tests/barrier-timeout-test.c
[ ... ]
> +static char *expiry_skip_test(const struct smp_cond_expiry_params *p)
> +{
> +#ifdef CONFIG_ARM64
> +
> +	/*
> +	 * WFXT waits for the full p->timeout_ns so limit it to a reasonable
> +	 * duration.
> +	 */
> +	if (alternative_has_cap_unlikely(ARM64_HAS_WFXT) &&
> +	    p->timeout_ns >= NSEC_PER_SEC) {

[Severity: Low]
Does this skip the invalid timeout test cases on ARM64 by mistake?

The expiry_params_list includes intentional negative values (like -1LL or
~0ULL) to verify error handling, but p->timeout_ns is defined as a u64.
When these invalid inputs are compared as unsigned, they evaluate to numbers
larger than NSEC_PER_SEC, causing them to be skipped with the reason
"runtime duration too long".

Because the smp_cond_load_relaxed_timeout() macro casts timeouts to s64 and
returns instantly for negative values, these cases never actually execute a
WFXT sleep. Skipping them deprives ARM64 of test coverage for the
early-return fast path.

Could this regression be resolved by checking if
(s64)p->timeout_ns >= NSEC_PER_SEC, or by avoiding the skip when
p->maxiters == 0?

> +
> +		return "runtime duration too long";
> +	}
> +#endif
> +	return NULL;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=15

Re: [PATCH v16 13/16] arm64/delay: enable testing smp_cond_load_relaxed_timeout()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: barrier: missing exports for smp_cond_load_*_timeout() backends
--

commit bfc8166431acdeebcd1866a53cbd230aa1bcd738
Author: Ankur Arora <ankur.a.arora@oracle.com>

arm64/delay: enable testing smp_cond_load_relaxed_timeout()

This commit enables the barrier tests to be built as a module by exporting
the underlying delay and timer functions when KUnit is enabled.

> diff --git a/arch/arm64/lib/delay.c b/arch/arm64/lib/delay.c
> index f08eacef2f0a4..20bc2894b65b9 100644
> --- a/arch/arm64/lib/delay.c
> +++ b/arch/arm64/lib/delay.c
[ ... ]
> @@ -30,6 +31,7 @@ u64 notrace __delay_cycles(void)
>  	guard(preempt_notrace)();
>  	return __arch_counter_get_cntvct_stable();
>  }
> +EXPORT_SYMBOL_IF_KUNIT(__delay_cycles);

[Severity: Medium]
Does exporting __delay_cycles() only for KUnit cause unresolved symbol errors
for standard loadable modules?

If a standard non-test module uses atomic_cond_read_acquire_timeout() or
smp_cond_load_relaxed_timeout() on ARM64, the macros expand to inline barrier
functions like __cmpwait_relaxed_timeout(), which call __delay_cycles().

Since this symbol is only exported when CONFIG_KUNIT is enabled, will standard
production builds fail to build or load modules using these APIs?

>  
>  void __delay(unsigned long cycles)
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index cc9a8b399004d..227447b19b143 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
[ ... ]
> @@ -893,6 +894,7 @@ bool arch_timer_evtstrm_available(void)
>  	 */
>  	return cpumask_test_cpu(raw_smp_processor_id(), &evtstrm_available);
>  }
> +EXPORT_SYMBOL_IF_KUNIT(arch_timer_evtstrm_available);

[Severity: Medium]
Similarly, does restricting the export of arch_timer_evtstrm_available() to
KUnit builds break standard modules?

The inline cpu_poll_relax() backend relies on this function. Standard modules
using the new timeout APIs might fail to resolve this symbol when compiled
without CONFIG_KUNIT. Should these be exported unconditionally using standard
EXPORT_SYMBOL_GPL() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=13

Re: [PATCH v16 06/16] asm-generic: barrier: Add smp_cond_load_acquire_timeout()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] barrier: smp_cond_load_acquire_timeout() lacks ACQUIRE ordering on timeout
- [Medium] barrier: __smp_cond_load_relaxed_timeout() blocks for 1us on first iteration
--

commit 8af3000143cb6b1410d5362105d417992cfb019c
Author: Ankur Arora <ankur.a.arora@oracle.com>

asm-generic: barrier: Add smp_cond_load_acquire_timeout()

This commit adds smp_cond_load_acquire_timeout(), which serves as the acquire
variant of smp_cond_load_relaxed_timeout(). It performs its own error checking
and reuses the relaxed logic for the wait, relying on a control dependency
followed by an acquire barrier to provide LOAD->STORE ordering.

> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index ac0a8b5e0ffe7..fc244dbd040b0 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h
[ ... ]
> @@ -383,6 +383,53 @@ do {									\
[ ... ]
> +	VAL = __smp_cond_load_relaxed_timeout(ptr,			\
> +					      cond_expr,		\
> +					      time_expr_ns,		\
> +					      __scl_timeout_ns);	\

[Severity: Medium]
Will this macro block for 1us on the first iteration even if the caller
requests a very short or zero timeout?

Looking at __smp_cond_load_relaxed_timeout(), it initializes
__scl_timeout to NSEC_PER_USEC and unconditionally calls cpu_poll_relax()
before checking the actual timeout_ns:

    u64 __scl_timeout = NSEC_PER_USEC, __scl_time_end = 0;
    ...
    for (;;) {
        VAL = READ_ONCE(*__PTR);
        if (cond_expr || __scl_timed_out)
            break;
        cpu_poll_relax(__PTR, VAL, (u64)__scl_timeout);

If an architecture implements cpu_poll_relax() with a precise timeout, could
this penalize callers who requested a minimum wait with an unnecessary 1000ns
delay?

> +	/*								\
> +	 * We return having waited on the control dependency,		\
> +	 * (cond_expr || __scl_timed_out). Since we break out only	\
> +	 * after either of these conditions are true, the conditional	\
> +	 * separates the execution stream into two happened-before	\
> +	 * and happened-after sequences. And since STOREs are not	\
> +	 * speculated, this takes cares of the LOAD->STORE ordering.	\
> +	 *								\
> +	 * Provide the remaining LOAD->LOAD ordering.			\
> +	 */								\
> +	smp_acquire__after_ctrl_dep();					\

[Severity: High]
Does this code actually provide ACQUIRE ordering on the timeout path?

Inside __smp_cond_load_relaxed_timeout(), the loop exits via:

    if (cond_expr || __scl_timed_out)
        break;

When a timeout occurs, __scl_timed_out is set to true, and the loop continues
to the next iteration. On this final iteration, the compiler statically knows
that __scl_timed_out is true.

Could it optimize the branch into an unconditional jump to break, destroying
the control dependency?

Without a control dependency, smp_acquire__after_ctrl_dep() only provides
LOAD->LOAD ordering, and the LOAD->STORE ordering required by ACQUIRE
semantics is lost on weakly-ordered CPUs. Can this lead to data corruption?

> +__scl_out:								\
> +	(typeof(*(ptr)))VAL;						\
> +})
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=6

Re: [PATCH v16 01/16] asm-generic: barrier: Add smp_cond_load_relaxed_timeout()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] locking: smp_cond_load_relaxed_timeout breaks control dependency for ACQUIRE semantics
--

commit 33828050548c87a2007a049070718fdd9167df34
Author: Ankur Arora <ankur.a.arora@oracle.com>

asm-generic: barrier: Add smp_cond_load_relaxed_timeout()

This commit adds smp_cond_load_relaxed_timeout(), extending
smp_cond_load_relaxed() to allow waiting for a duration with a periodic
time-check. It uses cpu_poll_relax() in a loop and defers the potentially
costly time evaluation to a slowpath.

> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index b99cb57dfcccc..ac0a8b5e0ffe7 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h
[ ... ]
> +#ifndef smp_cond_load_relaxed_timeout
> +#define __smp_cond_load_relaxed_timeout(ptr, cond_expr,			\
> +				      time_expr_ns, timeout_ns)		\
> +({									\
> +	typeof(ptr) __PTR = (ptr);					\
> +	__unqual_scalar_typeof(*(ptr)) VAL;				\
> +	u32 __scl_count = 0, __scl_spin = SMP_TIMEOUT_POLL_COUNT;	\
> +	u64 __scl_timeout = NSEC_PER_USEC, __scl_time_end = 0;		\
> +	s64 __scl_time_now;						\
> +	bool __scl_timed_out = false;					\
> +									\
> +	for (;;) {							\
> +		VAL = READ_ONCE(*__PTR);				\
> +		if (cond_expr || __scl_timed_out)			\

[Severity: High]
In include/asm-generic/barrier.h, __smp_cond_load_relaxed_timeout() evaluates
cond_expr alongside __scl_timed_out. Does this break the control dependency
required for ACQUIRE semantics?

If __scl_timed_out is true from the previous iteration, this branch is taken
independently of cond_expr. Because the branch outcome is independent of the
loaded value VAL, the compiler can optimize the exit into an unconditional
break, meaning the CPU has no hardware-level control dependency to enforce.

Since smp_cond_load_acquire_timeout() (added in a subsequent patch) relies on
smp_cond_load_relaxed_timeout() to provide a control dependency on the loaded
value which is then upgraded to an ACQUIRE barrier by
smp_acquire__after_ctrl_dep(), this could allow subsequent critical section
memory accesses to be reordered before the lock acquisition load.

If the returned VAL concurrently satisfies the caller's acquisition check
at the exact moment of timeout, the caller will enter the critical section
without hardware-enforced LOAD-STORE ordering.

> +			break;						\
> +		cpu_poll_relax(__PTR, VAL, (u64)__scl_timeout);		\
> +		if (++__scl_count < __scl_spin)				\
> +			continue;					\
[ ... ]
> +		if (__scl_time_now <= 0 ||				\
> +		    __scl_time_end <= __scl_time_now) {			\
> +			__scl_timed_out = true;				\
> +			continue;					\
> +		}							\
[ ... ]
> +	}								\
> +	(typeof(*(ptr)))VAL;						\
> +})
> +
> +#define smp_cond_load_relaxed_timeout(ptr, cond_expr,			\
> +				      time_expr_ns, timeout_ns)		\
> +({									\
> +	s64 __scl_timeout_ns = (s64)(timeout_ns);			\
> +	__unqual_scalar_typeof(*(ptr)) VAL;				\
> +									\
> +	if (unlikely(__scl_timeout_ns < 0))				\
> +		VAL = READ_ONCE(*(ptr));				\

[Severity: High]
In include/asm-generic/barrier.h, smp_cond_load_relaxed_timeout() handles
negative timeouts. Does this negative timeout path also bypass the control
dependency?

Because this path executes VAL = READ_ONCE(*(ptr)); and returns without
executing a conditional branch on VAL, no control dependency is established
for the acquire barrier to upgrade.

> +	else								\
> +		VAL = __smp_cond_load_relaxed_timeout(ptr, cond_expr,	\
> +						      time_expr_ns,	\
> +						      __scl_timeout_ns);\
> +	(typeof(*(ptr)))VAL;						\
> +})
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=1

Re: [PATCH v16 02/16] arm64: barrier: Support smp_cond_load_relaxed_timeout()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] arm64/barrier: massive timekeeping contention in spin loops without event stream
- [Medium] arm64: barrier: missing export of arch_timer_evtstrm_available() breaks modules
- [Medium] arm64: noinstr violation in cpuidle via cpu_poll_relax()
--

commit 7a1bb257570118eb8f711a76f172539e20eeaac8
Author: Ankur Arora <ankur.a.arora@oracle.com>

arm64: barrier: Support smp_cond_load_relaxed_timeout()

This commit supports waiting in smp_cond_load_relaxed_timeout() via
__cmpwait_relaxed() when the event-stream is available, falling back to
cpu_relax() otherwise. It defines CPU_POLL_RELAX_WAITS to indicate that
there is an optimized wait implementation.

> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 9495c4441a46b..e2263c754a550 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h

[ ... ]

> @@ -219,6 +220,27 @@ do {									\
>  	(typeof(*ptr))VAL;						\
>  })
>  
> +/* Re-declared here to avoid include dependency. */
> +extern bool arch_timer_evtstrm_available(void);
> +
> +/*
> + * In the common case, cpu_poll_relax() sits waiting in __cmpwait_relaxed()
> + * for @ptr value to change.
> + *
> + * State this by defining CPU_POLL_RELAX_WAITS. This enables a time-check
> + * optimization in smp_cond_load_{relaxed,acquire}_timeout(). Note that this
> + * is not ideal for the cpu_relax() fallback but that's not a production
> + * configuration.
> + */
> +#define CPU_POLL_RELAX_WAITS

[Severity: High]
Does defining CPU_POLL_RELAX_WAITS here cause massive timekeeping contention
when the event stream is disabled?

When CPU_POLL_RELAX_WAITS is defined, include/asm-generic/barrier.h sets
SMP_TIMEOUT_POLL_COUNT to 1. This causes the loop in
__smp_cond_load_relaxed_timeout() to evaluate the expensive time_expr_ns
on every single iteration.

If the event stream is disabled, cpu_poll_relax() drops back to a fast
cpu_relax() yield. Evaluating the time expression (e.g. reading timer hardware
or seqcounts) on every fast spin loop iteration across multiple CPUs can cause
massive lock contention and interconnect flooding.

Is it accurate that disabling the event stream is "not a production
configuration"? It can be disabled via Kconfig
(CONFIG_ARM_ARCH_TIMER_EVTSTREAM=n), kernel command line
(clocksource.arm_arch_timer.evtstrm=0), or under hypervisors that lack
virtual event stream support.

> +
> +#define cpu_poll_relax(ptr, val, timeout_ns) do {			\
> +	if (arch_timer_evtstrm_available())				\

[Severity: Medium]
Does this break the build for loadable kernel modules that use the new
smp_cond_load_relaxed_timeout() or smp_cond_load_acquire_timeout() macros?

This macro injects a direct call to arch_timer_evtstrm_available() into the
caller's context. However, arch_timer_evtstrm_available() is defined in
drivers/clocksource/arm_arch_timer.c and lacks an EXPORT_SYMBOL_GPL directive.

Without the export, standard loadable modules using the generic barrier
timeout API will fail to build with undefined symbol errors.

[Severity: Medium]
Can this introduce a noinstr violation in the idle entry path?

arch_timer_evtstrm_available() is an out-of-line, instrumentable function
since it is not marked noinstr.

When smp_cond_load_relaxed_timeout() is used in poll_idle()
(in drivers/cpuidle/poll_state.c), it is executed in a noinstr context
because poll_idle() is marked __cpuidle, which places it in the
.cpuidle.text section. Objtool strictly validates .cpuidle.text as noinstr.

Calling an instrumentable function directly from the idle spin-loop macro
risks tracing recursion and objtool validation errors.

> +		__cmpwait_relaxed(ptr, val);				\
> +	else								\
> +		cpu_relax();						\
> +} while (0)
> +
>  #include <asm-generic/barrier.h>
>  
>  #endif	/* __ASSEMBLER__ */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=2

Re: [PATCH v16 07/16] atomic: Add atomic_cond_read_*_timeout()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] atomic: torn reads in atomic64_cond_read_*_timeout on 32-bit
- [Low] docs: incorrect categorization of atomic_t operations
--

commit 9e35fd2167c99d05e59317145e35667a8118c80e
Author: Ankur Arora <ankur.a.arora@oracle.com>

atomic: Add atomic_cond_read_*_timeout()

Add atomic load wrappers, atomic_cond_read_*_timeout() and
atomic64_cond_read_*_timeout() for the cond-load timeout interfaces.

> diff --git a/Documentation/atomic_t.txt b/Documentation/atomic_t.txt
> index bee3b1bca9a7b..0e53f6ccb5580 100644
> --- a/Documentation/atomic_t.txt
> +++ b/Documentation/atomic_t.txt
> @@ -16,6 +16,10 @@ Non-RMW ops:
>    atomic_read(), atomic_set()
>    atomic_read_acquire(), atomic_set_release()
>  
> +Non-RMW, non-atomic_t ops:
> +
> +  atomic_cond_read_{relaxed,acquire}()
> +  atomic_cond_read_{relaxed,acquire}_timeout()

[Severity: Low]
Does this section header miscategorize these operations?

These macros explicitly take an atomic_t pointer as their primary argument and
operate on atomic types. Was the intention to denote them as conditional wait
operations instead of non-atomic_t operations?

[ ... ]

> diff --git a/include/linux/atomic.h b/include/linux/atomic.h
> index 8dd57c3a99e9b..5bcb86e077845 100644
> --- a/include/linux/atomic.h
> +++ b/include/linux/atomic.h
> @@ -31,6 +31,16 @@
>  #define atomic64_cond_read_acquire(v, c) smp_cond_load_acquire(&(v)->counter, (c))
>  #define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
>  
> +#define atomic_cond_read_acquire_timeout(v, c, e, t) \
> +	smp_cond_load_acquire_timeout(&(v)->counter, (c), (e), (t))
> +#define atomic_cond_read_relaxed_timeout(v, c, e, t) \
> +	smp_cond_load_relaxed_timeout(&(v)->counter, (c), (e), (t))
> +
> +#define atomic64_cond_read_acquire_timeout(v, c, e, t) \
> +	smp_cond_load_acquire_timeout(&(v)->counter, (c), (e), (t))
> +#define atomic64_cond_read_relaxed_timeout(v, c, e, t) \
> +	smp_cond_load_relaxed_timeout(&(v)->counter, (c), (e), (t))

[Severity: High]
Could passing the address of the 64-bit counter directly to the unlocked
polling macro lead to torn reads on 32-bit architectures?

The generic atomic64_t implementation for 32-bit architectures (used when
lacking native 64-bit atomic instructions) updates the counter via two 32-bit
stores protected by a hashed array of spinlocks in lib/atomic64.c.

Because smp_cond_load_acquire_timeout polls the variable using an unlocked
READ_ONCE(), concurrent updates to the counter will bypass the spinlock.

On 32-bit architectures, READ_ONCE() on an 8-byte value evaluates successfully
but compiles into two independent 32-bit reads, which can result in data
corruption.

Additionally, smp_cond_load_acquire_timeout relies on smp_load_acquire() when
timeout_ns < 0. Can this cause a build failure on 32-bit architectures due to
compiletime_assert_atomic_type() preventing 8-byte acquire loads?

The pre-existing atomic64_cond_read_acquire and atomic64_cond_read_relaxed
macros share this identical architectural flaw, and it looks like the new
API macros propagate the same vulnerability.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=7

Re: [PATCH v16 04/16] arm64: support WFET in smp_cond_load_relaxed_timeout()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] arm64: missing EXPORT_SYMBOL for cpu_poll_relax() dependencies
--

commit 763c188baf64d23043b0ab18c53d5eb2722c766e
Author: Ankur Arora <ankur.a.arora@oracle.com>

arm64: support WFET in smp_cond_load_relaxed_timeout()

This patch adds support for WFET in smp_cond_load_relaxed_timeout() and
smp_cond_load_acquire_timeout() by introducing timeout-aware variants of
__cmpwait() and cpu_poll_relax().

> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index e2263c754a550..3b024058cf2f6 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
[ ... ]
> @@ -235,7 +235,9 @@ extern bool arch_timer_evtstrm_available(void);
>  #define CPU_POLL_RELAX_WAITS
>  
>  #define cpu_poll_relax(ptr, val, timeout_ns) do {			\
> -	if (arch_timer_evtstrm_available())				\
> +	if (alternative_has_cap_unlikely(ARM64_HAS_WFXT))		\
> +		__cmpwait_relaxed_timeout(ptr, val, timeout_ns);	\
> +	else if (arch_timer_evtstrm_available())			\

[Severity: Low]
Does this lack an EXPORT_SYMBOL for loadable modules? 

Like __delay_cycles() below, arch_timer_evtstrm_available() is called within
a macro that can be expanded by loadable modules. If it isn't exported to
general modules, will modules using smp_cond_load_relaxed_timeout() fail to
link?

>  		__cmpwait_relaxed(ptr, val);				\
>  	else								\
>  		cpu_relax();						\
> diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
> index 6cf3cd6873f5f..30dca19c9a7b5 100644
> --- a/arch/arm64/include/asm/cmpxchg.h
> +++ b/arch/arm64/include/asm/cmpxchg.h
[ ... ]
> @@ -235,20 +237,52 @@ __CMPWAIT_CASE( ,  , 64);
>  
>  #undef __CMPWAIT_CASE
>  
> -#define __CMPWAIT_GEN(sfx)						\
> -static __always_inline void __cmpwait##sfx(volatile void *ptr,		\
> -				  unsigned long val,			\
> -				  int size)				\
> +#define __CMPWAIT_TIMEOUT_CASE(w, sfx, sz)				\
> +static inline void __cmpwait_case_timeout_##sz(volatile void *ptr,	\
> +					       unsigned long val,	\
> +					       u64 timeout_ns)		\
> +{									\
> +	unsigned long tmp;						\
> +	u64 ecycles = __delay_cycles() +				\
> +			nsecs_to_cycles(timeout_ns);			\

[Severity: Low]
Will this cause linker errors for loadable modules?

Since __cmpwait_case_timeout_##sz() is an inline function instantiated by
generic barrier APIs like smp_cond_load_relaxed_timeout(), it injects a call
to __delay_cycles() into the caller. 

Is __delay_cycles() exported for use by loadable modules, or will this break
out-of-tree or standard kernel modules that include these headers and use the
timeout macros?

> +	asm volatile(							\
> +	"	sevl\n"							\
> +	"	wfe\n"							\
> +	"	ldxr" #sfx "\t%" #w "[tmp], %[v]\n"			\

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016070837.440748-1-ankur.a.arora@oracle.com?part=4

Re: [PATCH v16 15/16] barrier: timeout validity checks for smp_cond_load_relaxed_timeout()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

> barrier: timeout validity checks for smp_cond_load_relaxed_timeout()

TL;DR: Nits only, in the new KUnit test: iteration bounds print as unprefixed
hex ("10" for 16), which is confusing in test names, and a format string has a
stray double space.

> diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
> index 9c12cd36e7d2..88b374ceb78b 100644
> --- a/lib/tests/barrier-timeout-test.c
> +++ b/lib/tests/barrier-timeout-test.c

[ ... ]

> +static void expiry_param_to_desc(const struct smp_cond_expiry_params *p, char *desc)
> +{
> +	char iters[32] = "";
> +
> +	if (p->miniters != -1)
> +		snprintf(iters, 32, ">= %llx", (u64)p->miniters);
> +	else if (p->maxiters != -1)
> +		snprintf(iters, 32, "%s %llx", p->maxiters == 0 ? "==" : "<=", (u64)p->maxiters);
> +
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE,
> +		 "smp_cond_*_timeout: clock=%s,  timeout=%s, iterations %s",
> +		 "synthetic", p->desc, iters);
> +}

This isn't a bug, but would printing the iteration bound in decimal (%lld)
or with a 0x prefix make the test names easier to match against the checks?

With "%llx" and no prefix, the U32_MAX row (miniters = 1 << (32-28)) is
reported as "iterations >= 10" and the 1<<33 row (miniters = 1 << (33-28))
as "iterations >= 20", while test_smp_cond_relaxed() actually checks for
>= 16 and >= 32.  The timeout labels in the same table use the "1<<33"
style, so the hex counts read inconsistently in the KTAP output.

Also, is the double space after "clock=%s," in the format string intended?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38867477666

Re: [PATCH v16 03/16] arm64/delay: move, fixup usecs_to_cycles()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2530 bytes --]

> arm64/delay: move, fixup usecs_to_cycles()

TL;DR: The new generic usecs_to_cycles() in arm64 delay-const.h likely
conflicts with gpio-aspeed.c's function of the same name once a later patch
includes it, so arm64 builds with CONFIG_GPIO_ASPEED (allmodconfig, defconfig)
would fail.

> diff --git a/arch/arm64/include/asm/delay-const.h b/arch/arm64/include/asm/delay-const.h
> new file mode 100644
> index 000000000000..6ccdfe0a4130
> --- /dev/null
> +++ b/arch/arm64/include/asm/delay-const.h
> @@ -0,0 +1,35 @@

[ ... ]

> +static inline u64 usecs_to_cycles(u64 time_usecs)
> +{
> +	return xloops_to_cycles(time_usecs * __usecs_to_xloops_mult);
> +}

[ ... ]

Can these generic names conflict with other file-scope definitions once this
header is included widely?

The commit message says the rpmh-rsc.c helpers were renamed because they
"were occupying the same namespace", but drivers/gpio/gpio-aspeed.c also
defines a function with the same name and a different signature:

drivers/gpio/gpio-aspeed.c:
static int usecs_to_cycles(struct aspeed_gpio *gpio, unsigned long usecs,
		u32 *cycles)

Later in this series, commit 117729ec2d64 ("arm64: support WFET in
smp_cond_load_relaxed_timeout()") adds this to
arch/arm64/include/asm/cmpxchg.h:

+#include <asm/delay-const.h>

gpio-aspeed.c reaches it through:

linux/spinlock.h -> linux/atomic.h -> asm/atomic.h -> asm/cmpxchg.h ->
asm/delay-const.h

At that point 'static inline u64 usecs_to_cycles(u64)' and 'static int
usecs_to_cycles(struct aspeed_gpio *, unsigned long, u32 *)' are two
file-scope declarations of one identifier with incompatible types, which
should fail with "conflicting types for 'usecs_to_cycles'".

GPIO_ASPEED depends on ARCH_ASPEED || COMPILE_TEST, and ARCH_ASPEED is an
arm64 platform option that is enabled in arch/arm64/configs/defconfig.  So
it looks like any arm64 config enabling CONFIG_GPIO_ASPEED, including
allmodconfig and allyesconfig, would fail to build.

The build first breaks at 117729ec2d64, but the cause is the namespace
cleanup here only covering rpmh-rsc.c.  Nothing up to the end of the series
(c43dfae4ee10) changes the gpio-aspeed definition.

Would renaming the gpio-aspeed helper, or giving the arm64 helpers a
namespaced prefix (for example arch_ or __arm64_), avoid the collision?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38867477666

Re: [PATCH v16 14/16] barrier: add tests for smp_cond_load_*_timeout()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

> barrier: add tests for smp_cond_load_*_timeout()

TL;DR: Nit only, in the KUnit test patch: the suite name
"smp-cond-load-*-timeout" and file name barrier-timeout-test.c may not follow
KUnit naming conventions (underscores, _kunit.c suffix).

> diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
> new file mode 100644
> index 000000000000..9c12cd36e7d2
> --- /dev/null
> +++ b/lib/tests/barrier-timeout-test.c
> @@ -0,0 +1,129 @@

[ ... ]

> +static struct kunit_suite barrier_timeout_test_suite = {
> +	.name = "smp-cond-load-*-timeout",
> +	.test_cases = barrier_timeout_test_cases,
> +};

this isn't a bug, but would a different suite name fit the KUnit naming
conventions better?

Documentation/dev-tools/kunit/style.rst asks for underscores rather than
dashes in suite names, and for the test file to be named
tests/<suite>_kunit.c.  The current name "smp-cond-load-*-timeout" uses
dashes and a literal '*', and the file is barrier-timeout-test.c.

Would something like smp_cond_load_timeout (with the file named
lib/tests/barrier_timeout_kunit.c or similar) work instead?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38867477666

Re: [PATCH v16 04/16] arm64: support WFET in smp_cond_load_relaxed_timeout()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

> arm64: support WFET in smp_cond_load_relaxed_timeout()

TL;DR: Including asm/delay-const.h from asm/cmpxchg.h may break the arm64
build of drivers/gpio/gpio-aspeed.c, whose usecs_to_cycles() would conflict
with the new one; it appears unfixed by the series.

> diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
> index 6cf3cd6873f5..30dca19c9a7b 100644
> --- a/arch/arm64/include/asm/cmpxchg.h
> +++ b/arch/arm64/include/asm/cmpxchg.h
> @@ -12,6 +12,7 @@
>
>  #include <asm/barrier.h>
>  #include <asm/lse.h>
> +#include <asm/delay-const.h>

Can this include break the build of drivers/gpio/gpio-aspeed.c?

asm/cmpxchg.h is pulled into almost every arm64 translation unit through
linux/spinlock.h -> linux/atomic.h -> asm/atomic.h -> asm/cmpxchg.h, so the
static inlines in asm/delay-const.h become visible everywhere.

gpio-aspeed.c already defines a helper with the same name and a different
signature:

drivers/gpio/gpio-aspeed.c:
    static int usecs_to_cycles(struct aspeed_gpio *gpio, unsigned long usecs,
                               u32 *cycles)

while asm/delay-const.h now provides:

arch/arm64/include/asm/delay-const.h:
    static inline u64 usecs_to_cycles(u64 time_usecs)

That looks like it would fail with "conflicting types for 'usecs_to_cycles'".
GPIO_ASPEED depends on ARCH_ASPEED || COMPILE_TEST, and ARCH_ASPEED is an
arm64 platform, so arm64 allmodconfig/allyesconfig and any config enabling
CONFIG_GPIO_ASPEED would stop building at this commit.

The previous patch ("arm64/delay: move, fixup usecs_to_cycles()") renamed the
rpmh-rsc helpers for the same name clash, but gpio-aspeed.c was not touched.
No commit in the rest of the series modifies gpio-aspeed.c or delay-const.h,
so the breakage appears to remain at the end of the series.

Could the gpio-aspeed helper be renamed, could the delay-const.h helpers get
an arch-specific prefix, or could the cycle computation be moved out of line
so that asm/cmpxchg.h doesn't need to include asm/delay-const.h?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38867477666

Re: [PATCH v16 01/16] asm-generic: barrier: Add smp_cond_load_relaxed_timeout()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

> asm-generic: barrier: Add smp_cond_load_relaxed_timeout()

TL;DR: Nit only: the kernel-doc comment in asm-generic/barrier.h binds to the
internal __smp_cond_load_relaxed_timeout() helper, causing a new kernel-doc
warning; it should move above the public macro.

> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index b99cb57dfccc..ac0a8b5e0ffe 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h

[ ... ]

> @@ -273,6 +274,115 @@ do {									\
>  })
>  #endif
>

[ ... ]

> +/**
> + * smp_cond_load_relaxed_timeout() - (Spin) wait for cond with no ordering
> + * guarantees until a timeout expires.
> + * @ptr: pointer to the variable to wait on.
> + * @cond_expr: boolean expression to wait for.

[ ... ]

> + * dependent ways.
> + */
> +#ifndef smp_cond_load_relaxed_timeout
> +#define __smp_cond_load_relaxed_timeout(ptr, cond_expr,			\
> +				      time_expr_ns, timeout_ns)		\
> +({									\

[ ... ]

> +#define smp_cond_load_relaxed_timeout(ptr, cond_expr,			\
> +				      time_expr_ns, timeout_ns)		\
> +({									\

[ ... ]

Does the kernel-doc comment above attach to the right macro?  It sits
directly above "#ifndef smp_cond_load_relaxed_timeout" followed by the
"#define __smp_cond_load_relaxed_timeout(" helper, so kernel-doc skips the
#ifndef and binds the comment to the internal helper:

  $ tools/docs/kernel-doc -none include/asm-generic/barrier.h
  Warning: include/asm-generic/barrier.h expecting prototype for
  smp_cond_load_relaxed_timeout(). Prototype was for
  __smp_cond_load_relaxed_timeout() instead

The parent commit produces no kernel-doc warnings for this header, and the
warning is still present at the end of the series, so no later patch
addresses it.  The public smp_cond_load_relaxed_timeout() macro ends up
with no kernel-doc.

Could the /** block be moved down so it sits directly above
"#define smp_cond_load_relaxed_timeout(", with the
__smp_cond_load_relaxed_timeout() helper getting a plain /* comment?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38867477666

Re: [PATCH v16 03/16] arm64/delay: move, fixup usecs_to_cycles()

[Catalin Marinas]

On Fri, Oct 16, 2026 at 12:08:24AM -0700, Ankur Arora wrote:
> diff --git a/arch/arm64/include/asm/delay-const.h b/arch/arm64/include/asm/delay-const.h
> new file mode 100644
> index 000000000000..6ccdfe0a4130
> --- /dev/null
> +++ b/arch/arm64/include/asm/delay-const.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _ASM_DELAY_CONST_H
> +#define _ASM_DELAY_CONST_H
> +
> +#include <linux/math64.h>
> +#include <linux/types.h>
> +#include <asm/param.h>	/* For HZ */
> +
> +/* 2**32 / 1000000 (rounded up) */
> +#define __usecs_to_xloops_mult	0x10C7UL
> +
> +/* 2**32 / 1000000000 (rounded up) */
> +#define __nsecs_to_xloops_mult	0x5UL
> +
> +extern unsigned long loops_per_jiffy;
> +static inline u64 xloops_to_cycles(u64 xloops)
> +{
> +	u64 loops_per_sec = (u64)loops_per_jiffy * HZ;
> +
> +	return mul_u64_u64_shr(xloops, loops_per_sec, 32);
> +}
> +
> +static inline u64 usecs_to_cycles(u64 time_usecs)
> +{
> +	return xloops_to_cycles(time_usecs * __usecs_to_xloops_mult);
> +}
> +
> +static inline u64 nsecs_to_cycles(u64 time_nsecs)
> +{
> +	return xloops_to_cycles((time_nsecs) * __nsecs_to_xloops_mult);
> +}
> +
> +u64 notrace __delay_cycles(void);
> +
> +#endif	/* _ASM_DELAY_CONST_H */

The bot reports are valid, moving these into headers pulled elsewhere
will clash with existing functions with the same name. You fixed one but
there's another. Isn't it easier to just add an arm64_ prefix to these?

Alternatively, I think we only need nsecs_to_cycles(). We have an out of
line call to __delay_cycles() already, so can we not just add a new
function to use directly and avoid polluting the headers:

u64 arm64_wfet_deadline(u64 timeout_ns)
{
        return __delay_cycles() + nsecs_to_cycles(timeout_ns);
}
EXPORT_SYMBOL_GPL(arm64_wfet_deadline);

I prefer the latter if it works.

-- 
Catalin