[RFC] WRITE_ONCE_INC() and friends

[RFC] WRITE_ONCE_INC() and friends

[Petko Manolov]

	Hi Paul,

Recently I started reading up on KCSAN and at some point I ran into stuff like:

WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);

Some of these are a bit eye-watering for me and could easily be converted to:

WRITE_ONCE_INC(ssp->srcu_lock_nesting[idx]);
WRITE_ONCE_INC(p->mm->numa_scan_seq);

where the above macro could be either: 

#define	WRITE_ONCE_INC(x)	WRITE_ONCE(x, READ_ONCE(x) + 1)

or the more relaxed version:

#define	WRITE_ONCE_INC(x)	WRITE_ONCE(x, x + 1)

I personally like the stronger version better as a) it doesn't seem to increase 
code size (relative to the relaxed one), and b) should be less prone to load 
tearing, etc.  Given the growing popularity of KCSAN I expect a lot of 
concurrent code soon will get the READ_ONCE/WRITE_ONCE conversion.

If you think the above makes sense we could also do this:

#define	WRITE_ONCE_DEC(x)	WRITE_ONCE(x, READ_ONCE(x) - 1)
#define	WRITE_ONCE_ADD(x, v)	WRITE_ONCE(x, READ_ONCE(x) + v)
#define	WRITE_ONCE_SUB(x, v)	WRITE_ONCE(x, READ_ONCE(x) - v)


cheers,
Petko

RE: [RFC] WRITE_ONCE_INC() and friends

[David Laight]

From: Petko Manolov
> Sent: 19 April 2020 10:45
> Recently I started reading up on KCSAN and at some point I ran into stuff like:
> 
> WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);

If all the accesses use READ/WRITE_ONCE() why not just mark the
structure field 'volatile'?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [RFC] WRITE_ONCE_INC() and friends

[Petko Manolov]

On 20-04-19 18:02:50, David Laight wrote:
> From: Petko Manolov
> > Sent: 19 April 2020 10:45
> > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > 
> > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> 
> If all the accesses use READ/WRITE_ONCE() why not just mark the structure 
> field 'volatile'?

This is a bit heavy.  I guess you've read this one: 

	https://lwn.net/Articles/233479/

And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for 
example, all others are from withing spin_lock/unlock pairs then we _may_ not 
need READ/WRITE_ONCE().

I merely proposed the _INC() variant for better readability.


		Petko

RE: [RFC] WRITE_ONCE_INC() and friends

[David Laight]

From: Petko Manolov
> Sent: 19 April 2020 19:30
> 
> On 20-04-19 18:02:50, David Laight wrote:
> > From: Petko Manolov
> > > Sent: 19 April 2020 10:45
> > > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > >
> > > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> >
> > If all the accesses use READ/WRITE_ONCE() why not just mark the structure
> > field 'volatile'?
> 
> This is a bit heavy.  I guess you've read this one:
> 
> 	https://lwn.net/Articles/233479/

I remember reading something similar before.
I also remember a very old gcc (2.95?) that did a readback
after every volatile write on sparc (to flush the store buffer).
That broke everything.

I suspect there is a lot more code that is attempting to be lockless
these days.
Ring buffers (one writer and one reader) are a typical example where
you don't need locks but do need to use a consistent value.

Now you may also need ordering between accesses - which I think needs
more than volatile.

> And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for
> example, all others are from withing spin_lock/unlock pairs then we _may_ not
> need READ/WRITE_ONCE().

The cost of volatile accesses is probably minimal unless the
code is written assuming the compiler will only access things once.

> I merely proposed the _INC() variant for better readability.

More like shorter code lines :-)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [RFC] WRITE_ONCE_INC() and friends

[Paul E. McKenney]

