Re: [PATCH v15 04/16] arm64: support WFET in 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] WFET can miss wakeups and cause large latency spikes if the event stream is disabled, because cpu_poll_relax() bypasses the event stream check.
- [Medium] Missing EXPORT_SYMBOL for __delay_cycles breaks loadable modules using smp_cond_load_relaxed_timeout().
--

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

arm64: support WFET in smp_cond_load_relaxed_timeout()

This commit introduces support for using the WFET instruction when a timeout
is provided to smp_cond_load_relaxed_timeout(). It achieves this by adding a
new __cmpwait_relaxed_timeout() helper and underlying
__cmpwait_case_timeout_##sz macros.

> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index d186a45587766..d1f4f571adaa8 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h

[ ... ]

> @@ -233,7 +233,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: High]
Does bypassing the arch_timer_evtstrm_available() check for WFET risk missed
wakeups and large latency spikes if the event stream is disabled?

If a task executes ldxr in __cmpwait_case_timeout_##sz(), is preempted
by an interrupt that clears the exclusive monitor, and then another task
executes WFE (clearing the Event register), the original task resuming and
executing WFET could sleep for the entire timeout duration.

Since the monitor is Open (cleared), a store to the lock word by another CPU
would not generate an event. The arch_timer_evtstrm_available() check helps
prevent this exact issue by providing periodic wakeups.

>  		__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..7985ae5ceb0f9 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: Medium]
Will this break the build for loadable modules using this API?

Since __delay_cycles() lacks an EXPORT_SYMBOL() declaration, any loadable
module calling smp_cond_load_relaxed_timeout() will expand this inline
function and encounter linker errors.

> +	asm volatile(							\
> +	"	sevl\n"							\
> +	"	wfe\n"							\
> +	"	ldxr" #sfx "\t%" #w "[tmp], %[v]\n"			\
> +	"	eor	%" #w "[tmp], %" #w "[tmp], %" #w "[val]\n"	\
> +	"	cbnz	%" #w "[tmp], 2f\n"				\
> +	"	msr s0_3_c1_c0_0, %[ecycles]\n"				\
> +	"2:"								\
> +	: [tmp] "=&r" (tmp), [v] "+Q" (*(u##sz *)ptr)			\
> +	: [val] "r" (val), [ecycles] "r" (ecycles));			\
> +}

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