locking/local_lock: fix _Generic() matching of local_trylock_t
Michael Larabel reported [1] a nginx performance regression in v6.15-rc3 and bisected it to commit51339d99c0("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:51339d99c0("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>
This commit is contained in:
committed by
Linus Torvalds
parent
0251ddbffb
commit
82efd569a8
@@ -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) \
|
||||
|
||||
Reference in New Issue
Block a user