On Sun, Apr 19, 2020 at 09:37:10PM +0000, David Laight wrote:
> From: Petko Manolov
> > Sent: 19 April 2020 19:30
> > 
> > On 20-04-19 18:02:50, David Laight wrote:
> > > From: Petko Manolov
> > > > Sent: 19 April 2020 10:45
> > > > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > > >
> > > > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > > > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> > >
> > > If all the accesses use READ/WRITE_ONCE() why not just mark the structure
> > > field 'volatile'?
> > 
> > This is a bit heavy.  I guess you've read this one:
> > 
> > 	https://lwn.net/Articles/233479/
> 
> I remember reading something similar before.
> I also remember a very old gcc (2.95?) that did a readback
> after every volatile write on sparc (to flush the store buffer).
> That broke everything.
> 
> I suspect there is a lot more code that is attempting to be lockless
> these days.
> Ring buffers (one writer and one reader) are a typical example where
> you don't need locks but do need to use a consistent value.
> 
> Now you may also need ordering between accesses - which I think needs
> more than volatile.

In Petko's patch, all needed ordering is supplied by the fact that it
is the same variable being read and written.  But yes, in many other
cases, more ordering is required.

> > And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for
> > example, all others are from withing spin_lock/unlock pairs then we _may_ not
> > need READ/WRITE_ONCE().
> 
> The cost of volatile accesses is probably minimal unless the
> code is written assuming the compiler will only access things once.

And there are variables marked as volatile, for example, jiffies.

But one downside of declaring variables volatile is that it can prevent
KCSAN from spotting violations of the concurrency design for those
variables.

> > I merely proposed the _INC() variant for better readability.
> 
> More like shorter code lines :-)

That too!  ;-)

							Thanx, Paul

Re: [RFC] WRITE_ONCE_INC() and friends

[Petko Manolov]

On 20-04-20 08:05:45, Paul E. McKenney wrote:
> On Sun, Apr 19, 2020 at 09:37:10PM +0000, David Laight wrote:
> > From: Petko Manolov
> > > Sent: 19 April 2020 19:30
> > > 
> > > On 20-04-19 18:02:50, David Laight wrote:
> > > > From: Petko Manolov
> > > > > Sent: 19 April 2020 10:45
> > > > > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > > > >
> > > > > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > > > > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> > > >
> > > > If all the accesses use READ/WRITE_ONCE() why not just mark the structure
> > > > field 'volatile'?
> > > 
> > > This is a bit heavy.  I guess you've read this one:
> > > 
> > > 	https://lwn.net/Articles/233479/
> > 
> > I remember reading something similar before.
> > I also remember a very old gcc (2.95?) that did a readback
> > after every volatile write on sparc (to flush the store buffer).
> > That broke everything.
> > 
> > I suspect there is a lot more code that is attempting to be lockless
> > these days.
> > Ring buffers (one writer and one reader) are a typical example where
> > you don't need locks but do need to use a consistent value.
> > 
> > Now you may also need ordering between accesses - which I think needs
> > more than volatile.
> 
> In Petko's patch, all needed ordering is supplied by the fact that it is the 
> same variable being read and written.  But yes, in many other cases, more 
> ordering is required.

There's pros and cons, as usual.  Yet another macro(s) versus sorter/more 
readable code.  This is why i decided to spam the list (and Paul) - in search 
for another opinion.

> > > And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for
> > > example, all others are from withing spin_lock/unlock pairs then we _may_ not
> > > need READ/WRITE_ONCE().
> > 
> > The cost of volatile accesses is probably minimal unless the
> > code is written assuming the compiler will only access things once.
> 
> And there are variables marked as volatile, for example, jiffies.
> 
> But one downside of declaring variables volatile is that it can prevent KCSAN 
> from spotting violations of the concurrency design for those variables.

Which would be unfortunate.

I just wish there was C type declaration that would force the compiler to do 
what READ/WRITE_ONCE() does now, but i also know this is too naive... :)


		Petko


> > > I merely proposed the _INC() variant for better readability.
> > 
> > More like shorter code lines :-)
> 
> That too!  ;-)
> 
> 							Thanx, Paul

RE: [RFC] WRITE_ONCE_INC() and friends

