Re: [PATCH v2 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL

Waiman Long
2015-07-16T14:08:04Z

On 07/16/2015 01:42 AM, Peter Zijlstra wrote:
> On Wed, Jul 15, 2015 at 08:18:23PM -0400, Waiman Long wrote:
>> On 07/15/2015 05:10 AM, Peter Zijlstra wrote:
>>> /*
>>> + * A failed cmpxchg doesn't provide any memory-ordering guarantees,
>>> + * so we need a barrier to order the read of the node data in
>>> + * pv_unhash *after* we've read the lock being _Q_SLOW_VAL.
>>> + *
>>> + * Matches the cmpxchg() in pv_wait_head() setting _Q_SLOW_VAL.
>>> + */
>>> + smp_rmb();
>> According to memory_barriers.txt, cmpxchg() is a full memory barrier. It
>> didn't say a failed cmpxchg will lose its memory guarantee. So is the
>> documentation right?
> The documentation is not entirely clear on this; but there are hints
> that this is so.
>
>> Or is that true for some architectures? I think it is
>> not true for x86.
> On x86 LOCK CMPXCHG is always a sync point, but yes there are archs for
> which a failed cmpxchg does _NOT_ provide any barrier semantics.
>
> The reason I started looking was because Will made Argh64 one of those.

That is what I suspected. In that case, I am fine with the patch as
smp_rmb() is an nop in x86 anyway.

Acked-by: Waiman Long <Waiman.Long@hp.com>

BTW, I think we also need to update the documentation to make it clear
that a failed cmpxchg() or atomic_cmpxchg() may not be a full memory
barrier as most people may not be aware of that.

Cheers,
Longman