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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 16 May 2016 19:50:41 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Peter Hurley <peter@...leysoftware.com>
Cc:	paulmck@...ux.vnet.ibm.com, Waiman Long <Waiman.Long@....com>,
	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	Davidlohr Bueso <dave@...olabs.net>,
	Jason Low <jason.low2@...com>,
	Dave Chinner <david@...morbit.com>,
	Scott J Norton <scott.norton@....com>,
	Douglas Hatch <doug.hatch@....com>, kcc@...gle.com,
	dvyukov@...gle.com
Subject: Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner
 field

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.