Re: [PATCH v12 02/15] arm64: barrier: Support smp_cond_load_relaxed_timeout()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Zero or negative timeout requests incorrectly block in WFE/WFET.
- [Low] The macro `cpu_poll_relax` references `arch_timer_evtstrm_available`, which is not exported, causing link errors for loadable modules.
--

commit aa12eab164f56555b3ffac8b49cd2f96ce88e36c
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 on ARM64 when the event-stream is enabled.

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

[ ... ]

> @@ -219,6 +220,26 @@ 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 the ptr value to change.
> + *
> + * Since this period is reasonably long, choose SMP_TIMEOUT_POLL_COUNT
> + * to be 1, so smp_cond_load_{relaxed,acquire}_timeout() does a
> + * time-check in each iteration.
> + */
> +#define SMP_TIMEOUT_POLL_COUNT	1
> +
> +#define cpu_poll_relax(ptr, val, timeout_ns) do {			\
> +	if (arch_timer_evtstrm_available())				\

[Severity: Low]
Will this regression cause undefined symbol errors for loadable modules?

Since smp_cond_load_relaxed_timeout is a generic synchronization API, it will
be expanded in caller modules and rely on cpu_poll_relax.

However, arch_timer_evtstrm_available is currently only exported for KUnit
testing in drivers/clocksource/arm_arch_timer.c:

    EXPORT_SYMBOL_IF_KUNIT(arch_timer_evtstrm_available);

Does this need a general EXPORT_SYMBOL to prevent linking failures when used in
standard loadable modules?

> +		__cmpwait_relaxed(ptr, val);				\

[Severity: High]
Could this inadvertently block when given a zero or negative timeout?

When looking at the generic wrapper in include/asm-generic/barrier.h,
smp_cond_load_relaxed_timeout executes cpu_poll_relax on the first iteration
before evaluating if the timeout has expired:

        cpu_poll_relax(__PTR, VAL, (u64)__timeout);
        if (++__count < __spin)
                continue;

If a caller provides a zero or negative timeout expecting an immediate
non-blocking poll, this falls through to __cmpwait_relaxed.

For negative timeouts, casting to u64 results in a huge unsigned value
(U64_MAX). Might this cause WFET to block indefinitely?

Even if WFET is unsupported, __cmpwait_relaxed uses WFE, which ignores the
timeout and blocks the CPU until the variable changes or an event stream
interrupt occurs. Does this introduce unintended latency for what should be a
zero-latency fast-path failure?

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

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