While investigating a number of data races, we've encountered data-racy
accesses on flags variables to be very common. The typical pattern is a
reader masking all but one bit, and the writer setting/clearing only 1
bit (current->flags being a frequently encountered case; mm/sl[au]b.c
disables KCSAN for this reason currently).
Since these types of "trivial" data races are common (assuming they're
intentional and hard to miscompile!), having the option to filter them
(like we currently do for other types of data races) will avoid forcing
everyone to mark them, and deliberately left to preference at this time.
The primary motivation is to move closer towards more easily filtering
interesting data races (like [1], [2], [3]) on CI systems (e.g. syzbot),
without the churn to mark all such "trivial" data races.
[1] https://lkml.kernel.org/r/[email protected]
[2] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
Notably, the need for further built-in filtering has become clearer as
we notice some other CI systems (without active moderation) trying to
employ KCSAN, but usually have to turn it down quickly because their
reports are quickly met with negative feedback:
https://lkml.kernel.org/r/YHSPfiJ/h/[email protected]
The rules are implemented and guarded by a new option
CONFIG_KCSAN_PERMISSIVE. With it, we will ignore data races with only
1-bit value changes. Please see more details in in patch 7/7.
The rest of the patches are cleanups and improving configuration.
I ran some experiments to see what data races we're left with. With
CONFIG_KCSAN_PERMISSIVE=y paired with syzbot's current KCSAN config
(minimal kernel, most permissive KCSAN options), we're "just" about ~100
reports away to a pretty silent KCSAN kernel:
https://github.com/google/ktsan/tree/kcsan-permissive-with-dataraces
[ !!Disclaimer!! None of the commits are usable patches nor guaranteed
to be correct -- they merely resolve a data race so it wouldn't be
shown again and then moved on. Expect that simply marking is not
enough for some! ]
Most of the data races look interesting enough, and only few already had
a comment nearby explaining what's happening.
All data races on current->flags, and most other flags are absent
(unlike before). Those that were reported all had value changes with >1
bit. A limitation is that few data races are still reported where the
reader is only interested in 1 bit but the writer changed more than 1
bit. A complete approach would require compiler changes in addition to
the changes in this series -- but since that would further reduce the
data races reported, the simpler and conservative approach is to stick
to the value-change based rules for now.
Marco Elver (7):
kcsan: Improve some Kconfig comments
kcsan: Remove CONFIG_KCSAN_DEBUG
kcsan: Introduce CONFIG_KCSAN_STRICT
kcsan: Reduce get_ctx() uses in kcsan_found_watchpoint()
kcsan: Rework atomic.h into permissive.h
kcsan: Print if strict or non-strict during init
kcsan: permissive: Ignore data-racy 1-bit value changes
Documentation/dev-tools/kcsan.rst | 12 ++++
kernel/kcsan/atomic.h | 23 --------
kernel/kcsan/core.c | 77 ++++++++++++++++---------
kernel/kcsan/kcsan_test.c | 32 +++++++++++
kernel/kcsan/permissive.h | 94 +++++++++++++++++++++++++++++++
lib/Kconfig.kcsan | 39 +++++++++----
6 files changed, 215 insertions(+), 62 deletions(-)
delete mode 100644 kernel/kcsan/atomic.h
create mode 100644 kernel/kcsan/permissive.h
--
2.32.0.rc1.229.g3e70b5a671-goog
By this point CONFIG_KCSAN_DEBUG is pretty useless, as the system just
isn't usable with it due to spamming console (I imagine a randconfig
test robot will run into this sooner or later). Remove it.
Back in 2019 I used it occasionally to record traces of watchpoints and
verify the encoding is correct, but these days we have proper tests. If
something similar is needed in future, just add it back ad-hoc.
Signed-off-by: Marco Elver <[email protected]>
---
kernel/kcsan/core.c | 9 ---------
lib/Kconfig.kcsan | 3 ---
2 files changed, 12 deletions(-)
diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
index 26709ea65c71..d92977ede7e1 100644
--- a/kernel/kcsan/core.c
+++ b/kernel/kcsan/core.c
@@ -479,15 +479,6 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
break; /* ignore; we do not diff the values */
}
- if (IS_ENABLED(CONFIG_KCSAN_DEBUG)) {
- kcsan_disable_current();
- pr_err("watching %s, size: %zu, addr: %px [slot: %d, encoded: %lx]\n",
- is_write ? "write" : "read", size, ptr,
- watchpoint_slot((unsigned long)ptr),
- encode_watchpoint((unsigned long)ptr, size, is_write));
- kcsan_enable_current();
- }
-
/*
* Delay this thread, to increase probability of observing a racy
* conflicting access.
diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index 6152fbd5cbb4..5304f211f81f 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -62,9 +62,6 @@ config KCSAN_VERBOSE
generated from any one of them, system stability may suffer due to
deadlocks or recursion. If in doubt, say N.
-config KCSAN_DEBUG
- bool "Debugging of KCSAN internals"
-
config KCSAN_SELFTEST
bool "Perform short selftests on boot"
default y
--
2.32.0.rc1.229.g3e70b5a671-goog
Rework atomic.h into permissive.h to better reflect its purpose, and
introduce kcsan_ignore_address() and kcsan_ignore_data_race().
Introduce CONFIG_KCSAN_PERMISSIVE and update the stub functions in
preparation for subsequent changes.
As before, developers who choose to use KCSAN in "strict" mode will see
all data races and are not affected. Furthermore, by relying on the
value-change filter logic for kcsan_ignore_data_race(), even if the
permissive rules are enabled, the opt-outs in report.c:skip_report()
override them (such as for RCU-related functions by default).
The option CONFIG_KCSAN_PERMISSIVE is disabled by default, so that the
documented default behaviour of KCSAN does not change. Instead, like
CONFIG_KCSAN_IGNORE_ATOMICS, the option needs to be explicitly opted in.
Signed-off-by: Marco Elver <[email protected]>
---
Documentation/dev-tools/kcsan.rst | 8 ++++++
kernel/kcsan/atomic.h | 23 ---------------
kernel/kcsan/core.c | 33 ++++++++++++++++------
kernel/kcsan/permissive.h | 47 +++++++++++++++++++++++++++++++
lib/Kconfig.kcsan | 10 +++++++
5 files changed, 89 insertions(+), 32 deletions(-)
delete mode 100644 kernel/kcsan/atomic.h
create mode 100644 kernel/kcsan/permissive.h
diff --git a/Documentation/dev-tools/kcsan.rst b/Documentation/dev-tools/kcsan.rst
index 17f974213b88..9df98a48e69d 100644
--- a/Documentation/dev-tools/kcsan.rst
+++ b/Documentation/dev-tools/kcsan.rst
@@ -124,6 +124,14 @@ Kconfig options:
causes KCSAN to not report data races due to conflicts where the only plain
accesses are aligned writes up to word size.
+* ``CONFIG_KCSAN_PERMISSIVE``: Enable additional permissive rules to ignore
+ certain classes of common data races. Unlike the above, the rules are more
+ complex involving value-change patterns, access type, and address. This
+ option depends on ``CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=y``. For details
+ please see the ``kernel/kcsan/permissive.h``. Testers and maintainers that
+ only focus on reports from specific subsystems and not the whole kernel are
+ recommended to disable this option.
+
To use the strictest possible rules, select ``CONFIG_KCSAN_STRICT=y``, which
configures KCSAN to follow the Linux-kernel memory consistency model (LKMM) as
closely as possible.
diff --git a/kernel/kcsan/atomic.h b/kernel/kcsan/atomic.h
deleted file mode 100644
index 530ae1bda8e7..000000000000
--- a/kernel/kcsan/atomic.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Rules for implicitly atomic memory accesses.
- *
- * Copyright (C) 2019, Google LLC.
- */
-
-#ifndef _KERNEL_KCSAN_ATOMIC_H
-#define _KERNEL_KCSAN_ATOMIC_H
-
-#include <linux/types.h>
-
-/*
- * Special rules for certain memory where concurrent conflicting accesses are
- * common, however, the current convention is to not mark them; returns true if
- * access to @ptr should be considered atomic. Called from slow-path.
- */
-static bool kcsan_is_atomic_special(const volatile void *ptr)
-{
- return false;
-}
-
-#endif /* _KERNEL_KCSAN_ATOMIC_H */
diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
index 906100923b88..439edb9dcbb1 100644
--- a/kernel/kcsan/core.c
+++ b/kernel/kcsan/core.c
@@ -20,9 +20,9 @@
#include <linux/sched.h>
#include <linux/uaccess.h>
-#include "atomic.h"
#include "encoding.h"
#include "kcsan.h"
+#include "permissive.h"
static bool kcsan_early_enable = IS_ENABLED(CONFIG_KCSAN_EARLY_ENABLE);
unsigned int kcsan_udelay_task = CONFIG_KCSAN_UDELAY_TASK;
@@ -353,6 +353,7 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
atomic_long_t *watchpoint,
long encoded_watchpoint)
{
+ const bool is_assert = (type & KCSAN_ACCESS_ASSERT) != 0;
struct kcsan_ctx *ctx = get_ctx();
unsigned long flags;
bool consumed;
@@ -374,6 +375,16 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
if (ctx->access_mask)
return;
+ /*
+ * If the other thread does not want to ignore the access, and there was
+ * a value change as a result of this thread's operation, we will still
+ * generate a report of unknown origin.
+ *
+ * Use CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN=n to filter.
+ */
+ if (!is_assert && kcsan_ignore_address(ptr))
+ return;
+
/*
* Consuming the watchpoint must be guarded by kcsan_is_enabled() to
* avoid erroneously triggering reports if the context is disabled.
@@ -396,7 +407,7 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_REPORT_RACES]);
}
- if ((type & KCSAN_ACCESS_ASSERT) != 0)
+ if (is_assert)
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_ASSERT_FAILURES]);
else
atomic_long_inc(&kcsan_counters[KCSAN_COUNTER_DATA_RACES]);
@@ -427,12 +438,10 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
goto out;
/*
- * Special atomic rules: unlikely to be true, so we check them here in
- * the slow-path, and not in the fast-path in is_atomic(). Call after
- * kcsan_is_enabled(), as we may access memory that is not yet
- * initialized during early boot.
+ * Check to-ignore addresses after kcsan_is_enabled(), as we may access
+ * memory that is not yet initialized during early boot.
*/
- if (!is_assert && kcsan_is_atomic_special(ptr))
+ if (!is_assert && kcsan_ignore_address(ptr))
goto out;
if (!check_encodable((unsigned long)ptr, size)) {
@@ -518,8 +527,14 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
if (access_mask)
diff &= access_mask;
- /* Were we able to observe a value-change? */
- if (diff != 0)
+ /*
+ * Check if we observed a value change.
+ *
+ * Also check if the data race should be ignored (the rules depend on
+ * non-zero diff); if it is to be ignored, the below rules for
+ * KCSAN_VALUE_CHANGE_MAYBE apply.
+ */
+ if (diff && !kcsan_ignore_data_race(size, type, old, new, diff))
value_change = KCSAN_VALUE_CHANGE_TRUE;
/* Check if this access raced with another. */
diff --git a/kernel/kcsan/permissive.h b/kernel/kcsan/permissive.h
new file mode 100644
index 000000000000..f90e30800c11
--- /dev/null
+++ b/kernel/kcsan/permissive.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Special rules for ignoring entire classes of data-racy memory accesses. None
+ * of the rules here imply that such data races are generally safe!
+ *
+ * All rules in this file can be configured via CONFIG_KCSAN_PERMISSIVE. Keep
+ * them separate from core code to make it easier to audit.
+ *
+ * Copyright (C) 2019, Google LLC.
+ */
+
+#ifndef _KERNEL_KCSAN_PERMISSIVE_H
+#define _KERNEL_KCSAN_PERMISSIVE_H
+
+#include <linux/types.h>
+
+/*
+ * Access ignore rules based on address.
+ */
+static __always_inline bool kcsan_ignore_address(const volatile void *ptr)
+{
+ if (!IS_ENABLED(CONFIG_KCSAN_PERMISSIVE))
+ return false;
+
+ return false;
+}
+
+/*
+ * Data race ignore rules based on access type and value change patterns.
+ */
+static bool
+kcsan_ignore_data_race(size_t size, int type, u64 old, u64 new, u64 diff)
+{
+ if (!IS_ENABLED(CONFIG_KCSAN_PERMISSIVE))
+ return false;
+
+ /*
+ * Rules here are only for plain read accesses, so that we still report
+ * data races between plain read-write accesses.
+ */
+ if (type || size > sizeof(long))
+ return false;
+
+ return false;
+}
+
+#endif /* _KERNEL_KCSAN_PERMISSIVE_H */
diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index c76fbb3ee09e..26f03c754d39 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -231,4 +231,14 @@ config KCSAN_IGNORE_ATOMICS
due to two conflicting plain writes will be reported (aligned and
unaligned, if CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=n).
+config KCSAN_PERMISSIVE
+ bool "Enable all additional permissive rules"
+ depends on KCSAN_REPORT_VALUE_CHANGE_ONLY
+ help
+ Enable additional permissive rules to ignore certain classes of data
+ races (also see kernel/kcsan/permissive.h). None of the permissive
+ rules imply that such data races are generally safe, but can be used
+ to further reduce reported data races due to data-racy patterns
+ common across the kernel.
+
endif # KCSAN
--
2.32.0.rc1.229.g3e70b5a671-goog
Hi Marco,
On Mon, Jun 07, 2021 at 02:56:46PM +0200, Marco Elver wrote:
> While investigating a number of data races, we've encountered data-racy
> accesses on flags variables to be very common. The typical pattern is a
> reader masking all but one bit, and the writer setting/clearing only 1
> bit (current->flags being a frequently encountered case; mm/sl[au]b.c
> disables KCSAN for this reason currently).
As a heads up, I just sent out the series I promised for
thread_info::flags, at:
https://lore.kernel.org/lkml/[email protected]/T/#t
... which I think is complementary to this (IIUC it should help with the
multi-bit cases you mention below), and may help to make the checks more
stringent in future.
FWIW, for this series:
Acked-by: Mark Rutland <[email protected]>
Thanks,
Mark.
> Since these types of "trivial" data races are common (assuming they're
> intentional and hard to miscompile!), having the option to filter them
> (like we currently do for other types of data races) will avoid forcing
> everyone to mark them, and deliberately left to preference at this time.
>
> The primary motivation is to move closer towards more easily filtering
> interesting data races (like [1], [2], [3]) on CI systems (e.g. syzbot),
> without the churn to mark all such "trivial" data races.
> [1] https://lkml.kernel.org/r/[email protected]
> [2] https://lkml.kernel.org/r/[email protected]
> [3] https://lkml.kernel.org/r/[email protected]
>
> Notably, the need for further built-in filtering has become clearer as
> we notice some other CI systems (without active moderation) trying to
> employ KCSAN, but usually have to turn it down quickly because their
> reports are quickly met with negative feedback:
> https://lkml.kernel.org/r/YHSPfiJ/h/[email protected]
>
> The rules are implemented and guarded by a new option
> CONFIG_KCSAN_PERMISSIVE. With it, we will ignore data races with only
> 1-bit value changes. Please see more details in in patch 7/7.
>
> The rest of the patches are cleanups and improving configuration.
>
> I ran some experiments to see what data races we're left with. With
> CONFIG_KCSAN_PERMISSIVE=y paired with syzbot's current KCSAN config
> (minimal kernel, most permissive KCSAN options), we're "just" about ~100
> reports away to a pretty silent KCSAN kernel:
>
> https://github.com/google/ktsan/tree/kcsan-permissive-with-dataraces
> [ !!Disclaimer!! None of the commits are usable patches nor guaranteed
> to be correct -- they merely resolve a data race so it wouldn't be
> shown again and then moved on. Expect that simply marking is not
> enough for some! ]
>
> Most of the data races look interesting enough, and only few already had
> a comment nearby explaining what's happening.
>
> All data races on current->flags, and most other flags are absent
> (unlike before). Those that were reported all had value changes with >1
> bit. A limitation is that few data races are still reported where the
> reader is only interested in 1 bit but the writer changed more than 1
> bit. A complete approach would require compiler changes in addition to
> the changes in this series -- but since that would further reduce the
> data races reported, the simpler and conservative approach is to stick
> to the value-change based rules for now.
>
> Marco Elver (7):
> kcsan: Improve some Kconfig comments
> kcsan: Remove CONFIG_KCSAN_DEBUG
> kcsan: Introduce CONFIG_KCSAN_STRICT
> kcsan: Reduce get_ctx() uses in kcsan_found_watchpoint()
> kcsan: Rework atomic.h into permissive.h
> kcsan: Print if strict or non-strict during init
> kcsan: permissive: Ignore data-racy 1-bit value changes
>
> Documentation/dev-tools/kcsan.rst | 12 ++++
> kernel/kcsan/atomic.h | 23 --------
> kernel/kcsan/core.c | 77 ++++++++++++++++---------
> kernel/kcsan/kcsan_test.c | 32 +++++++++++
> kernel/kcsan/permissive.h | 94 +++++++++++++++++++++++++++++++
> lib/Kconfig.kcsan | 39 +++++++++----
> 6 files changed, 215 insertions(+), 62 deletions(-)
> delete mode 100644 kernel/kcsan/atomic.h
> create mode 100644 kernel/kcsan/permissive.h
>
> --
> 2.32.0.rc1.229.g3e70b5a671-goog
>
On Wed, 9 Jun 2021 at 14:38, Mark Rutland <[email protected]> wrote:
> Hi Marco,
>
> On Mon, Jun 07, 2021 at 02:56:46PM +0200, Marco Elver wrote:
> > While investigating a number of data races, we've encountered data-racy
> > accesses on flags variables to be very common. The typical pattern is a
> > reader masking all but one bit, and the writer setting/clearing only 1
> > bit (current->flags being a frequently encountered case; mm/sl[au]b.c
> > disables KCSAN for this reason currently).
>
> As a heads up, I just sent out the series I promised for
> thread_info::flags, at:
>
> https://lore.kernel.org/lkml/[email protected]/T/#t
>
> ... which I think is complementary to this (IIUC it should help with the
> multi-bit cases you mention below), and may help to make the checks more
> stringent in future.
Nice, glad to see this.
And yes, this series isn't a permission to let the 'flags' variables
be forgotten, but perhaps not every subsystem wants to go through this
now. So seeing any progress on this front helps and we can also use it
to give concrete suggestions how to approach it (e.g. your accessors).
> FWIW, for this series:
>
> Acked-by: Mark Rutland <[email protected]>
Thank you!
On Wed, Jun 09, 2021 at 01:38:10PM +0100, Mark Rutland wrote:
> Hi Marco,
>
> On Mon, Jun 07, 2021 at 02:56:46PM +0200, Marco Elver wrote:
> > While investigating a number of data races, we've encountered data-racy
> > accesses on flags variables to be very common. The typical pattern is a
> > reader masking all but one bit, and the writer setting/clearing only 1
> > bit (current->flags being a frequently encountered case; mm/sl[au]b.c
> > disables KCSAN for this reason currently).
>
> As a heads up, I just sent out the series I promised for
> thread_info::flags, at:
>
> https://lore.kernel.org/lkml/[email protected]/T/#t
>
> ... which I think is complementary to this (IIUC it should help with the
> multi-bit cases you mention below), and may help to make the checks more
> stringent in future.
>
> FWIW, for this series:
>
> Acked-by: Mark Rutland <[email protected]>
Queued and pushed for v5.15, thank you both!
I also queued the following patch making use of CONFIG_KCSAN_STRICT, and I
figured that I should run it past you guys to make check my understanding.
Thoughts?
Thanx, Paul
------------------------------------------------------------------------
commit 023f1604e373575be6335f85abf36fd475d78da3
Author: Paul E. McKenney <[email protected]>
Date: Tue Jun 15 11:14:19 2021 -0700
torture: Apply CONFIG_KCSAN_STRICT to kvm.sh --kcsan argument
Currently, the --kcsan argument to kvm.sh applies a laundry list of
Kconfig options. Now that KCSAN provides the CONFIG_KCSAN_STRICT Kconfig
option, this commit reduces the laundry list to this one option.
Signed-off-by: Paul E. McKenney <[email protected]>
diff --git a/tools/testing/selftests/rcutorture/bin/kvm.sh b/tools/testing/selftests/rcutorture/bin/kvm.sh
index b4ac4ee33222..f2bd80391999 100755
--- a/tools/testing/selftests/rcutorture/bin/kvm.sh
+++ b/tools/testing/selftests/rcutorture/bin/kvm.sh
@@ -184,7 +184,7 @@ do
TORTURE_KCONFIG_KASAN_ARG="CONFIG_DEBUG_INFO=y CONFIG_KASAN=y"; export TORTURE_KCONFIG_KASAN_ARG
;;
--kcsan)
- TORTURE_KCONFIG_KCSAN_ARG="CONFIG_DEBUG_INFO=y CONFIG_KCSAN=y CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=n CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=n CONFIG_KCSAN_REPORT_ONCE_IN_MS=100000 CONFIG_KCSAN_INTERRUPT_WATCHER=y CONFIG_KCSAN_VERBOSE=y CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_PROVE_LOCKING=y"; export TORTURE_KCONFIG_KCSAN_ARG
+ TORTURE_KCONFIG_KCSAN_ARG="CONFIG_DEBUG_INFO=y CONFIG_KCSAN=y CONFIG_KCSAN_STRICT=y CONFIG_KCSAN_REPORT_ONCE_IN_MS=100000 CONFIG_KCSAN_VERBOSE=y CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_PROVE_LOCKING=y"; export TORTURE_KCONFIG_KCSAN_ARG
;;
--kmake-arg|--kmake-args)
checkarg --kmake-arg "(kernel make arguments)" $# "$2" '.*' '^error$'
On Tue, Jun 15, 2021 at 11:19AM -0700, Paul E. McKenney wrote:
[...]
> Queued and pushed for v5.15, thank you both!
>
> I also queued the following patch making use of CONFIG_KCSAN_STRICT, and I
> figured that I should run it past you guys to make check my understanding.
>
> Thoughts?
You still need CONFIG_KCSAN_INTERRUPT_WATCHER=y, but otherwise looks
good.
I thought I'd leave that out for now, but now thinking about it, we
might as well imply interruptible watchers. If you agree, feel free to
queue the below patch ahead of yours.
Thanks,
-- Marco
------ >8 ------
From: Marco Elver <[email protected]>
Date: Tue, 15 Jun 2021 20:39:38 +0200
Subject: [PATCH] kcsan: Make strict mode imply interruptible watchers
If CONFIG_KCSAN_STRICT=y, select CONFIG_KCSAN_INTERRUPT_WATCHER as well.
With interruptible watchers, we'll also report same-CPU data races; if
we requested strict mode, we might as well show these, too.
Suggested-by: Paul E. McKenney <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
lib/Kconfig.kcsan | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index 26f03c754d39..e0a93ffdef30 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -150,7 +150,8 @@ config KCSAN_SKIP_WATCH_RANDOMIZE
KCSAN_WATCH_SKIP.
config KCSAN_INTERRUPT_WATCHER
- bool "Interruptible watchers"
+ bool "Interruptible watchers" if !KCSAN_STRICT
+ default KCSAN_STRICT
help
If enabled, a task that set up a watchpoint may be interrupted while
delayed. This option will allow KCSAN to detect races between
--
2.32.0.272.g935e593368-goog
On Tue, Jun 15, 2021 at 08:51:18PM +0200, Marco Elver wrote:
> On Tue, Jun 15, 2021 at 11:19AM -0700, Paul E. McKenney wrote:
> [...]
> > Queued and pushed for v5.15, thank you both!
> >
> > I also queued the following patch making use of CONFIG_KCSAN_STRICT, and I
> > figured that I should run it past you guys to make check my understanding.
> >
> > Thoughts?
>
> You still need CONFIG_KCSAN_INTERRUPT_WATCHER=y, but otherwise looks
> good.
I knew I was missing something... :-/
> I thought I'd leave that out for now, but now thinking about it, we
> might as well imply interruptible watchers. If you agree, feel free to
> queue the below patch ahead of yours.
That works for me! I have queued the patch below and rebased it to
precede my change to the torture-test infrastructure.
Thanx, Paul
> Thanks,
> -- Marco
>
> ------ >8 ------
>
> From: Marco Elver <[email protected]>
> Date: Tue, 15 Jun 2021 20:39:38 +0200
> Subject: [PATCH] kcsan: Make strict mode imply interruptible watchers
>
> If CONFIG_KCSAN_STRICT=y, select CONFIG_KCSAN_INTERRUPT_WATCHER as well.
>
> With interruptible watchers, we'll also report same-CPU data races; if
> we requested strict mode, we might as well show these, too.
>
> Suggested-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>
> ---
> lib/Kconfig.kcsan | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index 26f03c754d39..e0a93ffdef30 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -150,7 +150,8 @@ config KCSAN_SKIP_WATCH_RANDOMIZE
> KCSAN_WATCH_SKIP.
>
> config KCSAN_INTERRUPT_WATCHER
> - bool "Interruptible watchers"
> + bool "Interruptible watchers" if !KCSAN_STRICT
> + default KCSAN_STRICT
> help
> If enabled, a task that set up a watchpoint may be interrupted while
> delayed. This option will allow KCSAN to detect races between
> --
> 2.32.0.272.g935e593368-goog
>