[David Laight]

From: Petko Manolov
> Sent: 20 April 2020 17:32
...
> > But one downside of declaring variables volatile is that it can prevent KCSAN
> > from spotting violations of the concurrency design for those variables.
> 
> Which would be unfortunate.
> 
> I just wish there was C type declaration that would force the compiler to do
> what READ/WRITE_ONCE() does now, but i also know this is too naive... :)

It does, it is called 'volatile'.

All READ_ONCE() does is force the access through a volatile pointer.

The thing to do is mark the structure elements 'volatile'
rather than using a 'pointer to volatile structure'.

I'm sure KASAN could be taught about volatile structure members.

	David.

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [RFC] WRITE_ONCE_INC() and friends

[Petko Manolov]

On 20-04-21 08:00:48, David Laight wrote:
> From: Petko Manolov
> > Sent: 20 April 2020 17:32
> ...
> > > But one downside of declaring variables volatile is that it can prevent 
> > > KCSAN from spotting violations of the concurrency design for those 
> > > variables.
> > 
> > Which would be unfortunate.
> > 
> > I just wish there was C type declaration that would force the compiler to do 
> > what READ/WRITE_ONCE() does now, but i also know this is too naive... :)
> 
> It does, it is called 'volatile'.

Well, it doesn't.  I'd hate it if this thread become too academic so please 
read:

	https://lwn.net/Articles/233482/

I am really bad in naming things, but assume that something like "shared int a" 
tells the compiler everything what READ_ONCE(a) would if it was only "int a".  
Here "shared" is the magic qualifier.

	shared int a, b;

	a = x;
	y = b;

The compiler should also be forbidden from reordering these two statements, 
perform exactly one store to "a", exactly one load from "b", etc, etc.  IOW just 
like with:

	WRITE_ONCE(a, x);
	y = READ_ONCE(b);

> All READ_ONCE() does is force the access through a volatile pointer.

It does a lot more than that.  Again, please read the comments in:

	include/linux/compiler.h

> The thing to do is mark the structure elements 'volatile' rather than using a 
> 'pointer to volatile structure'.

Err, don't see the point in either of these.  It won't replace the need of 
locking or using some sort of barrier if these were to be used as some sort of 
shared memory.

> I'm sure KASAN could be taught about volatile structure members.

That it may.


		Petko

Re: [RFC] WRITE_ONCE_INC() and friends

[Marco Elver]

On Mon, 20 Apr 2020, Paul E. McKenney wrote:

> On Sun, Apr 19, 2020 at 09:37:10PM +0000, David Laight wrote:
> > From: Petko Manolov
> > > Sent: 19 April 2020 19:30
> > > 
> > > On 20-04-19 18:02:50, David Laight wrote:
> > > > From: Petko Manolov
> > > > > Sent: 19 April 2020 10:45
> > > > > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > > > >
> > > > > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > > > > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> > > >
> > > > If all the accesses use READ/WRITE_ONCE() why not just mark the structure
> > > > field 'volatile'?
> > > 
> > > This is a bit heavy.  I guess you've read this one:
> > > 
> > > 	https://lwn.net/Articles/233479/
> > 
> > I remember reading something similar before.
> > I also remember a very old gcc (2.95?) that did a readback
> > after every volatile write on sparc (to flush the store buffer).
> > That broke everything.
> > 
> > I suspect there is a lot more code that is attempting to be lockless
> > these days.
> > Ring buffers (one writer and one reader) are a typical example where
> > you don't need locks but do need to use a consistent value.
> > 
> > Now you may also need ordering between accesses - which I think needs
> > more than volatile.
> 
> In Petko's patch, all needed ordering is supplied by the fact that it
> is the same variable being read and written.  But yes, in many other
> cases, more ordering is required.
> 
> > > And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for
> > > example, all others are from withing spin_lock/unlock pairs then we _may_ not
> > > need READ/WRITE_ONCE().
> > 
> > The cost of volatile accesses is probably minimal unless the
> > code is written assuming the compiler will only access things once.
> 
> And there are variables marked as volatile, for example, jiffies.
> 
> But one downside of declaring variables volatile is that it can prevent
> KCSAN from spotting violations of the concurrency design for those
> variables.

