2020-02-05 20:45:31

by Marco Elver

[permalink] [raw]
Subject: [PATCH 2/3] kcsan: Introduce ASSERT_EXCLUSIVE_* macros

Introduces ASSERT_EXCLUSIVE_WRITER and ASSERT_EXCLUSIVE_ACCESS, which
may be used to assert properties of synchronization logic, where
violation cannot be detected as a normal data race.

Examples of the reports that may be generated:

==================================================================
BUG: KCSAN: data-race in test_thread / test_thread

write to 0xffffffffab3d1540 of 8 bytes by task 466 on cpu 2:
test_thread+0x8d/0x111
debugfs_write.cold+0x32/0x44
...

assert no writes to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
test_thread+0xa3/0x111
debugfs_write.cold+0x32/0x44
...
==================================================================

==================================================================
BUG: KCSAN: data-race in test_thread / test_thread

assert no accesses to 0xffffffffab3d1540 of 8 bytes by task 465 on cpu 1:
test_thread+0xb9/0x111
debugfs_write.cold+0x32/0x44
...

read to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
test_thread+0x77/0x111
debugfs_write.cold+0x32/0x44
...
==================================================================

Signed-off-by: Marco Elver <[email protected]>
Suggested-by: Paul E. McKenney <[email protected]>
---

Please let me know if the names make sense, given they do not include a
KCSAN_ prefix.

The names are unique across the kernel. I wouldn't expect another macro
with the same name but different semantics to pop up any time soon. If
there is a dual use to these macros (e.g. another tool that could hook
into it), we could also move it elsewhere (include/linux/compiler.h?).

We can also revisit the original suggestion of WRITE_ONCE_EXCLUSIVE(),
if it is something that'd be used very widely. It'd be straightforward
to add with the help of these macros, but would need to be added to
include/linux/compiler.h.
---
include/linux/kcsan-checks.h | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
index 21b1d1f214ad5..1a7b51e516335 100644
--- a/include/linux/kcsan-checks.h
+++ b/include/linux/kcsan-checks.h
@@ -96,4 +96,38 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
kcsan_check_access(ptr, size, KCSAN_ACCESS_ATOMIC | KCSAN_ACCESS_WRITE)
#endif

+/**
+ * ASSERT_EXCLUSIVE_WRITER - assert no other threads are writing @var
+ *
+ * Assert that there are no other threads writing @var; other readers are
+ * allowed. This assertion can be used to specify properties of synchronization
+ * logic, 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.
+ *
+ * @var variable to assert on
+ */
+#define ASSERT_EXCLUSIVE_WRITER(var) \
+ __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_ASSERT)
+
+/**
+ * ASSERT_EXCLUSIVE_ACCESS - assert no other threads are accessing @var
+ *
+ * Assert that no other thread is accessing @var (no readers nor writers). This
+ * assertion can be used to specify properties of synchronization logic, where
+ * violation cannot be detected as a normal data race.
+ *
+ * For example, if a variable is not read nor written by the current thread, nor
+ * should it be touched by any other threads during the current execution phase.
+ *
+ * @var variable to assert on
+ */
+#define ASSERT_EXCLUSIVE_ACCESS(var) \
+ __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT)
+
#endif /* _LINUX_KCSAN_CHECKS_H */
--
2.25.0.341.g760bfbb309-goog


2020-02-05 21:34:12

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 2/3] kcsan: Introduce ASSERT_EXCLUSIVE_* macros

On Wed, Feb 05, 2020 at 09:43:32PM +0100, Marco Elver wrote:
> Introduces ASSERT_EXCLUSIVE_WRITER and ASSERT_EXCLUSIVE_ACCESS, which
> may be used to assert properties of synchronization logic, where
> violation cannot be detected as a normal data race.
>
> Examples of the reports that may be generated:
>
> ==================================================================
> BUG: KCSAN: data-race in test_thread / test_thread
>
> write to 0xffffffffab3d1540 of 8 bytes by task 466 on cpu 2:
> test_thread+0x8d/0x111
> debugfs_write.cold+0x32/0x44
> ...
>
> assert no writes to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> test_thread+0xa3/0x111
> debugfs_write.cold+0x32/0x44
> ...
> ==================================================================
>
> ==================================================================
> BUG: KCSAN: data-race in test_thread / test_thread
>
> assert no accesses to 0xffffffffab3d1540 of 8 bytes by task 465 on cpu 1:
> test_thread+0xb9/0x111
> debugfs_write.cold+0x32/0x44
> ...
>
> read to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> test_thread+0x77/0x111
> debugfs_write.cold+0x32/0x44
> ...
> ==================================================================
>
> Signed-off-by: Marco Elver <[email protected]>
> Suggested-by: Paul E. McKenney <[email protected]>
> ---
>
> Please let me know if the names make sense, given they do not include a
> KCSAN_ prefix.

