Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751637AbdF2DPu (ORCPT ); Wed, 28 Jun 2017 23:15:50 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:36831 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751541AbdF2DPq (ORCPT ); Wed, 28 Jun 2017 23:15:46 -0400 Date: Thu, 29 Jun 2017 11:17:26 +0800 From: Boqun Feng To: "Paul E. McKenney" Cc: Linus Torvalds , Alan Stern , Andrea Parri , Linux Kernel Mailing List , priyalee.kushwaha@intel.com, =?utf-8?Q?Stanis=C5=82aw?= Drozd , Arnd Bergmann , ldr709@gmail.com, Thomas Gleixner , Peter Zijlstra , Josh Triplett , Nicolas Pitre , Krister Johansen , Vegard Nossum , dcb314@hotmail.com, Wu Fengguang , Frederic Weisbecker , Rik van Riel , Steven Rostedt , Ingo Molnar , Luc Maranget , Jade Alglave Subject: Re: [GIT PULL rcu/next] RCU commits for 4.13 Message-ID: <20170629031726.pb5dhjnxxiif25ma@tardis> References: <20170628170321.GQ3721@linux.vnet.ibm.com> <20170628235412.GB3721@linux.vnet.ibm.com> <20170629004556.GD3721@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="av44rvzcsn4rtwvc" Content-Disposition: inline In-Reply-To: <20170629004556.GD3721@linux.vnet.ibm.com> User-Agent: NeoMutt/20170225 (1.8.0) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5105 Lines: 121 --av44rvzcsn4rtwvc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 28, 2017 at 05:45:56PM -0700, Paul E. McKenney wrote: > On Wed, Jun 28, 2017 at 05:05:46PM -0700, Linus Torvalds wrote: > > On Wed, Jun 28, 2017 at 4:54 PM, Paul E. McKenney > > wrote: > > > > > > Linus, are you dead-set against defining spin_unlock_wait() to be > > > spin_lock + spin_unlock? For example, is the current x86 implementat= ion > > > of spin_unlock_wait() really a non-negotiable hard requirement? Or > > > would you be willing to live with the spin_lock + spin_unlock semanti= cs? > >=20 > > So I think the "same as spin_lock + spin_unlock" semantics are kind of = insane. > >=20 > > One of the issues is that the same as "spin_lock + spin_unlock" is > > basically now architecture-dependent. Is it really the > > architecture-dependent ordering you want to define this as? > >=20 > > So I just think it's a *bad* definition. If somebody wants something > > that is exactly equivalent to spin_lock+spin_unlock, then dammit, just > > do *THAT*. It's completely pointless to me to define > > spin_unlock_wait() in those terms. > >=20 > > And if it's not equivalent to the *architecture* behavior of > > spin_lock+spin_unlock, then I think it should be descibed in terms > > that aren't about the architecture implementation (so you shouldn't > > describe it as "spin_lock+spin_unlock", you should describe it in > > terms of memory barrier semantics. > >=20 > > And if we really have to use the spin_lock+spinunlock semantics for > > this, then what is the advantage of spin_unlock_wait at all, if it > > doesn't fundamentally avoid some locking overhead of just taking the > > spinlock in the first place? > >=20 > > And if we can't use a cheaper model, maybe we should just get rid of > > it entirely? > >=20 > > Finally: if the memory barrier semantics are exactly the same, and > > it's purely about avoiding some nasty contention case, I think the > > concept is broken - contention is almost never an actual issue, and if > > it is, the problem is much deeper than spin_unlock_wait(). >=20 > All good points! >=20 > I must confess that your sentence about getting rid of spin_unlock_wait() > entirely does resonate with me, especially given the repeated bouts of > "but what -exactly- is it -supposed- to do?" over the past 18 months > or so. ;-) >=20 > Just for completeness, here is a list of the definitions that have been > put forward, just in case it inspires someone to come up with something > better: >=20 > 1. spin_unlock_wait() provides only acquire semantics. Code > placed after the spin_unlock_wait() will see the effects of > all previous critical sections, but there is no guarantees for > subsequent critical sections. The x86 implementation provides > this. I -think- that the ARM and PowerPC implementations could > get rid of a memory-barrier instruction and still provide this. >=20 Yes, except we still need a smp_lwsync() in powerpc's spin_unlock_wait(). And FWIW, the two smp_mb()s in spin_unlock_wait() on PowerPC exist there just because when Peter worked on commit 726328d92a42, we decided to let the fix for spin_unlock_wait() on PowerPC(i.e. commit 6262db7c088bb ) go into the tree first to avoid some possible conflicts. And.. I forgot to do the clean-up for an aquire-semantics spin_unlock_wait() later.. ;-) I could send out the necessary fix once we have a conclusion for the semantics part. Regards, Boqun > 2. As #1 above, but a "smp_mb();spin_unlock_wait();" provides the > additional guarantee that code placed before this construct is > seen by all subsequent critical sections. The x86 implementation > provides this, as do ARM and PowerPC, but it is not clear that all > architectures do. As Alan noted, this is an extremely unnatural > definition for the current memory model. >=20 > 3. [ Just for completeness, yes, this is off the table! ] The > spin_unlock_wait() has the same semantics as a spin_lock() > followed immediately by a spin_unlock(). >=20 > 4. spin_unlock_wait() is analogous to synchronize_rcu(), where > spin_unlock_wait()'s "read-side critical sections" are the lock's > normal critical sections. This was the first definition I heard > that made any sense to me, but it turns out to be equivalent > to #3. Thus, also off the table. >=20 > Does anyone know of any other possible definitions? >=20 > Thanx, Paul >=20 --av44rvzcsn4rtwvc Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEj5IosQTPz8XU1wRHSXnow7UH+rgFAllUcUIACgkQSXnow7UH +rjaKQf8DKxvEJLW7Po4VfNM8uBZ6viZ9Ke35R1CiFZLBOULEGNNv9hBf5UQWvzH Qwx0Y36pGTZWxzOwF4ulZv2LVGgLKLYx1Z6IhHVQYibWW4QFsjn+ONrYIjEowyCP OmD5FYH7szctkM37r35GIy82H4tAP8oHSAFl4LKVfLQdl3E97rMMHz4iHcKgDnf4 7Tx74FaA7chi/V6LMnjh/JRBJFGceFhxqKPwED2RLRlFRdAH3T9gjFcHvRuNtC1W dgd1Oz5XZRgfJT2Hhp+1WHl0mtA+r6tT5Y14AKJn9bm5w4w6TFxE1HNt5BnA1up+ WdqhSFXvr3rNCkGsSqqSAbDS7oiB0Q== =TDaE -----END PGP SIGNATURE----- --av44rvzcsn4rtwvc--