On Wed, 2024-11-06 at 11:08 +0000, Catalin Marinas wrote:

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



On Tue, Nov 05, 2024 at 12:30:37PM -0600, Haris Okanovic wrote:

quoted

diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index d4f581c1e21d..112027eabbfc 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h

@@ -256,6 +256,31 @@ do {                                                                     \
 })
 #endif

+/**
+ * smp_vcond_load_relaxed() - (Spin) wait until an expected value at address
+ * with no ordering guarantees. Spins until `(*addr & mask) == val` or
+ * `nsecs` elapse, and returns the last observed `*addr` value.
+ *
+ * @nsecs: timeout in nanoseconds

FWIW, I don't mind the relative timeout, it makes the API easier to use.
Yes, it may take longer in absolute time if the thread is scheduled out
before local_clock_noinstr() is read but the same can happen in the
caller anyway. It's similar to udelay(), it can take longer if the
thread is scheduled out.

quoted

+ * @addr: pointer to an integer
+ * @mask: a bit mask applied to read values
+ * @val: Expected value with mask
+ */
+#ifndef smp_vcond_load_relaxed
+#define smp_vcond_load_relaxed(nsecs, addr, mask, val) ({    \
+     const u64 __start = local_clock_noinstr();              \
+     u64 __nsecs = (nsecs);                                  \
+     typeof(addr) __addr = (addr);                           \
+     typeof(*__addr) __mask = (mask);                        \
+     typeof(*__addr) __val = (val);                          \
+     typeof(*__addr) __cur;                                  \
+     smp_cond_load_relaxed(__addr, (                         \
+             (VAL & __mask) == __val ||                      \
+             local_clock_noinstr() - __start > __nsecs       \
+     ));                                                     \
+})

The generic implementation has the same problem as Ankur's current
series. smp_cond_load_relaxed() can't wait on anything other than the
variable at __addr. If it goes into a WFE, there's nothing executed to
read the timer and check for progress. Any generic implementation of
such function would have to use cpu_relax() and polling.

How would the caller enter wfe()? Can you give a specific scenario that
you're concerned about?

This code already reduces to a relaxed poll, something like this:

start = clock();
while((READ_ONCE(*addr) & mask) != val && (clock() - start) < nsecs) {
  cpu_relax();
}

--
Catalin