I am OK with this, but there might well be some bikeshedding later on.
Which should not be a real problem, irritating though it might be.

> The names are unique across the kernel. I wouldn't expect another macro
> with the same name but different semantics to pop up any time soon. If
> there is a dual use to these macros (e.g. another tool that could hook
> into it), we could also move it elsewhere (include/linux/compiler.h?).
>
> We can also revisit the original suggestion of WRITE_ONCE_EXCLUSIVE(),
> if it is something that'd be used very widely. It'd be straightforward
> to add with the help of these macros, but would need to be added to
> include/linux/compiler.h.

A more definite use case for ASSERT_EXCLUSIVE_ACCESS() is a
reference-counting algorithm where exclusive access is expected after
a successful atomic_dec_and_test(). Any objection to making the
docbook header use that example? I believe that a more familiar
example would help people see the point of all this. ;-)

I am queueing these as-is for review and testing, but please feel free
to send updated versions. Easy to do the replacement!

And you knew that this was coming... It looks to me that I can
do something like this:

struct foo {
int a;
char b;
long c;
atomic_t refctr;
};

void do_a_foo(struct foo *fp)
{
if (atomic_dec_and_test(&fp->refctr)) {
ASSERT_EXCLUSIVE_ACCESS(*fp);
safely_dispose_of(fp);
}
}

Does that work, or is it necessary to assert for each field separately?

Thanx, Paul

> ---
> include/linux/kcsan-checks.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
> index 21b1d1f214ad5..1a7b51e516335 100644
> --- a/include/linux/kcsan-checks.h
> +++ b/include/linux/kcsan-checks.h
> @@ -96,4 +96,38 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
> kcsan_check_access(ptr, size, KCSAN_ACCESS_ATOMIC | KCSAN_ACCESS_WRITE)
> #endif
>
> +/**
> + * ASSERT_EXCLUSIVE_WRITER - assert no other threads are writing @var
> + *
> + * Assert that there are no other threads writing @var; other readers are
> + * allowed. This assertion can be used to specify properties of synchronization
> + * logic, 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.
> + *
> + * @var variable to assert on
> + */
> +#define ASSERT_EXCLUSIVE_WRITER(var) \
> + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_ASSERT)
> +
> +/**
> + * ASSERT_EXCLUSIVE_ACCESS - assert no other threads are accessing @var
> + *
> + * Assert that no other thread is accessing @var (no readers nor writers). This
> + * assertion can be used to specify properties of synchronization logic, where
> + * violation cannot be detected as a normal data race.
> + *
> + * For example, if a variable is not read nor written by the current thread, nor
> + * should it be touched by any other threads during the current execution phase.
> + *
> + * @var variable to assert on
> + */
> +#define ASSERT_EXCLUSIVE_ACCESS(var) \
> + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT)
> +
> #endif /* _LINUX_KCSAN_CHECKS_H */
> --
> 2.25.0.341.g760bfbb309-goog
>

2020-02-05 21:49:45

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH 2/3] kcsan: Introduce ASSERT_EXCLUSIVE_* macros

