From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicholas Piggin Subject: Re: [PATCH v2 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR Date: Mon, 06 Jul 2020 10:30:05 +1000 Message-ID: <1593994632.syt8hwimv9.astroid@bobo.none> References: <20200703073516.1354108-1-npiggin@gmail.com> <20200703073516.1354108-6-npiggin@gmail.com> <81d9981b-8a20-729c-b861-c7229e40bb65@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <81d9981b-8a20-729c-b861-c7229e40bb65@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: virtualization-bounces@lists.linux-foundation.org Sender: "Virtualization" To: Waiman Long Cc: linux-arch@vger.kernel.org, Peter Zijlstra , Will Deacon , Boqun Feng , linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, virtualization@lists.linux-foundation.org, Ingo Molnar , Anton Blanchard , linuxppc-dev@lists.ozlabs.org List-Id: linux-arch.vger.kernel.org RXhjZXJwdHMgZnJvbSBXYWltYW4gTG9uZydzIG1lc3NhZ2Ugb2YgSnVseSA2LCAyMDIwIDU6MDAg YW06Cj4gT24gNy8zLzIwIDM6MzUgQU0sIE5pY2hvbGFzIFBpZ2dpbiB3cm90ZToKPj4gU2lnbmVk LW9mZi1ieTogTmljaG9sYXMgUGlnZ2luIDxucGlnZ2luQGdtYWlsLmNvbT4KPj4gLS0tCj4+ICAg YXJjaC9wb3dlcnBjL2luY2x1ZGUvYXNtL3BhcmF2aXJ0LmggICAgICAgICAgIHwgMjggKysrKysr KysrKwo+PiAgIGFyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9xc3BpbmxvY2suaCAgICAgICAgICB8 IDU1ICsrKysrKysrKysrKysrKysrKysKPj4gICBhcmNoL3Bvd2VycGMvaW5jbHVkZS9hc20vcXNw aW5sb2NrX3BhcmF2aXJ0LmggfCAgNSArKwo+PiAgIGFyY2gvcG93ZXJwYy9wbGF0Zm9ybXMvcHNl cmllcy9LY29uZmlnICAgICAgICB8ICA1ICsrCj4+ICAgYXJjaC9wb3dlcnBjL3BsYXRmb3Jtcy9w c2VyaWVzL3NldHVwLmMgICAgICAgIHwgIDYgKy0KPj4gICBpbmNsdWRlL2FzbS1nZW5lcmljL3Fz cGlubG9jay5oICAgICAgICAgICAgICAgfCAgMiArCj4+ICAgNiBmaWxlcyBjaGFuZ2VkLCAxMDAg aW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQo+PiAgIGNyZWF0ZSBtb2RlIDEwMDY0NCBhcmNo L3Bvd2VycGMvaW5jbHVkZS9hc20vcXNwaW5sb2NrX3BhcmF2aXJ0LmgKPj4KPj4gZGlmZiAtLWdp dCBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wYXJhdmlydC5oIGIvYXJjaC9wb3dlcnBjL2lu Y2x1ZGUvYXNtL3BhcmF2aXJ0LmgKPj4gaW5kZXggN2E4NTQ2NjYwYTYzLi5mMmQ1MWY5MjljZjUg MTAwNjQ0Cj4+IC0tLSBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wYXJhdmlydC5oCj4+ICsr KyBiL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9wYXJhdmlydC5oCj4+IEBAIC0yOSw2ICsyOSwx NiBAQCBzdGF0aWMgaW5saW5lIHZvaWQgeWllbGRfdG9fcHJlZW1wdGVkKGludCBjcHUsIHUzMiB5 aWVsZF9jb3VudCkKPj4gICB7Cj4+ICAgCXBscGFyX2hjYWxsX25vcmV0cyhIX0NPTkZFUiwgZ2V0 X2hhcmRfc21wX3Byb2Nlc3Nvcl9pZChjcHUpLCB5aWVsZF9jb3VudCk7Cj4+ICAgfQo+PiArCj4+ ICtzdGF0aWMgaW5saW5lIHZvaWQgcHJvZF9jcHUoaW50IGNwdSkKPj4gK3sKPj4gKwlwbHBhcl9o Y2FsbF9ub3JldHMoSF9QUk9ELCBnZXRfaGFyZF9zbXBfcHJvY2Vzc29yX2lkKGNwdSkpOwo+PiAr fQo+PiArCj4+ICtzdGF0aWMgaW5saW5lIHZvaWQgeWllbGRfdG9fYW55KHZvaWQpCj4+ICt7Cj4+ ICsJcGxwYXJfaGNhbGxfbm9yZXRzKEhfQ09ORkVSLCAtMSwgMCk7Cj4+ICt9Cj4+ICAgI2Vsc2UK Pj4gICBzdGF0aWMgaW5saW5lIGJvb2wgaXNfc2hhcmVkX3Byb2Nlc3Nvcih2b2lkKQo+PiAgIHsK Pj4gQEAgLTQ1LDYgKzU1LDE5IEBAIHN0YXRpYyBpbmxpbmUgdm9pZCB5aWVsZF90b19wcmVlbXB0 ZWQoaW50IGNwdSwgdTMyIHlpZWxkX2NvdW50KQo+PiAgIHsKPj4gICAJX19fYmFkX3lpZWxkX3Rv X3ByZWVtcHRlZCgpOyAvKiBUaGlzIHdvdWxkIGJlIGEgYnVnICovCj4+ICAgfQo+PiArCj4+ICtl eHRlcm4gdm9pZCBfX19iYWRfeWllbGRfdG9fYW55KHZvaWQpOwo+PiArc3RhdGljIGlubGluZSB2 b2lkIHlpZWxkX3RvX2FueSh2b2lkKQo+PiArewo+PiArCV9fX2JhZF95aWVsZF90b19hbnkoKTsg LyogVGhpcyB3b3VsZCBiZSBhIGJ1ZyAqLwo+PiArfQo+PiArCj4+ICtleHRlcm4gdm9pZCBfX19i YWRfcHJvZF9jcHUodm9pZCk7Cj4+ICtzdGF0aWMgaW5saW5lIHZvaWQgcHJvZF9jcHUoaW50IGNw dSkKPj4gK3sKPj4gKwlfX19iYWRfcHJvZF9jcHUoKTsgLyogVGhpcyB3b3VsZCBiZSBhIGJ1ZyAq Lwo+PiArfQo+PiArCj4+ICAgI2VuZGlmCj4+ICAgCj4+ICAgI2RlZmluZSB2Y3B1X2lzX3ByZWVt cHRlZCB2Y3B1X2lzX3ByZWVtcHRlZAo+PiBAQCAtNTcsNSArODAsMTAgQEAgc3RhdGljIGlubGlu ZSBib29sIHZjcHVfaXNfcHJlZW1wdGVkKGludCBjcHUpCj4+ICAgCXJldHVybiBmYWxzZTsKPj4g ICB9Cj4+ICAgCj4+ICtzdGF0aWMgaW5saW5lIGJvb2wgcHZfaXNfbmF0aXZlX3NwaW5fdW5sb2Nr KHZvaWQpCj4+ICt7Cj4+ICsgICAgIHJldHVybiAhaXNfc2hhcmVkX3Byb2Nlc3NvcigpOwo+PiAr fQo+PiArCj4+ICAgI2VuZGlmIC8qIF9fS0VSTkVMX18gKi8KPj4gICAjZW5kaWYgLyogX19BU01f UEFSQVZJUlRfSCAqLwo+PiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2luY2x1ZGUvYXNtL3Fz cGlubG9jay5oIGIvYXJjaC9wb3dlcnBjL2luY2x1ZGUvYXNtL3FzcGlubG9jay5oCj4+IGluZGV4 IGM0OWUzM2UyNGVkZC4uMDk2MGEwZGUyNDY3IDEwMDY0NAo+PiAtLS0gYS9hcmNoL3Bvd2VycGMv aW5jbHVkZS9hc20vcXNwaW5sb2NrLmgKPj4gKysrIGIvYXJjaC9wb3dlcnBjL2luY2x1ZGUvYXNt L3FzcGlubG9jay5oCj4+IEBAIC0zLDkgKzMsMzYgQEAKPj4gICAjZGVmaW5lIF9BU01fUE9XRVJQ Q19RU1BJTkxPQ0tfSAo+PiAgIAo+PiAgICNpbmNsdWRlIDxhc20tZ2VuZXJpYy9xc3BpbmxvY2tf dHlwZXMuaD4KPj4gKyNpbmNsdWRlIDxhc20vcGFyYXZpcnQuaD4KPj4gICAKPj4gICAjZGVmaW5l IF9RX1BFTkRJTkdfTE9PUFMJKDEgPDwgOSkgLyogbm90IHR1bmVkICovCj4+ICAgCj4+ICsjaWZk ZWYgQ09ORklHX1BBUkFWSVJUX1NQSU5MT0NLUwo+PiArZXh0ZXJuIHZvaWQgbmF0aXZlX3F1ZXVl ZF9zcGluX2xvY2tfc2xvd3BhdGgoc3RydWN0IHFzcGlubG9jayAqbG9jaywgdTMyIHZhbCk7Cj4+ ICtleHRlcm4gdm9pZCBfX3B2X3F1ZXVlZF9zcGluX2xvY2tfc2xvd3BhdGgoc3RydWN0IHFzcGlu bG9jayAqbG9jaywgdTMyIHZhbCk7Cj4+ICsKPj4gK3N0YXRpYyBfX2Fsd2F5c19pbmxpbmUgdm9p ZCBxdWV1ZWRfc3Bpbl9sb2NrX3Nsb3dwYXRoKHN0cnVjdCBxc3BpbmxvY2sgKmxvY2ssIHUzMiB2 YWwpCj4+ICt7Cj4+ICsJaWYgKCFpc19zaGFyZWRfcHJvY2Vzc29yKCkpCj4+ICsJCW5hdGl2ZV9x dWV1ZWRfc3Bpbl9sb2NrX3Nsb3dwYXRoKGxvY2ssIHZhbCk7Cj4+ICsJZWxzZQo+PiArCQlfX3B2 X3F1ZXVlZF9zcGluX2xvY2tfc2xvd3BhdGgobG9jaywgdmFsKTsKPj4gK30KPiAKPiBJbiBhIHBy ZXZpb3VzIG1haWwsIEkgc2FpZCB0aGF0OgoKSGV5LCB5ZWFoIEkgcmVhZCB0aGF0IHJpZ2h0IGFm dGVyIHNlbmRpbmcgdGhlIHNlcmllcyBvdXQuIFRoYW5rcyBmb3IgdGhlIAp0aG9yb3VnaCByZXZp ZXcuCgo+IFlvdSBtYXkgbmVlZCB0byBtYXRjaCB0aGUgdXNlIG9mIF9fcHZfcXVldWVkX3NwaW5f bG9ja19zbG93cGF0aCgpIHdpdGggCj4gdGhlIGNvcnJlc3BvbmRpbmcgX19wdl9xdWV1ZWRfc3Bp bl91bmxvY2soKSwgZS5nLgo+IAo+ICNkZWZpbmUgcXVldWVkX3NwaW5fdW5sb2NrIHF1ZXVlZF9z cGluX3VubG9jawo+IHN0YXRpYyBpbmxpbmUgcXVldWVkX3NwaW5fdW5sb2NrKHN0cnVjdCBxc3Bp bmxvY2sgKmxvY2spCj4gewo+ICDCoMKgwqDCoMKgwqDCoCBpZiAoIWlzX3NoYXJlZF9wcm9jZXNz b3IoKSkKPiAgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgIHNtcF9zdG9yZV9yZWxlYXNl KCZsb2NrLT5sb2NrZWQsIDApOwo+ICDCoMKgwqDCoMKgwqDCoCBlbHNlCj4gIMKgwqDCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoCBfX3B2X3F1ZXVlZF9zcGluX3VubG9jayhsb2NrKTsKPiB9Cj4g Cj4gT3RoZXJ3aXNlLCBwdl9raWNrKCkgd2lsbCBuZXZlciBiZSBjYWxsZWQuCj4gCj4gTWF5YmUg UG93ZXJQQyBITVQgaXMgZGlmZmVyZW50IHRoYXQgdGhlIHNoYXJlZCBjcHVzIGNhbiBzdGlsbCBw cm9jZXNzIAo+IGluc3RydWN0aW9uLCB0aG91Z2ggc2xvd2VyLCB0aGF0IGNwdSBraWNraW5nIGxp a2Ugd2hhdCB3YXMgZG9uZSBpbiBrdm0gCj4gaXMgbm90IHJlYWxseSBuZWNlc3NhcnkuIElmIHRo YXQgaXMgdGhlIGNhc2UsIEkgdGhpbmsgd2Ugc2hvdWxkIGRvY3VtZW50IAo+IHRoYXQuCgpJdCBk b2VzIHN0b3AgZGlzcGF0Y2gsIGJ1dCBpdCB3aWxsIHdha2UgdXAgYnkgaXRzZWxmIGFmdGVyIGFs bCBvdGhlciAKdkNQVXMgaGF2ZSBoYWQgYSBjaGFuY2UgdG8gZGlzcGF0Y2guIEkgd2lsbCByZS10 ZXN0IHdpdGggdGhlIGZpeCBpbgpwbGFjZSBhbmQgc2VlIGlmIHRoZXJlJ3MgYW55IHNpZ25pZmlj YW50IHBlcmZvcm1hbmNlIGRpZmZlcmVuY2VzLgoKVGhhbmtzLApOaWNrCgpfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpWaXJ0dWFsaXphdGlvbiBtYWlsaW5n IGxpc3QKVmlydHVhbGl6YXRpb25AbGlzdHMubGludXgtZm91bmRhdGlvbi5vcmcKaHR0cHM6Ly9s aXN0cy5saW51eGZvdW5kYXRpb24ub3JnL21haWxtYW4vbGlzdGluZm8vdmlydHVhbGl6YXRpb24= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728214AbgGFAaM (ORCPT ); Sun, 5 Jul 2020 20:30:12 -0400 Date: Mon, 06 Jul 2020 10:30:05 +1000 From: Nicholas Piggin Subject: Re: [PATCH v2 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR References: <20200703073516.1354108-1-npiggin@gmail.com> <20200703073516.1354108-6-npiggin@gmail.com> <81d9981b-8a20-729c-b861-c7229e40bb65@redhat.com> In-Reply-To: <81d9981b-8a20-729c-b861-c7229e40bb65@redhat.com> MIME-Version: 1.0 Message-ID: <1593994632.syt8hwimv9.astroid@bobo.none> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Sender: linux-arch-owner@vger.kernel.org List-ID: To: Waiman Long Cc: Anton Blanchard , Boqun Feng , kvm-ppc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Ingo Molnar , Peter Zijlstra , virtualization@lists.linux-foundation.org, Will Deacon Message-ID: <20200706003005.1DhwBV3uxgoMdTqeYP11rcZm2Y6N8iWEXeVIoN72uw8@z> Excerpts from Waiman Long's message of July 6, 2020 5:00 am: > On 7/3/20 3:35 AM, Nicholas Piggin wrote: >> Signed-off-by: Nicholas Piggin >> --- >> arch/powerpc/include/asm/paravirt.h | 28 ++++++++++ >> arch/powerpc/include/asm/qspinlock.h | 55 +++++++++++++++++++ >> arch/powerpc/include/asm/qspinlock_paravirt.h | 5 ++ >> arch/powerpc/platforms/pseries/Kconfig | 5 ++ >> arch/powerpc/platforms/pseries/setup.c | 6 +- >> include/asm-generic/qspinlock.h | 2 + >> 6 files changed, 100 insertions(+), 1 deletion(-) >> create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h >> >> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/= asm/paravirt.h >> index 7a8546660a63..f2d51f929cf5 100644 >> --- a/arch/powerpc/include/asm/paravirt.h >> +++ b/arch/powerpc/include/asm/paravirt.h >> @@ -29,6 +29,16 @@ static inline void yield_to_preempted(int cpu, u32 yi= eld_count) >> { >> plpar_hcall_norets(H_CONFER, get_hard_smp_processor_id(cpu), yield_co= unt); >> } >> + >> +static inline void prod_cpu(int cpu) >> +{ >> + plpar_hcall_norets(H_PROD, get_hard_smp_processor_id(cpu)); >> +} >> + >> +static inline void yield_to_any(void) >> +{ >> + plpar_hcall_norets(H_CONFER, -1, 0); >> +} >> #else >> static inline bool is_shared_processor(void) >> { >> @@ -45,6 +55,19 @@ static inline void yield_to_preempted(int cpu, u32 yi= eld_count) >> { >> ___bad_yield_to_preempted(); /* This would be a bug */ >> } >> + >> +extern void ___bad_yield_to_any(void); >> +static inline void yield_to_any(void) >> +{ >> + ___bad_yield_to_any(); /* This would be a bug */ >> +} >> + >> +extern void ___bad_prod_cpu(void); >> +static inline void prod_cpu(int cpu) >> +{ >> + ___bad_prod_cpu(); /* This would be a bug */ >> +} >> + >> #endif >> =20 >> #define vcpu_is_preempted vcpu_is_preempted >> @@ -57,5 +80,10 @@ static inline bool vcpu_is_preempted(int cpu) >> return false; >> } >> =20 >> +static inline bool pv_is_native_spin_unlock(void) >> +{ >> + return !is_shared_processor(); >> +} >> + >> #endif /* __KERNEL__ */ >> #endif /* __ASM_PARAVIRT_H */ >> diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include= /asm/qspinlock.h >> index c49e33e24edd..0960a0de2467 100644 >> --- a/arch/powerpc/include/asm/qspinlock.h >> +++ b/arch/powerpc/include/asm/qspinlock.h >> @@ -3,9 +3,36 @@ >> #define _ASM_POWERPC_QSPINLOCK_H >> =20 >> #include >> +#include >> =20 >> #define _Q_PENDING_LOOPS (1 << 9) /* not tuned */ >> =20 >> +#ifdef CONFIG_PARAVIRT_SPINLOCKS >> +extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u3= 2 val); >> +extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 = val); >> + >> +static __always_inline void queued_spin_lock_slowpath(struct qspinlock = *lock, u32 val) >> +{ >> + if (!is_shared_processor()) >> + native_queued_spin_lock_slowpath(lock, val); >> + else >> + __pv_queued_spin_lock_slowpath(lock, val); >> +} >=20 > In a previous mail, I said that: Hey, yeah I read that right after sending the series out. Thanks for the=20 thorough review. > You may need to match the use of __pv_queued_spin_lock_slowpath() with=20 > the corresponding __pv_queued_spin_unlock(), e.g. >=20 > #define queued_spin_unlock queued_spin_unlock > static inline queued_spin_unlock(struct qspinlock *lock) > { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!is_shared_processor()) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 smp_store_release(&lock->locked, 0); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 __pv_queued_spin_unlock(lock); > } >=20 > Otherwise, pv_kick() will never be called. >=20 > Maybe PowerPC HMT is different that the shared cpus can still process=20 > instruction, though slower, that cpu kicking like what was done in kvm=20 > is not really necessary. If that is the case, I think we should document=20 > that. It does stop dispatch, but it will wake up by itself after all other=20 vCPUs have had a chance to dispatch. I will re-test with the fix in place and see if there's any significant performance differences. Thanks, Nick