Re: [RESEND PATCH v7 2/7] arm64: barrier: Support smp_cond_load_relaxed_timeout()

[Ankur Arora <ankur.a.arora@oracle.com>]

Arnd Bergmann <arnd@arndb.de> writes:

> On Wed, Nov 5, 2025, at 09:27, Ankur Arora wrote:
>> Catalin Marinas <catalin.marinas@arm.com> writes:
>>> On Mon, Nov 03, 2025 at 01:00:33PM -0800, Ankur Arora wrote:
>>> With time_end_ns being passed to cpu_poll_relax(), we assume that this
>>> is always the absolute time. Do we still need time_expr in this case?
>>> It works for WFET as long as we can map this time_end_ns onto the
>>> hardware CNTVCT.
>>>
>>> Alternatively, we could pass something like remaining_ns, though not
>>> sure how smp_cond_load_relaxed_timeout() can decide to spin before
>>> checking time_expr again (we probably went over this in the past two
>>> years ;)).
>>
>> I'm sure it is in there somewhere :).
>> This one?: https://lore.kernel.org/lkml/aJy414YufthzC1nv@arm.com/.
>> Though the whole wait_policy thing confused the issue somewhat there.
>>
>> Though that problem exists for both remaining_ns and for time_end_ns
>> with WFE. I think we are fine so long as SMP_TIMEOUT_POLL_COUNT is
>> defined to be 1.
>
> We need to be careful with the definition of the time_expr() if
> cpu_poll_relax requires the absolute time in CNTVCT domain.

True. The absolute time assumes that CPU time and CNTVCT domain
times are freely translatable, and won't drift.

> We're probably fine here, but it feels like a bit of a layering
> violation. I think passing relative time into it would be cleaner
> because it avoids the ambiguity it

I'll play around a little; see if we can pass the relative time and
yet not depend on the conjoined value of SMP_TIMEOUT_POLL_COUNT.

> but probably requires an extra
> access to the timer register that is hopefully fast on arm64.
>
> I'm ok with either way.

I'll keep the caller parameter to be remaining_ns. This way we
can internally switch over to relative/absolute time if needed.

>> For now, I think it makes sense to always pass the absolute deadline
>> even if the caller uses remaining_ns. So:
>>
>> #define smp_cond_load_relaxed_timeout(ptr, cond_expr, time_expr, remaining_ns)	\
>> ({									\
>> 	typeof(ptr) __PTR = (ptr);					\
>> 	__unqual_scalar_typeof(*ptr) VAL;				\
>> 	u32 __n = 0, __spin = SMP_TIMEOUT_POLL_COUNT;			\
>> 	u64 __time_start_ns = (time_expr);				\
>> 	s64 __time_end_ns = __time_start_ns + (remaining_ns);		\
>> 									\
>> 	for (;;) {							\
>> 		VAL = READ_ONCE(*__PTR);				\
>> 		if (cond_expr)						\
>> 			break;						\
>> 		cpu_poll_relax(__PTR, VAL, __time_end_ns);		\
>
> This looks perfectly fine to me, thanks for the update!

Thanks for the review comments!

--
ankur