From: Waiman Long <waiman.long@hpe.com>
To: Will Deacon <will.deacon@arm.com>
Cc: Boqun Feng <boqun.feng@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, <linux-kernel@vger.kernel.org>,
<x86@kernel.org>, <linux-alpha@vger.kernel.org>,
<linux-ia64@vger.kernel.org>, <linux-s390@vger.kernel.org>,
<linux-arch@vger.kernel.org>, Davidlohr Bueso <dave@stgolabs.net>,
Jason Low <jason.low2@hp.com>, Dave Chinner <david@fromorbit.com>,
Scott J Norton <scott.norton@hpe.com>,
Douglas Hatch <doug.hatch@hpe.com>
Subject: Re: [RFC PATCH-tip v2 1/6] locking/osq: Make lock/unlock proper acquire/release barrier
Date: Fri, 17 Jun 2016 14:17:27 -0400 [thread overview]
Message-ID: <57643EB7.6030600@hpe.com> (raw)
In-Reply-To: <20160617154536.GB1284@arm.com>
On 06/17/2016 11:45 AM, Will Deacon wrote:
> On Fri, Jun 17, 2016 at 11:26:41AM -0400, Waiman Long wrote:
>> On 06/16/2016 08:48 PM, Boqun Feng wrote:
>>> On Thu, Jun 16, 2016 at 05:35:54PM -0400, Waiman Long wrote:
>>>> If you look into the actual code:
>>>>
>>>> next = xchg_release(&node->next, NULL);
>>>> if (next) {
>>>> WRITE_ONCE(next->locked, 1);
>>>> return;
>>>> }
>>>>
>>>> There is a control dependency that WRITE_ONCE() won't happen until
>>> But a control dependency only orders LOAD->STORE pairs, right? And here
>>> the control dependency orders the LOAD part of xchg_release() and the
>>> WRITE_ONCE().
>>>
>>> Along with the fact that RELEASE only orders the STORE part of xchg with
>>> the memory operations preceding the STORE part, so for the following
>>> code:
>>>
>>> WRTIE_ONCE(x,1);
>>> next = xchg_release(&node->next, NULL);
>>> if (next)
>>> WRITE_ONCE(next->locked, 1);
>>>
>>> such a reordering is allowed to happen on ARM64v8
>>>
>>> next = ldxr [&node->next] // LOAD part of xchg_release()
>>>
>>> if (next)
>>> WRITE_ONCE(next->locked, 1);
>>>
>>> WRITE_ONCE(x,1);
>>> stlxr NULL [&node->next] // STORE part of xchg_releae()
>>>
>>> Am I missing your point here?
>> My understanding of the release barrier is that both prior LOADs and STOREs
>> can't move after the barrier. If WRITE_ONCE(x, 1) can move to below as shown
>> above, it is not a real release barrier and we may need to change the
>> barrier code.
> You seem to be missing the point.
>
> {READ,WRITE}_ONCE accesses appearing in program order after a release
> are not externally ordered with respect to the release unless they
> access the same location.
>
> This is illustrated by Boqun's example, which shows two WRITE_ONCE
> accesses being reordered before a store-release forming the write
> component of an xchg_release. In both cases, WRITE_ONCE(x, 1) remains
> ordered before the store-release.
>
> Will
I am sorry that I misread the mail. I am not used to treating xchg as
two separate instructions. Yes, it is a problem. In that case, we have
to either keep the xchg() function as it is or use
smp_store_release(&next->locked, 1). So which one is a better
alternative for ARM or PPC?
Cheers,
Longman
WARNING: multiple messages have this Message-ID (diff)
From: Waiman Long <waiman.long@hpe.com>
To: Will Deacon <will.deacon@arm.com>
Cc: Boqun Feng <boqun.feng@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, x86@kernel.org,
linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-s390@vger.kernel.org, linux-arch@vger.kernel.org,
Davidlohr Bueso <dave@stgolabs.net>,
Jason Low <jason.low2@hp.com>, Dave Chinner <david@fromorbit.com>,
Scott J Norton <scott.norton@hpe.com>,
Douglas Hatch <doug.hatch@hpe.com>
Subject: Re: [RFC PATCH-tip v2 1/6] locking/osq: Make lock/unlock proper acquire/release barrier
Date: Fri, 17 Jun 2016 14:17:27 -0400 [thread overview]
Message-ID: <57643EB7.6030600@hpe.com> (raw)
In-Reply-To: <20160617154536.GB1284@arm.com>
On 06/17/2016 11:45 AM, Will Deacon wrote:
> On Fri, Jun 17, 2016 at 11:26:41AM -0400, Waiman Long wrote:
>> On 06/16/2016 08:48 PM, Boqun Feng wrote:
>>> On Thu, Jun 16, 2016 at 05:35:54PM -0400, Waiman Long wrote:
>>>> If you look into the actual code:
>>>>
>>>> next = xchg_release(&node->next, NULL);
>>>> if (next) {
>>>> WRITE_ONCE(next->locked, 1);
>>>> return;
>>>> }
>>>>
>>>> There is a control dependency that WRITE_ONCE() won't happen until
>>> But a control dependency only orders LOAD->STORE pairs, right? And here
>>> the control dependency orders the LOAD part of xchg_release() and the
>>> WRITE_ONCE().
>>>
>>> Along with the fact that RELEASE only orders the STORE part of xchg with
>>> the memory operations preceding the STORE part, so for the following
>>> code:
>>>
>>> WRTIE_ONCE(x,1);
>>> next = xchg_release(&node->next, NULL);
>>> if (next)
>>> WRITE_ONCE(next->locked, 1);
>>>
>>> such a reordering is allowed to happen on ARM64v8
>>>
>>> next = ldxr [&node->next] // LOAD part of xchg_release()
>>>
>>> if (next)
>>> WRITE_ONCE(next->locked, 1);
>>>
>>> WRITE_ONCE(x,1);
>>> stlxr NULL [&node->next] // STORE part of xchg_releae()
>>>
>>> Am I missing your point here?
>> My understanding of the release barrier is that both prior LOADs and STOREs
>> can't move after the barrier. If WRITE_ONCE(x, 1) can move to below as shown
>> above, it is not a real release barrier and we may need to change the
>> barrier code.
> You seem to be missing the point.
>
> {READ,WRITE}_ONCE accesses appearing in program order after a release
> are not externally ordered with respect to the release unless they
> access the same location.
>
> This is illustrated by Boqun's example, which shows two WRITE_ONCE
> accesses being reordered before a store-release forming the write
> component of an xchg_release. In both cases, WRITE_ONCE(x, 1) remains
> ordered before the store-release.
>
> Will
I am sorry that I misread the mail. I am not used to treating xchg as
two separate instructions. Yes, it is a problem. In that case, we have
to either keep the xchg() function as it is or use
smp_store_release(&next->locked, 1). So which one is a better
alternative for ARM or PPC?
Cheers,
Longman
WARNING: multiple messages have this Message-ID (diff)
From: Waiman Long <waiman.long@hpe.com>
To: Will Deacon <will.deacon@arm.com>
Cc: Boqun Feng <boqun.feng@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, x86@kernel.org,
linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-s390@vger.kernel.org, linux-arch@vger.kernel.org,
Davidlohr Bueso <dave@stgolabs.net>,
Jason Low <jason.low2@hp.com>, Dave Chinner <david@fromorbit.com>,
Scott J Norton <scott.norton@hpe.com>,
Douglas Hatch <doug.hatch@hpe.com>
Subject: Re: [RFC PATCH-tip v2 1/6] locking/osq: Make lock/unlock proper acquire/release barrier
Date: Fri, 17 Jun 2016 18:17:27 +0000 [thread overview]
Message-ID: <57643EB7.6030600@hpe.com> (raw)
In-Reply-To: <20160617154536.GB1284@arm.com>
On 06/17/2016 11:45 AM, Will Deacon wrote:
> On Fri, Jun 17, 2016 at 11:26:41AM -0400, Waiman Long wrote:
>> On 06/16/2016 08:48 PM, Boqun Feng wrote:
>>> On Thu, Jun 16, 2016 at 05:35:54PM -0400, Waiman Long wrote:
>>>> If you look into the actual code:
>>>>
>>>> next = xchg_release(&node->next, NULL);
>>>> if (next) {
>>>> WRITE_ONCE(next->locked, 1);
>>>> return;
>>>> }
>>>>
>>>> There is a control dependency that WRITE_ONCE() won't happen until
>>> But a control dependency only orders LOAD->STORE pairs, right? And here
>>> the control dependency orders the LOAD part of xchg_release() and the
>>> WRITE_ONCE().
>>>
>>> Along with the fact that RELEASE only orders the STORE part of xchg with
>>> the memory operations preceding the STORE part, so for the following
>>> code:
>>>
>>> WRTIE_ONCE(x,1);
>>> next = xchg_release(&node->next, NULL);
>>> if (next)
>>> WRITE_ONCE(next->locked, 1);
>>>
>>> such a reordering is allowed to happen on ARM64v8
>>>
>>> next = ldxr [&node->next] // LOAD part of xchg_release()
>>>
>>> if (next)
>>> WRITE_ONCE(next->locked, 1);
>>>
>>> WRITE_ONCE(x,1);
>>> stlxr NULL [&node->next] // STORE part of xchg_releae()
>>>
>>> Am I missing your point here?
>> My understanding of the release barrier is that both prior LOADs and STOREs
>> can't move after the barrier. If WRITE_ONCE(x, 1) can move to below as shown
>> above, it is not a real release barrier and we may need to change the
>> barrier code.
> You seem to be missing the point.
>
> {READ,WRITE}_ONCE accesses appearing in program order after a release
> are not externally ordered with respect to the release unless they
> access the same location.
>
> This is illustrated by Boqun's example, which shows two WRITE_ONCE
> accesses being reordered before a store-release forming the write
> component of an xchg_release. In both cases, WRITE_ONCE(x, 1) remains
> ordered before the store-release.
>
> Will
I am sorry that I misread the mail. I am not used to treating xchg as
two separate instructions. Yes, it is a problem. In that case, we have
to either keep the xchg() function as it is or use
smp_store_release(&next->locked, 1). So which one is a better
alternative for ARM or PPC?
Cheers,
Longman
next prev parent reply other threads:[~2016-06-17 18:17 UTC|newest]
Thread overview: 101+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-14 22:48 [RFC PATCH-tip v2 0/6] locking/rwsem: Enable reader optimistic spinning Waiman Long
2016-06-14 22:48 ` Waiman Long
2016-06-14 22:48 ` [RFC PATCH-tip v2 1/6] locking/osq: Make lock/unlock proper acquire/release barrier Waiman Long
2016-06-14 22:48 ` Waiman Long
2016-06-15 8:04 ` Boqun Feng
2016-06-15 8:04 ` Boqun Feng
2016-06-15 17:18 ` Peter Zijlstra
2016-06-15 17:18 ` Peter Zijlstra
2016-06-15 19:01 ` Waiman Long
2016-06-15 19:01 ` Waiman Long
2016-06-15 19:01 ` Waiman Long
2016-06-16 2:19 ` Boqun Feng
2016-06-16 2:19 ` Boqun Feng
2016-06-16 10:16 ` Will Deacon
2016-06-16 10:16 ` Will Deacon
2016-06-16 21:35 ` Waiman Long
2016-06-16 21:35 ` Waiman Long
2016-06-16 21:35 ` Waiman Long
2016-06-17 0:48 ` Boqun Feng
2016-06-17 0:48 ` Boqun Feng
2016-06-17 15:26 ` Waiman Long
2016-06-17 15:26 ` Waiman Long
2016-06-17 15:26 ` Waiman Long
2016-06-17 15:45 ` Will Deacon
2016-06-17 15:45 ` Will Deacon
2016-06-17 18:17 ` Waiman Long [this message]
2016-06-17 18:17 ` Waiman Long
2016-06-17 18:17 ` Waiman Long
2016-06-18 8:46 ` Boqun Feng
2016-06-18 8:46 ` Boqun Feng
2016-06-20 7:59 ` Will Deacon
2016-06-20 7:59 ` Will Deacon
2016-06-15 16:56 ` Davidlohr Bueso
2016-06-15 16:56 ` Davidlohr Bueso
2016-06-15 17:12 ` Peter Zijlstra
2016-06-15 17:12 ` Peter Zijlstra
2016-06-15 18:27 ` Davidlohr Bueso
2016-06-15 18:27 ` Davidlohr Bueso
2016-06-15 18:40 ` Peter Zijlstra
2016-06-15 18:40 ` Peter Zijlstra
2016-06-15 18:56 ` Davidlohr Bueso
2016-06-15 18:56 ` Davidlohr Bueso
2016-06-17 1:11 ` Davidlohr Bueso
2016-06-17 1:11 ` Davidlohr Bueso
2016-06-17 14:28 ` Waiman Long
2016-06-17 14:28 ` Waiman Long
2016-06-17 14:28 ` Waiman Long
2016-06-17 16:29 ` Davidlohr Bueso
2016-06-17 16:29 ` Davidlohr Bueso
2016-06-17 16:46 ` Davidlohr Bueso
2016-06-17 16:46 ` Davidlohr Bueso
2016-06-15 19:08 ` Waiman Long
2016-06-15 19:08 ` Waiman Long
2016-06-15 19:08 ` Waiman Long
2016-06-15 20:04 ` Waiman Long
2016-06-15 20:04 ` Waiman Long
2016-06-15 20:04 ` Waiman Long
2016-06-15 21:59 ` Peter Zijlstra
2016-06-15 21:59 ` Peter Zijlstra
2016-06-14 22:48 ` [RFC PATCH-tip v2 2/6] locking/rwsem: Stop active read lock ASAP Waiman Long
2016-06-14 22:48 ` Waiman Long
2016-06-15 17:22 ` Peter Zijlstra
2016-06-15 17:22 ` Peter Zijlstra
2016-06-15 19:17 ` Waiman Long
2016-06-15 19:17 ` Waiman Long
2016-06-15 19:17 ` Waiman Long
2016-06-16 2:14 ` Davidlohr Bueso
2016-06-16 2:14 ` Davidlohr Bueso
2016-06-16 21:25 ` Waiman Long
2016-06-16 21:25 ` Waiman Long
2016-06-16 21:25 ` Waiman Long
2016-06-14 22:48 ` [RFC PATCH-tip v2 3/6] locking/rwsem: Enable count-based spinning on reader Waiman Long
2016-06-14 22:48 ` Waiman Long
2016-06-15 17:38 ` Peter Zijlstra
2016-06-15 17:38 ` Peter Zijlstra
2016-06-15 19:28 ` Waiman Long
2016-06-15 19:28 ` Waiman Long
2016-06-15 19:28 ` Waiman Long
2016-06-14 22:48 ` [RFC PATCH-tip v2 4/6] locking/rwsem: move down rwsem_down_read_failed function Waiman Long
2016-06-14 22:48 ` Waiman Long
2016-06-15 17:40 ` Peter Zijlstra
2016-06-15 17:40 ` Peter Zijlstra
2016-06-15 19:21 ` Waiman Long
2016-06-15 19:21 ` Waiman Long
2016-06-15 19:21 ` Waiman Long
2016-06-14 22:48 ` [RFC PATCH-tip v2 5/6] locking/rwsem: Change RWSEM_WAITING_BIAS for better disambiguation Waiman Long
2016-06-14 22:48 ` Waiman Long
2016-06-15 17:43 ` Peter Zijlstra
2016-06-15 17:43 ` Peter Zijlstra
2016-06-15 19:31 ` Waiman Long
2016-06-15 19:31 ` Waiman Long
2016-06-15 19:31 ` Waiman Long
2016-06-15 21:57 ` Peter Zijlstra
2016-06-15 21:57 ` Peter Zijlstra
2016-06-15 17:45 ` Peter Zijlstra
2016-06-15 17:45 ` Peter Zijlstra
2016-06-15 19:35 ` Waiman Long
2016-06-15 19:35 ` Waiman Long
2016-06-15 19:35 ` Waiman Long
2016-06-14 22:48 ` [RFC PATCH-tip v2 6/6] locking/rwsem: Enable spinning readers Waiman Long
2016-06-14 22:48 ` Waiman Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57643EB7.6030600@hpe.com \
--to=waiman.long@hpe.com \
--cc=boqun.feng@gmail.com \
--cc=dave@stgolabs.net \
--cc=david@fromorbit.com \
--cc=doug.hatch@hpe.com \
--cc=jason.low2@hp.com \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=scott.norton@hpe.com \
--cc=will.deacon@arm.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.