Re: [PATCH v14 06/15] asm-generic: barrier: Add smp_cond_load_acquire_timeout()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] smp_cond_load_acquire_timeout() fails to provide LOAD->STORE ACQUIRE semantics when the condition is met, due to the loss of control dependency on the caller's subsequent instructions.
--

--- Patch [6]: [PATCH v14 06/15] asm-generic: barrier: Add smp_cond_load_acquire_timeout() ---
commit 012b1328d58cb0c51a4024152d82f79afc2a458d
Author: Ankur Arora <ankur.a.arora@oracle.com>

asm-generic: barrier: Add smp_cond_load_acquire_timeout()

This commit adds an acquire variant of smp_cond_load_relaxed_timeout(),
reusing the relaxed variant and attempting to provide acquire semantics by
re-evaluating the condition and applying smp_acquire__after_ctrl_dep().

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

[ ... ]

> +#ifndef smp_cond_load_acquire_timeout
> +#define smp_cond_load_acquire_timeout(ptr, cond_expr,			\
> +				      time_expr_ns, timeout_ns)		\
> +({									\
> +	__unqual_scalar_typeof(*(ptr)) VAL;				\
> +	VAL = smp_cond_load_relaxed_timeout(ptr, cond_expr,		\
> +					     time_expr_ns,		\
> +					     timeout_ns);		\

[ ... ]

> +	/*								\
> +	 * We arrive here once the loop condition is hit, on timeout,	\
> +	 * or, if we hit both the timeout and the loop condition.	\
> +	 *								\
> +	 * The last case is low probability, but possible in the last	\
> +	 * iteration, especially on architectures with waiting		\
> +	 * cpu_poll_relax() implementations (ex. arm64).		\
> +	 * Now since the loop condition is not evaluated on timeout,	\
> +	 * we have a missed control dependency.				\
> +	 *								\
> +	 * So, force a re-evaluation of the control dependency to	\
> +	 * provide an ACQUIRE ordering for that case as well.		\
> +	 */								\
> +	if (cond_expr)							\
> +		smp_acquire__after_ctrl_dep();				\

[Severity: High]
Does this standalone if block fail to extend a control dependency to the
macro caller's subsequent code?

Control dependencies apply only to the then or else clauses of an if
statement, and do not extend to code following it. Because the caller's
subsequent instructions execute unconditionally after this if block, it
appears no control dependency is established for them.

On weakly ordered architectures (like RISC-V and LoongArch) where
smp_acquire__after_ctrl_dep() defaults to a read barrier like smp_rmb(),
this relies entirely on the control dependency for load-store ordering.
Without a structural control dependency on the caller's code, could the CPU
reorder the caller's stores before the macro's condition load completes,
breaking the required acquire semantics?

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

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