Message ID | 1492018825-25634-2-git-send-email-paulmck@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
On Wed, Apr 12, 2017 at 10:39:47AM -0700, Paul E. McKenney wrote: > CONFIG_ARCH_WEAK_RELACQ Kconfig option. Naming in the changelog and patch don't match. > +config ARCH_WEAK_RELEASE_ACQUIRE > + bool > +
On Wed, Apr 12, 2017 at 10:39:47AM -0700, Paul E. McKenney wrote: > The definition of smp_mb__after_unlock_lock() is currently smp_mb() > for CONFIG_PPC and a no-op otherwise. It would be better to instead > provide an architecture-selectable Kconfig option, and select the > strength of smp_mb__after_unlock_lock() based on that option. Why is this better? Do we want to have more of this? I thought we still wanted to convince PPC to go RCsc and eradicate all this RCpc 'fun'. But instead now you're making it look like its OK to grow more of this pain.
On Thu, Apr 13, 2017 at 11:21:53AM +0200, Peter Zijlstra wrote: > On Wed, Apr 12, 2017 at 10:39:47AM -0700, Paul E. McKenney wrote: > > > CONFIG_ARCH_WEAK_RELACQ Kconfig option. > > Naming in the changelog and patch don't match. Good eyes! Fixed. Thanx, Paul > > +config ARCH_WEAK_RELEASE_ACQUIRE > > + bool > > + > >
On Thu, Apr 13, 2017 at 11:24:18AM +0200, Peter Zijlstra wrote: > On Wed, Apr 12, 2017 at 10:39:47AM -0700, Paul E. McKenney wrote: > > The definition of smp_mb__after_unlock_lock() is currently smp_mb() > > for CONFIG_PPC and a no-op otherwise. It would be better to instead > > provide an architecture-selectable Kconfig option, and select the > > strength of smp_mb__after_unlock_lock() based on that option. > > Why is this better? Do we want to have more of this? I thought we still > wanted to convince PPC to go RCsc and eradicate all this RCpc 'fun'. But > instead now you're making it look like its OK to grow more of this pain. ARCH_WEAK_RELEASE_ACQUIRE actually works both ways. To see this, imagine some strange alternate universe in which the Power hardware guys actually did decide to switch PPC to doing RCsc as you suggest. There would still be a lot of Power hardware out there that still does RCpc. Therefore, powerpc builds that needed to run on old Power hardware would select ARCH_WEAK_RELEASE_ACQUIRE, while kernels built to run only on the shiny new (but mythical) alternate-universe Power hardware would avoid selecting this Kconfig option. But the real reason I queued this patch is that Ingo asked me for it: https://lkml.org/lkml/2017/1/14/88 Thanx, Paul
On Thu, Apr 13, 2017 at 09:26:51AM -0700, Paul E. McKenney wrote: > ARCH_WEAK_RELEASE_ACQUIRE actually works both ways. > > To see this, imagine some strange alternate universe in which the Power > hardware guys actually did decide to switch PPC to doing RCsc as you > suggest. There would still be a lot of Power hardware out there that > still does RCpc. Therefore, powerpc builds that needed to run on old > Power hardware would select ARCH_WEAK_RELEASE_ACQUIRE, while kernels > built to run only on the shiny new (but mythical) alternate-universe > Power hardware would avoid selecting this Kconfig option. Ah, but Power software guys could do it today by replacing an LWSYNC with a SYNC in say arch_spin_unlock(). And yes, I know this isn't a popular suggestion, but it would do the trick. Its just that since there's one (PPC) we can sort of pressure them with the pain of being the only ones to hit all the bugs. But the moment more appear (and I'm afraid it'll be MIPS, with the excuse that PPC already does this) it will be ever so much harder to get rid of it. Then again, maybe I should just give up and accept the Linux kernel has RCpc locks..
On Thu, Apr 13, 2017 at 06:37:57PM +0200, Peter Zijlstra wrote: > On Thu, Apr 13, 2017 at 09:26:51AM -0700, Paul E. McKenney wrote: > > > ARCH_WEAK_RELEASE_ACQUIRE actually works both ways. > > > > To see this, imagine some strange alternate universe in which the Power > > hardware guys actually did decide to switch PPC to doing RCsc as you > > suggest. There would still be a lot of Power hardware out there that > > still does RCpc. Therefore, powerpc builds that needed to run on old > > Power hardware would select ARCH_WEAK_RELEASE_ACQUIRE, while kernels > > built to run only on the shiny new (but mythical) alternate-universe > > Power hardware would avoid selecting this Kconfig option. > > Ah, but Power software guys could do it today by replacing an LWSYNC > with a SYNC in say arch_spin_unlock(). > > And yes, I know this isn't a popular suggestion, but it would do the > trick. Indeed, there is a fine line between motivating people to move to new hardware on the one hand and terminally annoying existing users on the other. ;-) > Its just that since there's one (PPC) we can sort of pressure them with > the pain of being the only ones to hit all the bugs. But the moment more > appear (and I'm afraid it'll be MIPS, with the excuse that PPC already > does this) it will be ever so much harder to get rid of it. > > Then again, maybe I should just give up and accept the Linux kernel has > RCpc locks.. As usual, I must defer to the powerpc maintainers on this one. Thanx, Paul
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> writes: > On Thu, Apr 13, 2017 at 06:37:57PM +0200, Peter Zijlstra wrote: >> On Thu, Apr 13, 2017 at 09:26:51AM -0700, Paul E. McKenney wrote: >> >> > ARCH_WEAK_RELEASE_ACQUIRE actually works both ways. >> > >> > To see this, imagine some strange alternate universe in which the Power >> > hardware guys actually did decide to switch PPC to doing RCsc as you >> > suggest. There would still be a lot of Power hardware out there that >> > still does RCpc. Therefore, powerpc builds that needed to run on old >> > Power hardware would select ARCH_WEAK_RELEASE_ACQUIRE, while kernels >> > built to run only on the shiny new (but mythical) alternate-universe >> > Power hardware would avoid selecting this Kconfig option. >> >> Ah, but Power software guys could do it today by replacing an LWSYNC >> with a SYNC in say arch_spin_unlock(). >> >> And yes, I know this isn't a popular suggestion, but it would do the >> trick. > > Indeed, there is a fine line between motivating people to move to new > hardware on the one hand and terminally annoying existing users on > the other. ;-) > >> Its just that since there's one (PPC) we can sort of pressure them with >> the pain of being the only ones to hit all the bugs. But the moment more >> appear (and I'm afraid it'll be MIPS, with the excuse that PPC already >> does this) it will be ever so much harder to get rid of it. >> >> Then again, maybe I should just give up and accept the Linux kernel has >> RCpc locks.. > > As usual, I must defer to the powerpc maintainers on this one. I reworked my locking tests a bit, to run longer, disable ASLR and a few other things, and ran them again. They just bang repeatedly on an uncontended lock, so nothing fancy at all. Switching the release barrier to sync (from lwsync) slows it down by about 18%. So I think that pretty much rules it out, at least on current CPUs. I'll try and get some more time to make sure I didn't do something stupid in the test, and maybe do a version that includes some contention. cheers
On Wed, Apr 19, 2017 at 11:38:22PM +1000, Michael Ellerman wrote: > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> writes: > > > On Thu, Apr 13, 2017 at 06:37:57PM +0200, Peter Zijlstra wrote: > >> On Thu, Apr 13, 2017 at 09:26:51AM -0700, Paul E. McKenney wrote: > >> > >> > ARCH_WEAK_RELEASE_ACQUIRE actually works both ways. > >> > > >> > To see this, imagine some strange alternate universe in which the Power > >> > hardware guys actually did decide to switch PPC to doing RCsc as you > >> > suggest. There would still be a lot of Power hardware out there that > >> > still does RCpc. Therefore, powerpc builds that needed to run on old > >> > Power hardware would select ARCH_WEAK_RELEASE_ACQUIRE, while kernels > >> > built to run only on the shiny new (but mythical) alternate-universe > >> > Power hardware would avoid selecting this Kconfig option. > >> > >> Ah, but Power software guys could do it today by replacing an LWSYNC > >> with a SYNC in say arch_spin_unlock(). > >> > >> And yes, I know this isn't a popular suggestion, but it would do the > >> trick. > > > > Indeed, there is a fine line between motivating people to move to new > > hardware on the one hand and terminally annoying existing users on > > the other. ;-) > > > >> Its just that since there's one (PPC) we can sort of pressure them with > >> the pain of being the only ones to hit all the bugs. But the moment more > >> appear (and I'm afraid it'll be MIPS, with the excuse that PPC already > >> does this) it will be ever so much harder to get rid of it. > >> > >> Then again, maybe I should just give up and accept the Linux kernel has > >> RCpc locks.. > > > > As usual, I must defer to the powerpc maintainers on this one. > > I reworked my locking tests a bit, to run longer, disable ASLR and a few > other things, and ran them again. They just bang repeatedly on an > uncontended lock, so nothing fancy at all. > > Switching the release barrier to sync (from lwsync) slows it down by > about 18%. Ouch!!! > So I think that pretty much rules it out, at least on current CPUs. > > I'll try and get some more time to make sure I didn't do something > stupid in the test, and maybe do a version that includes some > contention. Looking forward to seeing what you come up with... Thanx, Paul
diff --git a/arch/Kconfig b/arch/Kconfig index cd211a14a88f..adefaf344239 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -320,6 +320,9 @@ config HAVE_CMPXCHG_LOCAL config HAVE_CMPXCHG_DOUBLE bool +config ARCH_WEAK_RELEASE_ACQUIRE + bool + config ARCH_WANT_IPC_PARSE_VERSION bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 97a8bc8a095c..7a5c9b764cd2 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -99,6 +99,7 @@ config PPC select ARCH_USE_BUILTIN_BSWAP select ARCH_USE_CMPXCHG_LOCKREF if PPC64 select ARCH_WANT_IPC_PARSE_VERSION + select ARCH_WEAK_RELEASE_ACQUIRE select BINFMT_ELF select BUILDTIME_EXTABLE_SORT select CLONE_BACKWARDS diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index de88b33c0974..e6146d0074f8 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -1127,11 +1127,11 @@ do { \ * if the UNLOCK and LOCK are executed by the same CPU or if the * UNLOCK and LOCK operate on the same lock variable. */ -#ifdef CONFIG_PPC +#ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE #define smp_mb__after_unlock_lock() smp_mb() /* Full ordering for lock. */ -#else /* #ifdef CONFIG_PPC */ +#else /* #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */ #define smp_mb__after_unlock_lock() do { } while (0) -#endif /* #else #ifdef CONFIG_PPC */ +#endif /* #else #ifdef CONFIG_ARCH_WEAK_RELEASE_ACQUIRE */ #endif /* __LINUX_RCUPDATE_H */