On Fri, 23 Aug 2024 at 10:52, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> The wake_up_var() infrastructure that the inode code uses is a bit
> more involved. Not only can the variable be anything at all (so the
> operations you can do on it are obviously largely unbounded), but the
> inode hack in particular then uses one thing for the actual variable,
> and another thing for the address that is used to match up waits and
> wakeups.
.. btw, that doesn't mean we can't have helpers for the common cases.
They might have to be macros (so that they just work regardless of the
type), but having a
set_var_and_wake(var, value);
macro that just expands to something like (completely untested "maybe
this works" macro):
#define set_var_and_wake(var,value) do { \
__auto_type __set_ptr = &(var); \
*(__set_ptr) = (value); \
smp_mb(); \
wake_up_var(__set_ptr); \
} while (0)
doesn't sound too bad for at least some common patterns.
Looking around, we do seem to have a pattern of
smp_store_release() -> wake_up_var()
instead of a memory barrier. I don't think that actually works. The
smp_store_release() means that *earlier* accesses will be bounded by
the store operation, but *later* accesses - including very much the
"look if the wait queue is empty" check - are totally unordered by it,
and can be done before the store by the CPU.
But I haven't thought deeply about it, that was just my gut reaction
when seeing the pattern. It superficially makes sense, but I think
it's entirely wrong (it's a "smp_load_acquire()" that would order with
later accesses, but there is no "store with acquire semantics"
operation).
Linus
