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

[sashiko-bot@kernel.org]

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