Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

[Peter Zijlstra]

On Mon, May 16, 2016 at 07:17:42AM -0700, Peter Hurley wrote:

> >> Correct; which is why we should always use {READ,WRITE}_ONCE() for
> >> anything that is used locklessly.
> > 
> > Completely agreed.  Improve readability of code by flagging lockless
> > shared-memory accesses, help checkers better find bugs, and prevent the
> > occasional compiler mischief!
> 
> I think this would be a mistake for 3 reasons:
> 
> 1. If READ_ONCE()/WRITE_ONCE() is necessary to prevent load/store tearing
>    of any normally-atomic type (char/int/long/void*), then _every_ access
>    would require READ_ONCE()/WRITE_ONCE(), thus eliminating any possibility
>    of compiler optimization (eg. eliding redundant loads) where it would
>    otherwise be possible.

Should not really be a problem I think; you already have to be very
careful when doing lockless stuff.

> 2. Makes a mess of otherwise readable code.

We have to disagree here; I think code with {READ,WRITE}_ONCE() is more
readable, as their presence is a clear indication something special is
up.

> 3. Error-prone; ie., easy to overlook in review.

lockless stuff is error prone by nature; what's your point?

> There is no practical difference between _always_ using READ_ONCE()/WRITE_ONCE()
> (to prevent tearing) and declaring the field volatile.

There is, declaring the field volatile doesn't make it stand out in the
code while reading at all; it also doesn't allow you to omit the
volatile qualifier in places where it doesn't matter.

The whole; variables aren't volatile, accesses are, thing is still very
much in effect.

> So we've come full-circle from volatile-considered-harmful.

I don't think so; the cases that document talks about are still very
much relevant and volatile should not be used for that.

But yes, our understanding of both the memory model and the
language/compiler has come a long way since then.