On Wed, 5 Feb 2020 at 22:33, Paul E. McKenney <[email protected]> wrote:
>
> On Wed, Feb 05, 2020 at 09:43:32PM +0100, Marco Elver wrote:
> > Introduces ASSERT_EXCLUSIVE_WRITER and ASSERT_EXCLUSIVE_ACCESS, which
> > may be used to assert properties of synchronization logic, where
> > violation cannot be detected as a normal data race.
> >
> > Examples of the reports that may be generated:
> >
> > ==================================================================
> > BUG: KCSAN: data-race in test_thread / test_thread
> >
> > write to 0xffffffffab3d1540 of 8 bytes by task 466 on cpu 2:
> > test_thread+0x8d/0x111
> > debugfs_write.cold+0x32/0x44
> > ...
> >
> > assert no writes to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> > test_thread+0xa3/0x111
> > debugfs_write.cold+0x32/0x44
> > ...
> > ==================================================================
> >
> > ==================================================================
> > BUG: KCSAN: data-race in test_thread / test_thread
> >
> > assert no accesses to 0xffffffffab3d1540 of 8 bytes by task 465 on cpu 1:
> > test_thread+0xb9/0x111
> > debugfs_write.cold+0x32/0x44
> > ...
> >
> > read to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> > test_thread+0x77/0x111
> > debugfs_write.cold+0x32/0x44
> > ...
> > ==================================================================
> >
> > Signed-off-by: Marco Elver <[email protected]>
> > Suggested-by: Paul E. McKenney <[email protected]>
> > ---
> >
> > Please let me know if the names make sense, given they do not include a
> > KCSAN_ prefix.
>
> I am OK with this, but there might well be some bikeshedding later on.
> Which should not be a real problem, irritating though it might be.
>
> > The names are unique across the kernel. I wouldn't expect another macro
> > with the same name but different semantics to pop up any time soon. If
> > there is a dual use to these macros (e.g. another tool that could hook
> > into it), we could also move it elsewhere (include/linux/compiler.h?).
> >
> > We can also revisit the original suggestion of WRITE_ONCE_EXCLUSIVE(),
> > if it is something that'd be used very widely. It'd be straightforward
> > to add with the help of these macros, but would need to be added to
> > include/linux/compiler.h.
>
> A more definite use case for ASSERT_EXCLUSIVE_ACCESS() is a
> reference-counting algorithm where exclusive access is expected after
> a successful atomic_dec_and_test(). Any objection to making the
> docbook header use that example? I believe that a more familiar
> example would help people see the point of all this. ;-)

Happy to update the example -- I'll send it tomorrow.

> I am queueing these as-is for review and testing, but please feel free
> to send updated versions. Easy to do the replacement!

Thank you!

> And you knew that this was coming... It looks to me that I can
> do something like this:
>
> struct foo {
> int a;
> char b;
> long c;
> atomic_t refctr;
> };
>
> void do_a_foo(struct foo *fp)
> {
> if (atomic_dec_and_test(&fp->refctr)) {
> ASSERT_EXCLUSIVE_ACCESS(*fp);
> safely_dispose_of(fp);
> }
> }
>
> Does that work, or is it necessary to assert for each field separately?

That works just fine, and will check for races on the whole struct.

Thanks,
-- Marco

> Thanx, Paul
>
> > ---
> > include/linux/kcsan-checks.h | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
> > index 21b1d1f214ad5..1a7b51e516335 100644
> > --- a/include/linux/kcsan-checks.h
> > +++ b/include/linux/kcsan-checks.h
> > @@ -96,4 +96,38 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
> > kcsan_check_access(ptr, size, KCSAN_ACCESS_ATOMIC | KCSAN_ACCESS_WRITE)
> > #endif
> >
> > +/**
> > + * ASSERT_EXCLUSIVE_WRITER - assert no other threads are writing @var
> > + *
> > + * Assert that there are no other threads writing @var; other readers are
> > + * allowed. This assertion can be used to specify properties of synchronization
> > + * logic, 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.
> > + *
> > + * @var variable to assert on
> > + */
> > +#define ASSERT_EXCLUSIVE_WRITER(var) \
> > + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_ASSERT)
> > +
> > +/**
> > + * ASSERT_EXCLUSIVE_ACCESS - assert no other threads are accessing @var
> > + *
> > + * Assert that no other thread is accessing @var (no readers nor writers). This
> > + * assertion can be used to specify properties of synchronization logic, where
> > + * violation cannot be detected as a normal data race.
> > + *
> > + * For example, if a variable is not read nor written by the current thread, nor
> > + * should it be touched by any other threads during the current execution phase.
> > + *
> > + * @var variable to assert on
> > + */
> > +#define ASSERT_EXCLUSIVE_ACCESS(var) \
> > + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT)
> > +
> > #endif /* _LINUX_KCSAN_CHECKS_H */
> > --
> > 2.25.0.341.g760bfbb309-goog
> >
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20200205213302.GA2935%40paulmck-ThinkPad-P72.