Note that, KCSAN currently treats volatiles not as special, except a
list of some known global volatiles (like jiffies). This means, that
KCSAN will tell us about data races involving unmarked volatiles (unless
they're in the list).

As far as I can tell, this is what we want. At least according to LKMM.

If, for whatever reason, volatiles should be treated differently, we'll
have to modify the compilers to emit different instrumentation for the
kernel.

Thanks,
-- Marco

Re: [RFC] WRITE_ONCE_INC() and friends

[Paul E. McKenney]

On Tue, Apr 21, 2020 at 12:57:15AM +0200, Marco Elver wrote:
> On Mon, 20 Apr 2020, Paul E. McKenney wrote:
> 
> > On Sun, Apr 19, 2020 at 09:37:10PM +0000, David Laight wrote:
> > > From: Petko Manolov
> > > > Sent: 19 April 2020 19:30
> > > > 
> > > > On 20-04-19 18:02:50, David Laight wrote:
> > > > > From: Petko Manolov
> > > > > > Sent: 19 April 2020 10:45
> > > > > > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > > > > >
> > > > > > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > > > > > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> > > > >
> > > > > If all the accesses use READ/WRITE_ONCE() why not just mark the structure
> > > > > field 'volatile'?
> > > > 
> > > > This is a bit heavy.  I guess you've read this one:
> > > > 
> > > > 	https://lwn.net/Articles/233479/
> > > 
> > > I remember reading something similar before.
> > > I also remember a very old gcc (2.95?) that did a readback
> > > after every volatile write on sparc (to flush the store buffer).
> > > That broke everything.
> > > 
> > > I suspect there is a lot more code that is attempting to be lockless
> > > these days.
> > > Ring buffers (one writer and one reader) are a typical example where
> > > you don't need locks but do need to use a consistent value.
> > > 
> > > Now you may also need ordering between accesses - which I think needs
> > > more than volatile.
> > 
> > In Petko's patch, all needed ordering is supplied by the fact that it
> > is the same variable being read and written.  But yes, in many other
> > cases, more ordering is required.
> > 
> > > > And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for
> > > > example, all others are from withing spin_lock/unlock pairs then we _may_ not
> > > > need READ/WRITE_ONCE().
> > > 
> > > The cost of volatile accesses is probably minimal unless the
> > > code is written assuming the compiler will only access things once.
> > 
> > And there are variables marked as volatile, for example, jiffies.
> > 
> > But one downside of declaring variables volatile is that it can prevent
> > KCSAN from spotting violations of the concurrency design for those
> > variables.
> 
> Note that, KCSAN currently treats volatiles not as special, except a
> list of some known global volatiles (like jiffies). This means, that
> KCSAN will tell us about data races involving unmarked volatiles (unless
> they're in the list).
> 
> As far as I can tell, this is what we want. At least according to LKMM.
> 
> If, for whatever reason, volatiles should be treated differently, we'll
> have to modify the compilers to emit different instrumentation for the
> kernel.

I stand corrected, then, thank you!

In the current arrangement, declaring a variable volatile will cause
KCSAN to generate lots of false positives.

I don't currently have a strong feeling on changing the current situation
with respect to volatile variables.  Is there a strong reason to change?
The general view of the community, as you say, has been that you don't use
the volatile keyword outside of exceptions such as jiffies, atomic_read(),
atomic_set(), READ_ONCE(), WRITE_ONCE() and perhaps a few others.

Thoughts?

							Thanx, Paul

Re: [RFC] WRITE_ONCE_INC() and friends

[Marco Elver]

On Tue, 21 Apr 2020 at 01:12, Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Tue, Apr 21, 2020 at 12:57:15AM +0200, Marco Elver wrote:
> > On Mon, 20 Apr 2020, Paul E. McKenney wrote:
> >
> > > On Sun, Apr 19, 2020 at 09:37:10PM +0000, David Laight wrote:
> > > > From: Petko Manolov
> > > > > Sent: 19 April 2020 19:30
> > > > >
> > > > > On 20-04-19 18:02:50, David Laight wrote:
> > > > > > From: Petko Manolov
> > > > > > > Sent: 19 April 2020 10:45
> > > > > > > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > > > > > >
> > > > > > > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > > > > > > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> > > > > >
> > > > > > If all the accesses use READ/WRITE_ONCE() why not just mark the structure
> > > > > > field 'volatile'?
> > > > >
> > > > > This is a bit heavy.  I guess you've read this one:
> > > > >
> > > > >         https://lwn.net/Articles/233479/
> > > >
> > > > I remember reading something similar before.
> > > > I also remember a very old gcc (2.95?) that did a readback
> > > > after every volatile write on sparc (to flush the store buffer).
> > > > That broke everything.
> > > >
> > > > I suspect there is a lot more code that is attempting to be lockless
> > > > these days.
> > > > Ring buffers (one writer and one reader) are a typical example where
> > > > you don't need locks but do need to use a consistent value.
> > > >
> > > > Now you may also need ordering between accesses - which I think needs
> > > > more than volatile.
> > >
> > > In Petko's patch, all needed ordering is supplied by the fact that it
> > > is the same variable being read and written.  But yes, in many other
> > > cases, more ordering is required.
> > >
> > > > > And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for
> > > > > example, all others are from withing spin_lock/unlock pairs then we _may_ not
> > > > > need READ/WRITE_ONCE().
> > > >
> > > > The cost of volatile accesses is probably minimal unless the
> > > > code is written assuming the compiler will only access things once.
> > >
> > > And there are variables marked as volatile, for example, jiffies.
> > >
> > > But one downside of declaring variables volatile is that it can prevent
> > > KCSAN from spotting violations of the concurrency design for those
> > > variables.
> >
> > Note that, KCSAN currently treats volatiles not as special, except a
> > list of some known global volatiles (like jiffies). This means, that
> > KCSAN will tell us about data races involving unmarked volatiles (unless
> > they're in the list).
> >
> > As far as I can tell, this is what we want. At least according to LKMM.
> >
> > If, for whatever reason, volatiles should be treated differently, we'll
> > have to modify the compilers to emit different instrumentation for the
> > kernel.
>
> I stand corrected, then, thank you!
>
> In the current arrangement, declaring a variable volatile will cause
> KCSAN to generate lots of false positives.
>
> I don't currently have a strong feeling on changing the current situation
> with respect to volatile variables.  Is there a strong reason to change?
> The general view of the community, as you say, has been that you don't use
> the volatile keyword outside of exceptions such as jiffies, atomic_read(),
> atomic_set(), READ_ONCE(), WRITE_ONCE() and perhaps a few others.
>
> Thoughts?

I certainly agree, and also want to point out that checkpatch.pl
complains about volatile. We know using volatile has problems. KCSAN
is (along with checkpatch.pl) another tool that can warn us about such
problems (warning in case there is real concurrency). Another thing to
point out is that volatile is not portable, in case
READ_ONCE()/WRITE_ONCE()'s smp_load_barrier_depends() is not a noop.
So from what I see, there are strong reasons against changing the
situation for volatiles and KCSAN.

Thanks,
-- Marco

Re: [RFC] WRITE_ONCE_INC() and friends

[Paul E. McKenney]

On Tue, Apr 21, 2020 at 11:33:57AM +0200, Marco Elver wrote:
> On Tue, 21 Apr 2020 at 01:12, Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Tue, Apr 21, 2020 at 12:57:15AM +0200, Marco Elver wrote:
> > > On Mon, 20 Apr 2020, Paul E. McKenney wrote:
> > >
> > > > On Sun, Apr 19, 2020 at 09:37:10PM +0000, David Laight wrote:
> > > > > From: Petko Manolov
> > > > > > Sent: 19 April 2020 19:30
> > > > > >
> > > > > > On 20-04-19 18:02:50, David Laight wrote:
> > > > > > > From: Petko Manolov
> > > > > > > > Sent: 19 April 2020 10:45
> > > > > > > > Recently I started reading up on KCSAN and at some point I ran into stuff like:
> > > > > > > >
> > > > > > > > WRITE_ONCE(ssp->srcu_lock_nesting[idx], ssp->srcu_lock_nesting[idx] + 1);
> > > > > > > > WRITE_ONCE(p->mm->numa_scan_seq, READ_ONCE(p->mm->numa_scan_seq) + 1);
> > > > > > >
> > > > > > > If all the accesses use READ/WRITE_ONCE() why not just mark the structure
> > > > > > > field 'volatile'?
> > > > > >
> > > > > > This is a bit heavy.  I guess you've read this one:
> > > > > >
> > > > > >         https://lwn.net/Articles/233479/
> > > > >
> > > > > I remember reading something similar before.
> > > > > I also remember a very old gcc (2.95?) that did a readback
> > > > > after every volatile write on sparc (to flush the store buffer).
> > > > > That broke everything.
> > > > >
> > > > > I suspect there is a lot more code that is attempting to be lockless
> > > > > these days.
> > > > > Ring buffers (one writer and one reader) are a typical example where
> > > > > you don't need locks but do need to use a consistent value.
> > > > >
> > > > > Now you may also need ordering between accesses - which I think needs
> > > > > more than volatile.
> > > >
> > > > In Petko's patch, all needed ordering is supplied by the fact that it
> > > > is the same variable being read and written.  But yes, in many other
> > > > cases, more ordering is required.
> > > >
> > > > > > And no, i am not sure all accesses are through READ/WRITE_ONCE().  If, for
> > > > > > example, all others are from withing spin_lock/unlock pairs then we _may_ not
> > > > > > need READ/WRITE_ONCE().
> > > > >
> > > > > The cost of volatile accesses is probably minimal unless the
> > > > > code is written assuming the compiler will only access things once.
> > > >
> > > > And there are variables marked as volatile, for example, jiffies.
> > > >
> > > > But one downside of declaring variables volatile is that it can prevent
> > > > KCSAN from spotting violations of the concurrency design for those
> > > > variables.
> > >
> > > Note that, KCSAN currently treats volatiles not as special, except a
> > > list of some known global volatiles (like jiffies). This means, that
> > > KCSAN will tell us about data races involving unmarked volatiles (unless
> > > they're in the list).
> > >
> > > As far as I can tell, this is what we want. At least according to LKMM.
> > >
> > > If, for whatever reason, volatiles should be treated differently, we'll
> > > have to modify the compilers to emit different instrumentation for the
> > > kernel.
> >
> > I stand corrected, then, thank you!
> >
> > In the current arrangement, declaring a variable volatile will cause
> > KCSAN to generate lots of false positives.
> >
> > I don't currently have a strong feeling on changing the current situation
> > with respect to volatile variables.  Is there a strong reason to change?
> > The general view of the community, as you say, has been that you don't use
> > the volatile keyword outside of exceptions such as jiffies, atomic_read(),
> > atomic_set(), READ_ONCE(), WRITE_ONCE() and perhaps a few others.
> >
> > Thoughts?
> 
> I certainly agree, and also want to point out that checkpatch.pl
> complains about volatile. We know using volatile has problems. KCSAN
> is (along with checkpatch.pl) another tool that can warn us about such
> problems (warning in case there is real concurrency). Another thing to
> point out is that volatile is not portable, in case
> READ_ONCE()/WRITE_ONCE()'s smp_load_barrier_depends() is not a noop.
> So from what I see, there are strong reasons against changing the
> situation for volatiles and KCSAN.

All good points, thank you!

							Thanx, Paul