bit_spinlock: Support Clang's context analysis

The annotations for bit_spinlock.h have simply been using "bitlock" as
the token. For Sparse, that was likely sufficient in most cases. But
Clang's context analysis is more precise, and we need to ensure we
can distinguish different bitlocks.

To do so, add a token context, and a macro __bitlock(bitnum, addr)
that is used to construct unique per-bitlock tokens.

Add the appropriate test.

<linux/list_bl.h> is implicitly included through other includes, and
requires 2 annotations to indicate that acquisition (without release)
and release (without prior acquisition) of its bitlock is intended.

Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20251219154418.3592607-14-elver@google.com

authored by

Marco Elver and committed by

Peter Zijlstra 5 months ago eb7d96a1 5f7ba059

+48 -5

4 changed files

expand all

Documentation

dev-tools

context-analysis.rst

include

linux

bit_spinlock.h

list_bl.h

lib

test_context-analysis.c

+2 -1

Documentation/dev-tools/context-analysis.rst

··· 79 79 ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 80 80 81 81 Currently the following synchronization primitives are supported: 82 - `raw_spinlock_t`, `spinlock_t`, `rwlock_t`, `mutex`, `seqlock_t`. 82 + `raw_spinlock_t`, `spinlock_t`, `rwlock_t`, `mutex`, `seqlock_t`, 83 + `bit_spinlock`. 83 84 84 85 For context locks with an initialization function (e.g., `spin_lock_init()`), 85 86 calling this function before initializing any guarded members or globals

+18 -4

include/linux/bit_spinlock.h

··· 10 10 #include <asm/processor.h> /* for cpu_relax() */ 11 11 12 12 /* 13 + * For static context analysis, we need a unique token for each possible bit 14 + * that can be used as a bit_spinlock. The easiest way to do that is to create a 15 + * fake context that we can cast to with the __bitlock(bitnum, addr) macro 16 + * below, which will give us unique instances for each (bit, addr) pair that the 17 + * static analysis can use. 18 + */ 19 + context_lock_struct(__context_bitlock) { }; 20 + #define __bitlock(bitnum, addr) (struct __context_bitlock *)(bitnum + (addr)) 21 + 22 + /* 13 23 * bit-based spin_lock() 14 24 * 15 25 * Don't use this unless you really need to: spin_lock() and spin_unlock() 16 26 * are significantly faster. 17 27 */ 18 28 static __always_inline void bit_spin_lock(int bitnum, unsigned long *addr) 29 + __acquires(__bitlock(bitnum, addr)) 19 30 { 20 31 /* 21 32 * Assuming the lock is uncontended, this never enters ··· 45 34 preempt_disable(); 46 35 } 47 36 #endif 48 - __acquire(bitlock); 37 + __acquire(__bitlock(bitnum, addr)); 49 38 } 50 39 51 40 /* 52 41 * Return true if it was acquired 53 42 */ 54 43 static __always_inline int bit_spin_trylock(int bitnum, unsigned long *addr) 44 + __cond_acquires(true, __bitlock(bitnum, addr)) 55 45 { 56 46 preempt_disable(); 57 47 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK) ··· 61 49 return 0; 62 50 } 63 51 #endif 64 - __acquire(bitlock); 52 + __acquire(__bitlock(bitnum, addr)); 65 53 return 1; 66 54 } 67 55 ··· 69 57 * bit-based spin_unlock() 70 58 */ 71 59 static __always_inline void bit_spin_unlock(int bitnum, unsigned long *addr) 60 + __releases(__bitlock(bitnum, addr)) 72 61 { 73 62 #ifdef CONFIG_DEBUG_SPINLOCK 74 63 BUG_ON(!test_bit(bitnum, addr)); ··· 78 65 clear_bit_unlock(bitnum, addr); 79 66 #endif 80 67 preempt_enable(); 81 - __release(bitlock); 68 + __release(__bitlock(bitnum, addr)); 82 69 } 83 70 84 71 /* ··· 87 74 * protecting the rest of the flags in the word. 88 75 */ 89 76 static __always_inline void __bit_spin_unlock(int bitnum, unsigned long *addr) 77 + __releases(__bitlock(bitnum, addr)) 90 78 { 91 79 #ifdef CONFIG_DEBUG_SPINLOCK 92 80 BUG_ON(!test_bit(bitnum, addr)); ··· 96 82 __clear_bit_unlock(bitnum, addr); 97 83 #endif 98 84 preempt_enable(); 99 - __release(bitlock); 85 + __release(__bitlock(bitnum, addr)); 100 86 } 101 87 102 88 /*

include/linux/list_bl.h

··· 144 144 } 145 145 146 146 static inline void hlist_bl_lock(struct hlist_bl_head *b) 147 + __acquires(__bitlock(0, b)) 147 148 { 148 149 bit_spin_lock(0, (unsigned long *)b); 149 150 } 150 151 151 152 static inline void hlist_bl_unlock(struct hlist_bl_head *b) 153 + __releases(__bitlock(0, b)) 152 154 { 153 155 __bit_spin_unlock(0, (unsigned long *)b); 154 156 }

+26

lib/test_context-analysis.c

··· 4 4 * positive errors when compiled with Clang's context analysis. 5 5 */ 6 6 7 + #include <linux/bit_spinlock.h> 7 8 #include <linux/build_bug.h> 8 9 #include <linux/mutex.h> 9 10 #include <linux/seqlock.h> ··· 257 256 { 258 257 scoped_seqlock_read (&d->sl, ss_lockless) { 259 258 (void)d->counter; 259 + } 260 + } 261 + 262 + struct test_bit_spinlock_data { 263 + unsigned long bits; 264 + int counter __guarded_by(__bitlock(3, &bits)); 265 + }; 266 + 267 + static void __used test_bit_spin_lock(struct test_bit_spinlock_data *d) 268 + { 269 + /* 270 + * Note, the analysis seems to have false negatives, because it won't 271 + * precisely recognize the bit of the fake __bitlock() token. 272 + */ 273 + bit_spin_lock(3, &d->bits); 274 + d->counter++; 275 + bit_spin_unlock(3, &d->bits); 276 + 277 + bit_spin_lock(3, &d->bits); 278 + d->counter++; 279 + __bit_spin_unlock(3, &d->bits); 280 + 281 + if (bit_spin_trylock(3, &d->bits)) { 282 + d->counter++; 283 + bit_spin_unlock(3, &d->bits); 260 284 } 261 285 }

Configure Feed

Configure Feed