Re: [PATCH v12 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] ACQUIRE semantics violation in `smp_cond_load_acquire_timeout()` on weakly-ordered architectures.
--

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

asm-generic: barrier: Add smp_cond_load_acquire_timeout()

This commit adds the acquire variant of smp_cond_load_relaxed_timeout().
It attempts to provide acquire ordering by reusing the relaxed variant and
adding a control dependency combined with smp_acquire__after_ctrl_dep().

> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index c56df9513a084..0ab26e98842c0 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h

[ ... ]

> +/**
> + * 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
> + * (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)		\
> +({									\
> +	__unqual_scalar_typeof(*(ptr)) VAL;				\
> +	VAL = smp_cond_load_relaxed_timeout(ptr, cond_expr,		\
> +					     time_expr_ns,		\
> +					     timeout_ns);		\
> +	if (cond_expr)							\
> +		barrier();						\
> +	smp_acquire__after_ctrl_dep();					\

[Severity: High]
Does this sequence correctly establish a control dependency for the caller's
subsequent instructions?

According to Documentation/memory-barriers.txt, control dependencies only
apply to stores executed strictly within the conditional clauses (the true
or false branches), not to code following the if statement. Since the
caller's subsequent stores reside outside this conditional block, they
appear unconditional to the CPU.

Furthermore, smp_acquire__after_ctrl_dep() typically falls back to
smp_rmb(). On weakly-ordered architectures like RISC-V, where smp_rmb() is
implemented as "fence r, r" and only enforces load-to-load ordering, could
the CPU speculatively execute subsequent stores before the final load of VAL
completes?

Could this violate the expected acquire semantics if the caller relies on
the barrier in the timeout or failure path?

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

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