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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 13 May 2016 17:07:49 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Peter Hurley <peter@...leysoftware.com>
Cc:	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>
Subject: Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner
 field

On Wed, May 11, 2016 at 03:04:20PM -0700, Peter Hurley wrote:
> > +	return !rwsem_is_reader_owned(READ_ONCE(sem->owner));
> 
> It doesn't make sense to force reload sem->owner here; if sem->owner
> is not being reloaded then the loop above will execute forever.
> 
> Arguably, this check should be bumped out to the optimistic spin and
> reload/check the owner there?
> 

Note that barrier() and READ_ONCE() have overlapping but not identical
results and the combined use actually makes sense here.

Yes, a barrier() anywhere in the loop will force a reload of the
variable, _however_ it doesn't force that reload to not suffer from
load tearing.

Using volatile also forces a reload, but also ensures the load cannot
be torn IFF it is of machine word side and naturally aligned.

So while the READ_ONCE() here is pointless for forcing the reload;
that's already ensured, we still need to make sure the load isn't torn.