locking/local_lock: fix _Generic() matching of local_trylock_t
authorVlastimil Babka <vbabka@suse.cz>
Wed, 23 Apr 2025 08:21:29 +0000 (10:21 +0200)
committerLinus Torvalds <torvalds@linux-foundation.org>
Wed, 23 Apr 2025 16:04:56 +0000 (09:04 -0700)
Michael Larabel reported [1] a nginx performance regression in v6.15-rc3
and bisected it to commit 51339d99c013 ("locking/local_lock, mm: replace
localtry_ helpers with local_trylock_t type")

The problem is the _Generic() usage with a default association that
masks the fact that "local_trylock_t *" association is not being
selected as expected.  Replacing the default with the only other
expected type "local_lock_t *" reveals the underlying problem:

  include/linux/local_lock_internal.h:174:26: error: ‘_Generic’ selector of type ‘__seg_gs local_lock_t *’ is not compatible with any association

The local_locki's are part of __percpu structures and thus the __percpu
attribute is needed to associate the type properly.  Add the attribute
and keep the default replaced to turn any further mismatches into
compile errors.

The failure to recognize local_try_lock_t in __local_lock_release()
means that a local_trylock[_irqsave]() operation will set tl->acquired
to 1 (there's no _Generic() part in the trylock code), but then
local_unlock[_irqrestore]() will not set tl->acquired back to 0, so
further trylock operations will always fail on the same cpu+lock, while
non-trylock operations continue to work - a lockdep_assert() is also not
being executed in the _Generic() part of local_lock() code.

This means consume_stock() and refill_stock() operations will fail
deterministically, resulting in taking the slow paths and worse
performance.

Fixes: 51339d99c013 ("locking/local_lock, mm: replace localtry_ helpers with local_trylock_t type")
Reported-by: Michael Larabel <Michael@phoronix.com>
Closes: https://www.phoronix.com/review/linux-615-nginx-regression/2 [1]
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
include/linux/local_lock_internal.h

index bf2bf40d7b18d0616b3d7ebfb1d1f8fc1490f7b2..8d5ac16a9b17941b3db8297b0c85383b2289eef1 100644 (file)
@@ -102,11 +102,11 @@ do {                                                              \
                l = (local_lock_t *)this_cpu_ptr(lock);                 \
                tl = (local_trylock_t *)l;                              \
                _Generic((lock),                                        \
-                       local_trylock_t *: ({                           \
+                       __percpu local_trylock_t *: ({                  \
                                lockdep_assert(tl->acquired == 0);      \
                                WRITE_ONCE(tl->acquired, 1);            \
                        }),                                             \
-                       default:(void)0);                               \
+                       __percpu local_lock_t *: (void)0);              \
                local_lock_acquire(l);                                  \
        } while (0)
 
@@ -171,11 +171,11 @@ do {                                                              \
                tl = (local_trylock_t *)l;                              \
                local_lock_release(l);                                  \
                _Generic((lock),                                        \
-                       local_trylock_t *: ({                           \
+                       __percpu local_trylock_t *: ({                  \
                                lockdep_assert(tl->acquired == 1);      \
                                WRITE_ONCE(tl->acquired, 0);            \
                        }),                                             \
-                       default:(void)0);                               \
+                       __percpu local_lock_t *: (void)0);              \
        } while (0)
 
 #define __local_unlock(lock)                                   \