linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	kernel-team@fb.com, mingo@kernel.org
Cc: stern@rowland.harvard.edu, parri.andrea@gmail.com,
	will@kernel.org, peterz@infradead.org, boqun.feng@gmail.com,
	npiggin@gmail.com, dhowells@redhat.com, j.alglave@ucl.ac.uk,
	luc.maranget@inria.fr, akiyks@gmail.com,
	"Paul E. McKenney" <paulmck@kernel.org>
Subject: [PATCH memory-model 4/4] tools/memory-model: Document data_race(READ_ONCE())
Date: Wed, 21 Jul 2021 14:10:03 -0700	[thread overview]
Message-ID: <20210721211003.869892-4-paulmck@kernel.org> (raw)
In-Reply-To: <20210721210726.GA828672@paulmck-ThinkPad-P17-Gen-1>

It is possible to cause KCSAN to ignore marked accesses by applying
__no_kcsan to the function or applying data_race() to the marked accesses.
These approaches allow the developer to restrict compiler optimizations
while also causing KCSAN to ignore diagnostic accesses.

This commit therefore updates the documentation accordingly.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 .../Documentation/access-marking.txt          | 49 +++++++++++++------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/tools/memory-model/Documentation/access-marking.txt b/tools/memory-model/Documentation/access-marking.txt
index fe4ad6d12d24c..a3dcc32e27b44 100644
--- a/tools/memory-model/Documentation/access-marking.txt
+++ b/tools/memory-model/Documentation/access-marking.txt
@@ -37,7 +37,9 @@ compiler's use of code-motion and common-subexpression optimizations.
 Therefore, if a given access is involved in an intentional data race,
 using READ_ONCE() for loads and WRITE_ONCE() for stores is usually
 preferable to data_race(), which in turn is usually preferable to plain
-C-language accesses.
+C-language accesses.  It is permissible to combine #2 and #3, for example,
+data_race(READ_ONCE(a)), which will both restrict compiler optimizations
+and disable KCSAN diagnostics.
 
 KCSAN will complain about many types of data races involving plain
 C-language accesses, but marking all accesses involved in a given data
@@ -86,6 +88,10 @@ that fail to exclude the updates.  In this case, it is important to use
 data_race() for the diagnostic reads because otherwise KCSAN would give
 false-positive warnings about these diagnostic reads.
 
+If it is necessary to both restrict compiler optimizations and disable
+KCSAN diagnostics, use both data_race() and READ_ONCE(), for example,
+data_race(READ_ONCE(a)).
+
 In theory, plain C-language loads can also be used for this use case.
 However, in practice this will have the disadvantage of causing KCSAN
 to generate false positives because KCSAN will have no way of knowing
@@ -279,19 +285,34 @@ tells KCSAN that data races are expected, and should be silently
 ignored.  This data_race() also tells the human reading the code that
 read_foo_diagnostic() might sometimes return a bogus value.
 
-However, please note that your kernel must be built with
-CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=n in order for KCSAN to
-detect a buggy lockless write.  If you need KCSAN to detect such a
-write even if that write did not change the value of foo, you also
-need CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=n.  If you need KCSAN to
-detect such a write happening in an interrupt handler running on the
-same CPU doing the legitimate lock-protected write, you also need
-CONFIG_KCSAN_INTERRUPT_WATCHER=y.  With some or all of these Kconfig
-options set properly, KCSAN can be quite helpful, although it is not
-necessarily a full replacement for hardware watchpoints.  On the other
-hand, neither are hardware watchpoints a full replacement for KCSAN
-because it is not always easy to tell hardware watchpoint to conditionally
-trap on accesses.
+If it is necessary to suppress compiler optimization and also detect
+buggy lockless writes, read_foo_diagnostic() can be updated as follows:
+
+	void read_foo_diagnostic(void)
+	{
+		pr_info("Current value of foo: %d\n", data_race(READ_ONCE(foo)));
+	}
+
+Alternatively, given that KCSAN is to ignore all accesses in this function,
+this function can be marked __no_kcsan and the data_race() can be dropped:
+
+	void __no_kcsan read_foo_diagnostic(void)
+	{
+		pr_info("Current value of foo: %d\n", READ_ONCE(foo));
+	}
+
+However, in order for KCSAN to detect buggy lockless writes, your kernel
+must be built with CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=n.  If you
+need KCSAN to detect such a write even if that write did not change
+the value of foo, you also need CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=n.
+If you need KCSAN to detect such a write happening in an interrupt handler
+running on the same CPU doing the legitimate lock-protected write, you
+also need CONFIG_KCSAN_INTERRUPT_WATCHER=y.  With some or all of these
+Kconfig options set properly, KCSAN can be quite helpful, although
+it is not necessarily a full replacement for hardware watchpoints.
+On the other hand, neither are hardware watchpoints a full replacement
+for KCSAN because it is not always easy to tell hardware watchpoint to
+conditionally trap on accesses.
 
 
 Lock-Protected Writes With Lockless Reads
-- 
2.31.1.189.g2e36527f23


      parent reply	other threads:[~2021-07-21 21:10 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 21:07 [PATCH kcsan 0/8] KCSAN updates for v5.15 Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 1/8] kcsan: Improve some Kconfig comments Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 2/8] kcsan: Remove CONFIG_KCSAN_DEBUG Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 3/8] kcsan: Introduce CONFIG_KCSAN_STRICT Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 4/8] kcsan: Reduce get_ctx() uses in kcsan_found_watchpoint() Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 5/8] kcsan: Rework atomic.h into permissive.h Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 6/8] kcsan: Print if strict or non-strict during init Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 7/8] kcsan: permissive: Ignore data-racy 1-bit value changes Paul E. McKenney
2021-07-21 21:08 ` [PATCH kcsan 8/8] kcsan: Make strict mode imply interruptible watchers Paul E. McKenney
2021-07-21 21:10 ` [PATCH memory-model 1/4] tools/memory-model: Make read_foo_diagnostic() more clearly diagnostic Paul E. McKenney
2021-07-21 21:10 ` [PATCH memory-model 2/4] tools/memory-model: Add example for heuristic lockless reads Paul E. McKenney
2021-07-23  2:08   ` Alan Stern
2021-07-23  6:52     ` Manfred Spraul
2021-07-23 13:05       ` Alan Stern
2021-07-23 13:57         ` Manfred Spraul
2021-07-23 16:30         ` Paul E. McKenney
2021-07-23 17:08           ` Alan Stern
2021-07-23 20:32             ` Paul E. McKenney
2021-07-23 21:03               ` Alan Stern
2021-07-23 22:29                 ` Paul E. McKenney
2021-07-23 16:24     ` Paul E. McKenney
2021-07-23 16:59       ` Alan Stern
2021-07-23 17:30         ` Paul E. McKenney
2021-07-23 18:11           ` Alan Stern
2021-07-23 20:28             ` Paul E. McKenney
2021-07-28 17:40   ` [PATCH v2 " Paul E. McKenney
2021-07-21 21:10 ` [PATCH memory-model 3/4] tools/memory-model: Heuristics using data_race() must handle all values Paul E. McKenney
2021-07-21 21:10 ` Paul E. McKenney [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210721211003.869892-4-paulmck@kernel.org \
    --to=paulmck@kernel.org \
    --cc=akiyks@gmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=kernel-team@fb.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=mingo@kernel.org \
    --cc=npiggin@gmail.com \
    --cc=parri.andrea@gmail.com \
    --cc=peterz@infradead.org \
    --cc=stern@rowland.harvard.edu \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).