The following commit has been merged into the locking/core branch of tip: Commit-ID: 068df05363b79f54241bd6bd612055b8c16c5964 Gitweb: https://git.kernel.org/tip/068df05363b79f54241bd6bd612055b8c16c5964 Author: Marco Elver <elver@xxxxxxxxxx> AuthorDate: Thu, 13 Aug 2020 18:38:59 +02:00 Committer: Paul E. McKenney <paulmck@xxxxxxxxxx> CommitterDate: Mon, 24 Aug 2020 15:10:24 -07:00 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@xxxxxxxxxx> Cc: Paul E. McKenney <paulmck@xxxxxxxxxx> Cc: Will Deacon <will@xxxxxxxxxx> Cc: Arnd Bergmann <arnd@xxxxxxxx> Cc: Daniel Axtens <dja@xxxxxxxxxx> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx> Cc: <linux-arch@xxxxxxxxxxxxxxx> Signed-off-by: Marco Elver <elver@xxxxxxxxxx> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx> --- include/asm-generic/bitops/instrumented-non-atomic.h | 30 +++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/include/asm-generic/bitops/instrumented-non-atomic.h b/include/asm-generic/bitops/instrumented-non-atomic.h index f86234c..37363d5 100644 --- a/include/asm-generic/bitops/instrumented-non-atomic.h +++ b/include/asm-generic/bitops/instrumented-non-atomic.h @@ -58,6 +58,30 @@ static inline void __change_bit(long nr, volatile unsigned long *addr) arch___change_bit(nr, addr); } +static inline void __instrument_read_write_bitop(long nr, volatile unsigned long *addr) +{ + if (IS_ENABLED(CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC)) { + /* + * We treat non-atomic read-write bitops a little more special. + * 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): + * 1. report read-modify-write races -> check read; + * 2. do not report races with marked readers, but do report + * races with unmarked readers -> check "atomic" write. + */ + kcsan_check_read(addr + BIT_WORD(nr), sizeof(long)); + /* + * Use generic write instrumentation, in case other sanitizers + * or tools are enabled alongside KCSAN. + */ + instrument_write(addr + BIT_WORD(nr), sizeof(long)); + } else { + instrument_read_write(addr + BIT_WORD(nr), sizeof(long)); + } +} + /** * __test_and_set_bit - Set a bit and return its old value * @nr: Bit to set @@ -68,7 +92,7 @@ static inline void __change_bit(long nr, volatile unsigned long *addr) */ static inline bool __test_and_set_bit(long nr, volatile unsigned long *addr) { - instrument_read_write(addr + BIT_WORD(nr), sizeof(long)); + __instrument_read_write_bitop(nr, addr); return arch___test_and_set_bit(nr, addr); } @@ -82,7 +106,7 @@ static inline bool __test_and_set_bit(long nr, volatile unsigned long *addr) */ static inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr) { - instrument_read_write(addr + BIT_WORD(nr), sizeof(long)); + __instrument_read_write_bitop(nr, addr); return arch___test_and_clear_bit(nr, addr); } @@ -96,7 +120,7 @@ static inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr) */ static inline bool __test_and_change_bit(long nr, volatile unsigned long *addr) { - instrument_read_write(addr + BIT_WORD(nr), sizeof(long)); + __instrument_read_write_bitop(nr, addr); return arch___test_and_change_bit(nr, addr); }
