Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1400924

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

Show all headers | View raw

On 05/13/2016 08:07 AM, Peter Zijlstra wrote:
> 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.

If load tearing a naturally aligned pointer is a real code generation
possibility then the rcu list code is broken too (which loads ->next
directly; cf. list_for_each_entry_rcu() & list_for_each_entry_lockless()).

For 4.4, Paul added READ_ONCE() checks for list_empty() et al, but iirc
that had to do with control dependencies and not load tearing.

OTOH, this patch might actually produce store-tearing:

+static inline void rwsem_set_reader_owned(struct rw_semaphore *sem)
+{
+	/*
+	 * We check the owner value first to make sure that we will only
+	 * do a write to the rwsem cacheline when it is really necessary
+	 * to minimize cacheline contention.
+	 */
+	if (sem->owner != RWSEM_READER_OWNED)
+		sem->owner = RWSEM_READER_OWNED;
+}


Regards,
Peter Hurley

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Zijlstra <peterz@infradead.org> - 2016-05-13 17:10 +0200
  Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Hurley <peter@hurleysoftware.com> - 2016-05-13 20:00 +0200
    Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Zijlstra <peterz@infradead.org> - 2016-05-16 13:20 +0200
      Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-16 14:20 +0200
        Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Hurley <peter@hurleysoftware.com> - 2016-05-16 16:20 +0200
          Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-16 19:30 +0200
            Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Hurley <peter@hurleysoftware.com> - 2016-05-17 21:50 +0200
              Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Hurley <peter@hurleysoftware.com> - 2016-05-17 22:00 +0200
          Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Zijlstra <peterz@infradead.org> - 2016-05-16 20:00 +0200
            Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Hurley <peter@hurleysoftware.com> - 2016-05-17 21:20 +0200
              Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-17 21:50 +0200
                Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Zijlstra <peterz@infradead.org> - 2016-05-18 13:10 +0200
                Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Waiman Long <waiman.long@hpe.com> - 2016-05-18 18:00 +0200
                Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-18 19:30 +0200
                Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-18 19:30 +0200
                Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field Peter Zijlstra <peterz@infradead.org> - 2016-05-19 11:10 +0200
                Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner  field "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-19 15:50 +0200

csiph-web