From: Marco Elver <elver@google.com>
To: elver@google.com
Cc: paulmck@kernel.org, andreyknvl@google.com, glider@google.com,
dvyukov@google.com, kasan-dev@googlegroups.com,
linux-kernel@vger.kernel.org, corbet@lwn.net,
linux-doc@vger.kernel.org
Subject: [PATCH v2 3/3] kcsan: Update API documentation in kcsan-checks.h
Date: Thu, 5 Mar 2020 15:21:09 +0100 [thread overview]
Message-ID: <20200305142109.50945-3-elver@google.com> (raw)
In-Reply-To: <20200305142109.50945-1-elver@google.com>
Update the API documentation for ASSERT_EXCLUSIVE_* macros and make them
generate readable documentation for the code examples.
All @variable short summaries were missing ':', which was updated for
the whole file.
Tested with "make htmldocs".
Signed-off-by: Marco Elver <elver@google.com>
---
include/linux/kcsan-checks.h | 98 ++++++++++++++++++++++--------------
1 file changed, 61 insertions(+), 37 deletions(-)
diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
index 1b8aac5d6a0b5..14fd10e5a9177 100644
--- a/include/linux/kcsan-checks.h
+++ b/include/linux/kcsan-checks.h
@@ -26,9 +26,9 @@
/**
* __kcsan_check_access - check generic access for races
*
- * @ptr address of access
- * @size size of access
- * @type access type modifier
+ * @ptr: address of access
+ * @size: size of access
+ * @type: access type modifier
*/
void __kcsan_check_access(const volatile void *ptr, size_t size, int type);
@@ -64,7 +64,7 @@ void kcsan_flat_atomic_end(void);
* Force treating the next n memory accesses for the current context as atomic
* operations.
*
- * @n number of following memory accesses to treat as atomic.
+ * @n: number of following memory accesses to treat as atomic.
*/
void kcsan_atomic_next(int n);
@@ -74,7 +74,7 @@ void kcsan_atomic_next(int n);
* Set the access mask for all accesses for the current context if non-zero.
* Only value changes to bits set in the mask will be reported.
*
- * @mask bitmask
+ * @mask: bitmask
*/
void kcsan_set_access_mask(unsigned long mask);
@@ -106,16 +106,16 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
/**
* __kcsan_check_read - check regular read access for races
*
- * @ptr address of access
- * @size size of access
+ * @ptr: address of access
+ * @size: size of access
*/
#define __kcsan_check_read(ptr, size) __kcsan_check_access(ptr, size, 0)
/**
* __kcsan_check_write - check regular write access for races
*
- * @ptr address of access
- * @size size of access
+ * @ptr: address of access
+ * @size: size of access
*/
#define __kcsan_check_write(ptr, size) \
__kcsan_check_access(ptr, size, KCSAN_ACCESS_WRITE)
@@ -123,16 +123,16 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
/**
* kcsan_check_read - check regular read access for races
*
- * @ptr address of access
- * @size size of access
+ * @ptr: address of access
+ * @size: size of access
*/
#define kcsan_check_read(ptr, size) kcsan_check_access(ptr, size, 0)
/**
* kcsan_check_write - check regular write access for races
*
- * @ptr address of access
- * @size size of access
+ * @ptr: address of access
+ * @size: size of access
*/
#define kcsan_check_write(ptr, size) \
kcsan_check_access(ptr, size, KCSAN_ACCESS_WRITE)
@@ -158,14 +158,26 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
* allowed. This assertion can be used to specify properties of concurrent code,
* where violation cannot be detected as a normal data race.
*
- * For example, if a per-CPU variable is only meant to be written by a single
- * CPU, but may be read from other CPUs; in this case, reads and writes must be
- * marked properly, however, if an off-CPU WRITE_ONCE() races with the owning
- * CPU's WRITE_ONCE(), would not constitute a data race but could be a harmful
- * race condition. Using this macro allows specifying this property in the code
- * and catch such bugs.
+ * For example, if we only have a single writer, but multiple concurrent
+ * readers, to avoid data races, all these accesses must be marked; even
+ * concurrent marked writes racing with the single writer are bugs.
+ * Unfortunately, due to being marked, they are no longer data races. For cases
+ * like these, we can use the macro as follows:
*
- * @var variable to assert on
+ * .. code-block:: c
+ *
+ * void writer(void) {
+ * spin_lock(&update_foo_lock);
+ * ASSERT_EXCLUSIVE_WRITER(shared_foo);
+ * WRITE_ONCE(shared_foo, ...);
+ * spin_unlock(&update_foo_lock);
+ * }
+ * void reader(void) {
+ * // update_foo_lock does not need to be held!
+ * ... = READ_ONCE(shared_foo);
+ * }
+ *
+ * @var: variable to assert on
*/
#define ASSERT_EXCLUSIVE_WRITER(var) \
__kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_ASSERT)
@@ -177,16 +189,22 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
* writers). This assertion can be used to specify properties of concurrent
* code, where violation cannot be detected as a normal data race.
*
- * For example, in a reference-counting algorithm where exclusive access is
- * expected after the refcount reaches 0. We can check that this property
- * actually holds as follows:
+ * For example, where exclusive access is expected after determining no other
+ * users of an object are left, but the object is not actually freed. We can
+ * check that this property actually holds as follows:
+ *
+ * .. code-block:: c
*
* if (refcount_dec_and_test(&obj->refcnt)) {
* ASSERT_EXCLUSIVE_ACCESS(*obj);
- * safely_dispose_of(obj);
+ * do_some_cleanup(obj);
+ * release_for_reuse(obj);
* }
*
- * @var variable to assert on
+ * Note: For cases where the object is freed, `KASAN <kasan.html>`_ is a better
+ * fit to detect use-after-free bugs.
+ *
+ * @var: variable to assert on
*/
#define ASSERT_EXCLUSIVE_ACCESS(var) \
__kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT)
@@ -200,7 +218,7 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
* concurrent readers are permitted. This assertion captures more detailed
* bit-level properties, compared to the other (word granularity) assertions.
* Only the bits set in @mask are checked for concurrent modifications, while
- * ignoring the remaining bits, i.e. concurrent writes (or reads) to ~@mask bits
+ * ignoring the remaining bits, i.e. concurrent writes (or reads) to ~mask bits
* are ignored.
*
* Use this for variables, where some bits must not be modified concurrently,
@@ -210,17 +228,21 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
* but other bits may still be modified concurrently. A reader may wish to
* assert that this is true as follows:
*
+ * .. code-block:: c
+ *
* ASSERT_EXCLUSIVE_BITS(flags, READ_ONLY_MASK);
* foo = (READ_ONCE(flags) & READ_ONLY_MASK) >> READ_ONLY_SHIFT;
*
- * Note: The access that immediately follows ASSERT_EXCLUSIVE_BITS() is
- * assumed to access the masked bits only, and KCSAN optimistically assumes it
- * is therefore safe, even in the presence of data races, and marking it with
- * READ_ONCE() is optional from KCSAN's point-of-view. We caution, however,
- * that it may still be advisable to do so, since we cannot reason about all
- * compiler optimizations when it comes to bit manipulations (on the reader
- * and writer side). If you are sure nothing can go wrong, we can write the
- * above simply as:
+ * Note: The access that immediately follows ASSERT_EXCLUSIVE_BITS() is assumed
+ * to access the masked bits only, and KCSAN optimistically assumes it is
+ * therefore safe, even in the presence of data races, and marking it with
+ * READ_ONCE() is optional from KCSAN's point-of-view. We caution, however, that
+ * it may still be advisable to do so, since we cannot reason about all compiler
+ * optimizations when it comes to bit manipulations (on the reader and writer
+ * side). If you are sure nothing can go wrong, we can write the above simply
+ * as:
+ *
+ * .. code-block:: c
*
* ASSERT_EXCLUSIVE_BITS(flags, READ_ONLY_MASK);
* foo = (flags & READ_ONLY_MASK) >> READ_ONLY_SHIFT;
@@ -230,15 +252,17 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
* be modified concurrently. Writers, where other bits may change concurrently,
* could use the assertion as follows:
*
+ * .. code-block:: c
+ *
* spin_lock(&foo_lock);
* ASSERT_EXCLUSIVE_BITS(flags, FOO_MASK);
- * old_flags = READ_ONCE(flags);
+ * old_flags = flags;
* new_flags = (old_flags & ~FOO_MASK) | (new_foo << FOO_SHIFT);
* if (cmpxchg(&flags, old_flags, new_flags) != old_flags) { ... }
* spin_unlock(&foo_lock);
*
- * @var variable to assert on
- * @mask only check for modifications to bits set in @mask
+ * @var: variable to assert on
+ * @mask: only check for modifications to bits set in @mask
*/
#define ASSERT_EXCLUSIVE_BITS(var, mask) \
do { \
--
2.25.0.265.gbab2e86ba0-goog
prev parent reply other threads:[~2020-03-05 14:21 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-05 14:21 [PATCH v2 1/3] kcsan: Fix a typo in a comment Marco Elver
2020-03-05 14:21 ` [PATCH v2 2/3] kcsan: Update Documentation/dev-tools/kcsan.rst Marco Elver
2020-03-05 14:21 ` Marco Elver [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=20200305142109.50945-3-elver@google.com \
--to=elver@google.com \
--cc=andreyknvl@google.com \
--cc=corbet@lwn.net \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.