2020-02-05 22:07:24

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 2/3] kcsan: Introduce ASSERT_EXCLUSIVE_* macros

On Wed, Feb 05, 2020 at 10:48:14PM +0100, Marco Elver wrote:
> On Wed, 5 Feb 2020 at 22:33, Paul E. McKenney <[email protected]> wrote:
> >
> > On Wed, Feb 05, 2020 at 09:43:32PM +0100, Marco Elver wrote:
> > > Introduces ASSERT_EXCLUSIVE_WRITER and ASSERT_EXCLUSIVE_ACCESS, which
> > > may be used to assert properties of synchronization logic, where
> > > violation cannot be detected as a normal data race.
> > >
> > > Examples of the reports that may be generated:
> > >
> > > ==================================================================
> > > BUG: KCSAN: data-race in test_thread / test_thread
> > >
> > > write to 0xffffffffab3d1540 of 8 bytes by task 466 on cpu 2:
> > > test_thread+0x8d/0x111
> > > debugfs_write.cold+0x32/0x44
> > > ...
> > >
> > > assert no writes to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> > > test_thread+0xa3/0x111
> > > debugfs_write.cold+0x32/0x44
> > > ...
> > > ==================================================================
> > >
> > > ==================================================================
> > > BUG: KCSAN: data-race in test_thread / test_thread
> > >
> > > assert no accesses to 0xffffffffab3d1540 of 8 bytes by task 465 on cpu 1:
> > > test_thread+0xb9/0x111
> > > debugfs_write.cold+0x32/0x44
> > > ...
> > >
> > > read to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> > > test_thread+0x77/0x111
> > > debugfs_write.cold+0x32/0x44
> > > ...
> > > ==================================================================
> > >
> > > Signed-off-by: Marco Elver <[email protected]>
> > > Suggested-by: Paul E. McKenney <[email protected]>
> > > ---
> > >
> > > Please let me know if the names make sense, given they do not include a
> > > KCSAN_ prefix.
> >
> > I am OK with this, but there might well be some bikeshedding later on.
> > Which should not be a real problem, irritating though it might be.
> >
> > > The names are unique across the kernel. I wouldn't expect another macro
> > > with the same name but different semantics to pop up any time soon. If
> > > there is a dual use to these macros (e.g. another tool that could hook
> > > into it), we could also move it elsewhere (include/linux/compiler.h?).
> > >
> > > We can also revisit the original suggestion of WRITE_ONCE_EXCLUSIVE(),
> > > if it is something that'd be used very widely. It'd be straightforward
> > > to add with the help of these macros, but would need to be added to
> > > include/linux/compiler.h.
> >
> > A more definite use case for ASSERT_EXCLUSIVE_ACCESS() is a
> > reference-counting algorithm where exclusive access is expected after
> > a successful atomic_dec_and_test(). Any objection to making the
> > docbook header use that example? I believe that a more familiar
> > example would help people see the point of all this. ;-)
>
> Happy to update the example -- I'll send it tomorrow.

Sounds great!

> > I am queueing these as-is for review and testing, but please feel free
> > to send updated versions. Easy to do the replacement!
>
> Thank you!
>
> > And you knew that this was coming... It looks to me that I can
> > do something like this:
> >
> > struct foo {
> > int a;
> > char b;
> > long c;
> > atomic_t refctr;
> > };
> >
> > void do_a_foo(struct foo *fp)
> > {
> > if (atomic_dec_and_test(&fp->refctr)) {
> > ASSERT_EXCLUSIVE_ACCESS(*fp);
> > safely_dispose_of(fp);
> > }
> > }
> >
> > Does that work, or is it necessary to assert for each field separately?
>
> That works just fine, and will check for races on the whole struct.

Nice!!!

Thanx, Paul

