Skip to content

Commit 068df05

Browse files
melverpaulmckrcu
authored andcommitted
bitops, kcsan: Partially revert instrumentation for non-atomic bitops
Previous to the change to distinguish read-write accesses, when CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=y is set, KCSAN would consider the non-atomic bitops as atomic. We want to partially revert to this behaviour, but with one important distinction: report racing modifications, since lost bits due to non-atomicity are certainly possible. Given the operations here only modify a single bit, assuming non-atomicity of the writer is sufficient may be reasonable for certain usage (and follows the permissible nature of the "assume plain writes atomic" rule). In other words: 1. We want non-atomic read-modify-write races to be reported; this is accomplished by kcsan_check_read(), where any concurrent write (atomic or not) will generate a report. 2. We do not want to report races with marked readers, but -do- want to report races with unmarked readers; this is accomplished by the instrument_write() ("assume atomic write" with Kconfig option set). With the above rules, when KCSAN_ASSUME_PLAIN_WRITES_ATOMIC is selected, it is hoped that KCSAN's reporting behaviour is better aligned with current expected permissible usage for non-atomic bitops. Note that, a side-effect of not telling KCSAN that the accesses are read-writes, is that this information is not displayed in the access summary in the report. It is, however, visible in inline-expanded stack traces. For now, it does not make sense to introduce yet another special case to KCSAN's runtime, only to cater to the case here. Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Paul E. McKenney <paulmck@kernel.org> Cc: Will Deacon <will@kernel.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Daniel Axtens <dja@axtens.net> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: <linux-arch@vger.kernel.org> Signed-off-by: Marco Elver <elver@google.com> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
1 parent 2e986b8 commit 068df05

1 file changed

Lines changed: 27 additions & 3 deletions

File tree

include/asm-generic/bitops/instrumented-non-atomic.h

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,30 @@ static inline void __change_bit(long nr, volatile unsigned long *addr)
5858
arch___change_bit(nr, addr);
5959
}
6060

61+
static inline void __instrument_read_write_bitop(long nr, volatile unsigned long *addr)
62+
{
63+
if (IS_ENABLED(CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC)) {
64+
/*
65+
* We treat non-atomic read-write bitops a little more special.
66+
* Given the operations here only modify a single bit, assuming
67+
* non-atomicity of the writer is sufficient may be reasonable
68+
* for certain usage (and follows the permissible nature of the
69+
* assume-plain-writes-atomic rule):
70+
* 1. report read-modify-write races -> check read;
71+
* 2. do not report races with marked readers, but do report
72+
* races with unmarked readers -> check "atomic" write.
73+
*/
74+
kcsan_check_read(addr + BIT_WORD(nr), sizeof(long));
75+
/*
76+
* Use generic write instrumentation, in case other sanitizers
77+
* or tools are enabled alongside KCSAN.
78+
*/
79+
instrument_write(addr + BIT_WORD(nr), sizeof(long));
80+
} else {
81+
instrument_read_write(addr + BIT_WORD(nr), sizeof(long));
82+
}
83+
}
84+
6185
/**
6286
* __test_and_set_bit - Set a bit and return its old value
6387
* @nr: Bit to set
@@ -68,7 +92,7 @@ static inline void __change_bit(long nr, volatile unsigned long *addr)
6892
*/
6993
static inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
7094
{
71-
instrument_read_write(addr + BIT_WORD(nr), sizeof(long));
95+
__instrument_read_write_bitop(nr, addr);
7296
return arch___test_and_set_bit(nr, addr);
7397
}
7498

@@ -82,7 +106,7 @@ static inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
82106
*/
83107
static inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
84108
{
85-
instrument_read_write(addr + BIT_WORD(nr), sizeof(long));
109+
__instrument_read_write_bitop(nr, addr);
86110
return arch___test_and_clear_bit(nr, addr);
87111
}
88112

@@ -96,7 +120,7 @@ static inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
96120
*/
97121
static inline bool __test_and_change_bit(long nr, volatile unsigned long *addr)
98122
{
99-
instrument_read_write(addr + BIT_WORD(nr), sizeof(long));
123+
__instrument_read_write_bitop(nr, addr);
100124
return arch___test_and_change_bit(nr, addr);
101125
}
102126

0 commit comments

Comments
 (0)