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.

Powered by blists - more mailing lists