> Thanks,
> -- Marco
>
> > Thanx, Paul
> >
> > > ---
> > > include/linux/kcsan-checks.h | 34 ++++++++++++++++++++++++++++++++++
> > > 1 file changed, 34 insertions(+)
> > >
> > > diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
> > > index 21b1d1f214ad5..1a7b51e516335 100644
> > > --- a/include/linux/kcsan-checks.h
> > > +++ b/include/linux/kcsan-checks.h
> > > @@ -96,4 +96,38 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
> > > kcsan_check_access(ptr, size, KCSAN_ACCESS_ATOMIC | KCSAN_ACCESS_WRITE)
> > > #endif
> > >
> > > +/**
> > > + * ASSERT_EXCLUSIVE_WRITER - assert no other threads are writing @var
> > > + *
> > > + * Assert that there are no other threads writing @var; other readers are
> > > + * allowed. This assertion can be used to specify properties of synchronization
> > > + * logic, 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.
> > > + *
> > > + * @var variable to assert on
> > > + */
> > > +#define ASSERT_EXCLUSIVE_WRITER(var) \
> > > + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_ASSERT)
> > > +
> > > +/**
> > > + * ASSERT_EXCLUSIVE_ACCESS - assert no other threads are accessing @var
> > > + *
> > > + * Assert that no other thread is accessing @var (no readers nor writers). This
> > > + * assertion can be used to specify properties of synchronization logic, where
> > > + * violation cannot be detected as a normal data race.
> > > + *
> > > + * For example, if a variable is not read nor written by the current thread, nor
> > > + * should it be touched by any other threads during the current execution phase.
> > > + *
> > > + * @var variable to assert on
> > > + */
> > > +#define ASSERT_EXCLUSIVE_ACCESS(var) \
> > > + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT)
> > > +
> > > #endif /* _LINUX_KCSAN_CHECKS_H */
> > > --
> > > 2.25.0.341.g760bfbb309-goog
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20200205213302.GA2935%40paulmck-ThinkPad-P72.

2020-02-06 15:57:51

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH 2/3] kcsan: Introduce ASSERT_EXCLUSIVE_* macros

On Wed, 5 Feb 2020 at 23:04, Paul E. McKenney <[email protected]> wrote:
>
> On Wed, Feb 05, 2020 at 10:48:14PM +0100, Marco Elver wrote:
> > On Wed, 5 Feb 2020 at 22:33, Paul E. McKenney <[email protected]> wrote:
> > >
> > > On Wed, Feb 05, 2020 at 09:43:32PM +0100, Marco Elver wrote:
> > > > Introduces ASSERT_EXCLUSIVE_WRITER and ASSERT_EXCLUSIVE_ACCESS, which
> > > > may be used to assert properties of synchronization logic, where
> > > > violation cannot be detected as a normal data race.
> > > >
> > > > Examples of the reports that may be generated:
> > > >
> > > > ==================================================================
> > > > BUG: KCSAN: data-race in test_thread / test_thread
> > > >
> > > > write to 0xffffffffab3d1540 of 8 bytes by task 466 on cpu 2:
> > > > test_thread+0x8d/0x111
> > > > debugfs_write.cold+0x32/0x44
> > > > ...
> > > >
> > > > assert no writes to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> > > > test_thread+0xa3/0x111
> > > > debugfs_write.cold+0x32/0x44
> > > > ...
> > > > ==================================================================
> > > >
> > > > ==================================================================
> > > > BUG: KCSAN: data-race in test_thread / test_thread
> > > >
> > > > assert no accesses to 0xffffffffab3d1540 of 8 bytes by task 465 on cpu 1:
> > > > test_thread+0xb9/0x111
> > > > debugfs_write.cold+0x32/0x44
> > > > ...
> > > >
> > > > read to 0xffffffffab3d1540 of 8 bytes by task 464 on cpu 0:
> > > > test_thread+0x77/0x111
> > > > debugfs_write.cold+0x32/0x44
> > > > ...
> > > > ==================================================================
> > > >
> > > > Signed-off-by: Marco Elver <[email protected]>
> > > > Suggested-by: Paul E. McKenney <[email protected]>
> > > > ---
> > > >
> > > > Please let me know if the names make sense, given they do not include a
> > > > KCSAN_ prefix.
> > >
> > > I am OK with this, but there might well be some bikeshedding later on.
> > > Which should not be a real problem, irritating though it might be.
> > >
> > > > The names are unique across the kernel. I wouldn't expect another macro
> > > > with the same name but different semantics to pop up any time soon. If
> > > > there is a dual use to these macros (e.g. another tool that could hook
> > > > into it), we could also move it elsewhere (include/linux/compiler.h?).
> > > >
> > > > We can also revisit the original suggestion of WRITE_ONCE_EXCLUSIVE(),
> > > > if it is something that'd be used very widely. It'd be straightforward
> > > > to add with the help of these macros, but would need to be added to
> > > > include/linux/compiler.h.
> > >
> > > A more definite use case for ASSERT_EXCLUSIVE_ACCESS() is a
> > > reference-counting algorithm where exclusive access is expected after
> > > a successful atomic_dec_and_test(). Any objection to making the
> > > docbook header use that example? I believe that a more familiar
> > > example would help people see the point of all this. ;-)
> >
> > Happy to update the example -- I'll send it tomorrow.
>
> Sounds great!

