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

From: Peter Zijlstra
Date: Fri May 13 2016 - 11:08:11 EST


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.