Message-ID: <55A7C7FE.5080907@hp.com>
Date: Thu, 16 Jul 2015 11:04:30 -0400
From: Waiman Long <waiman.long@hp.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12
MIME-Version: 1.0
To: Peter Zijlstra <peterz@infradead.org>
CC: Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org, Scott J Norton <scott.norton@hp.com>,
        Douglas Hatch <doug.hatch@hp.com>, Davidlohr Bueso <dave@stgolabs.net>,
        will.deacon@arm.com
Subject: Re: [PATCH v2 1/6] locking/pvqspinlock: Unconditional PV kick with
 _Q_SLOW_VAL
References: <1436926417-20256-1-git-send-email-Waiman.Long@hp.com> <1436926417-20256-2-git-send-email-Waiman.Long@hp.com> <20150715091012.GG2859@worktop.programming.kicks-ass.net> <55A6F84F.5050502@hp.com> <20150716054214.GU19282@twins.programming.kicks-ass.net> <55A7BABE.2070507@hp.com>
In-Reply-To: <55A7BABE.2070507@hp.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2029
Lines: 49

On 07/16/2015 10:07 AM, Waiman Long wrote:
> 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.
>

I suspect that there may be other places in the kernel that have similar 
problem. An alternative will be to strengthen cmpxchg() in architectures 
like arm64 to act as full memory barrier no matter the result and 
defined relaxed version with no such guarantee.

Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/