v2 sent: http://lkml.kernel.org/r/[email protected]

Thanks,
-- Marco

> > > I am queueing these as-is for review and testing, but please feel free
> > > to send updated versions. Easy to do the replacement!
> >
> > Thank you!
> >
> > > And you knew that this was coming... It looks to me that I can
> > > do something like this:
> > >
> > > struct foo {
> > > int a;
> > > char b;
> > > long c;
> > > atomic_t refctr;
> > > };
> > >
> > > void do_a_foo(struct foo *fp)
> > > {
> > > if (atomic_dec_and_test(&fp->refctr)) {
> > > ASSERT_EXCLUSIVE_ACCESS(*fp);
> > > safely_dispose_of(fp);
> > > }
> > > }
> > >
> > > Does that work, or is it necessary to assert for each field separately?
> >
> > That works just fine, and will check for races on the whole struct.
>
> Nice!!!
>
> Thanx, Paul
>
> > Thanks,
> > -- Marco
> >
> > > Thanx, Paul
> > >
> > > > ---
> > > > include/linux/kcsan-checks.h | 34 ++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 34 insertions(+)
> > > >
> > > > diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
> > > > index 21b1d1f214ad5..1a7b51e516335 100644
> > > > --- a/include/linux/kcsan-checks.h
> > > > +++ b/include/linux/kcsan-checks.h
> > > > @@ -96,4 +96,38 @@ static inline void kcsan_check_access(const volatile void *ptr, size_t size,
> > > > kcsan_check_access(ptr, size, KCSAN_ACCESS_ATOMIC | KCSAN_ACCESS_WRITE)
> > > > #endif
> > > >
> > > > +/**
> > > > + * ASSERT_EXCLUSIVE_WRITER - assert no other threads are writing @var
> > > > + *
> > > > + * Assert that there are no other threads writing @var; other readers are
> > > > + * allowed. This assertion can be used to specify properties of synchronization
> > > > + * logic, 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.
> > > > + *
> > > > + * @var variable to assert on
> > > > + */
> > > > +#define ASSERT_EXCLUSIVE_WRITER(var) \
> > > > + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_ASSERT)
> > > > +
> > > > +/**
> > > > + * ASSERT_EXCLUSIVE_ACCESS - assert no other threads are accessing @var
> > > > + *
> > > > + * Assert that no other thread is accessing @var (no readers nor writers). This
> > > > + * assertion can be used to specify properties of synchronization logic, where
> > > > + * violation cannot be detected as a normal data race.
> > > > + *
> > > > + * For example, if a variable is not read nor written by the current thread, nor
> > > > + * should it be touched by any other threads during the current execution phase.
> > > > + *
> > > > + * @var variable to assert on
> > > > + */
> > > > +#define ASSERT_EXCLUSIVE_ACCESS(var) \
> > > > + __kcsan_check_access(&(var), sizeof(var), KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT)
> > > > +
> > > > #endif /* _LINUX_KCSAN_CHECKS_H */
> > > > --
> > > > 2.25.0.341.g760bfbb309-goog
> > > >
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20200205213302.GA2935%40paulmck-ThinkPad-P72.