From: will.deacon@arm•com (Will Deacon)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] arm64: spinlock: serialise spin_unlock_wait against concurrent lockers
Date: Thu, 3 Dec 2015 16:32:43 +0000 [thread overview]
Message-ID: <20151203163243.GI11337@arm.com> (raw)
In-Reply-To: <20151203132839.GA3816@twins.programming.kicks-ass.net>
Hi Peter, Paul,
Firstly, thanks for writing that up. I agree that you have something
that can work in theory, but see below.
On Thu, Dec 03, 2015 at 02:28:39PM +0100, Peter Zijlstra wrote:
> On Wed, Dec 02, 2015 at 04:11:41PM -0800, Paul E. McKenney wrote:
> > This looks architecture-agnostic to me:
> >
> > a. TSO systems have smp_mb__after_unlock_lock() be a no-op, and
> > have a read-only implementation for spin_unlock_wait().
> >
> > b. Small-scale weakly ordered systems can also have
> > smp_mb__after_unlock_lock() be a no-op, but must instead
> > have spin_unlock_wait() acquire the lock and immediately
> > release it, or some optimized implementation of this.
> >
> > c. Large-scale weakly ordered systems are required to define
> > smp_mb__after_unlock_lock() as smp_mb(), but can have a
> > read-only implementation of spin_unlock_wait().
>
> This would still require all relevant spin_lock() sites to be annotated
> with smp_mb__after_unlock_lock(), which is going to be a painful (no
> warning when done wrong) exercise and expensive (added MBs all over the
> place).
>
> But yes, I think the proposal is technically sound, just not quite sure
> how far we'll want to push this.
When I said that the solution isn't architecture-agnostic, I was referring
to the fact that spin_unlock_wait acts as a LOCK operation on all
architectures other than those using case (c) above. You've resolved
this by requiring smp_mb__after_unlock_lock() after every LOCK, but I
share Peter's concerns that this isn't going to work in practice because:
1. The smp_mb__after_unlock_lock additions aren't necessarily in the
code where the spin_unlock_wait() is being added, so it's going to
require a lot of diligence for developers to get this right
2. Only PowerPC is going to see the (very occassional) failures, so
testing this is nigh on impossible :(
3. We've now made the kernel memory model even more difficult to
understand, so people might not even bother with this addition
Will
next prev parent reply other threads:[~2015-12-03 16:32 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-27 11:44 [PATCH] arm64: spinlock: serialise spin_unlock_wait against concurrent lockers Will Deacon
2015-11-30 15:58 ` Peter Zijlstra
2015-11-30 18:21 ` Paul E. McKenney
2015-12-01 16:40 ` Will Deacon
2015-12-03 0:11 ` Paul E. McKenney
2015-12-03 13:28 ` Peter Zijlstra
2015-12-03 16:32 ` Will Deacon [this message]
2015-12-03 17:22 ` Paul E. McKenney
2015-12-04 9:21 ` Peter Zijlstra
2015-12-04 16:07 ` Paul E. McKenney
2015-12-04 16:24 ` Will Deacon
2015-12-04 16:44 ` Paul E. McKenney
2015-12-06 7:37 ` Boqun Feng
2015-12-06 19:23 ` Paul E. McKenney
2015-12-06 23:28 ` Boqun Feng
2015-12-07 0:00 ` Paul E. McKenney
2015-12-07 0:45 ` Boqun Feng
2015-12-07 10:34 ` Peter Zijlstra
2015-12-07 15:45 ` Paul E. McKenney
2015-12-08 8:42 ` Boqun Feng
2015-12-08 19:17 ` Paul E. McKenney
2015-12-09 6:43 ` Boqun Feng
2015-12-04 9:36 ` Peter Zijlstra
2015-12-04 16:13 ` Paul E. McKenney
2015-12-07 2:12 ` Michael Ellerman
2015-12-06 8:16 ` Boqun Feng
2015-12-06 19:27 ` Paul E. McKenney
2015-12-07 0:26 ` Boqun Feng
2015-12-11 8:09 ` Boqun Feng
2015-12-11 9:46 ` Will Deacon
2015-12-11 12:20 ` Boqun Feng
2015-12-11 13:42 ` Paul E. McKenney
2015-12-11 13:54 ` Will Deacon
2015-12-01 0:40 ` Boqun Feng
2015-12-01 16:32 ` Will Deacon
2015-12-02 9:40 ` Boqun Feng
2015-12-02 11:16 ` Boqun Feng
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=20151203163243.GI11337@arm.com \
--to=will.deacon@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox