From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932258AbdJYHvs (ORCPT ); Wed, 25 Oct 2017 03:51:48 -0400 Received: from mx2.suse.de ([195.135.220.15]:40382 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932109AbdJYHvo (ORCPT ); Wed, 25 Oct 2017 03:51:44 -0400 Subject: Re: [PATCH] paravirt/locks: avoid modifying static key before jump_label_init() To: Dou Liyang , linux-kernel@vger.kernel.org, x86@kernel.org Cc: hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com, arnd@arndb.de, peterz@infradead.org References: <20171023134948.24886-1-jgross@suse.com> <7223e00c-0fe6-8844-d0f2-a7cabfba9c03@suse.com> <1c852010-e9df-fac8-8970-6c60f769f40e@cn.fujitsu.com> From: Juergen Gross Message-ID: <6890aed1-c811-b803-00cb-3e87119c8b2b@suse.com> Date: Wed, 25 Oct 2017 09:51:41 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1c852010-e9df-fac8-8970-6c60f769f40e@cn.fujitsu.com> Content-Type: text/plain; charset=utf-8 Content-Language: de-DE Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 25/10/17 09:35, Dou Liyang wrote: > Hi Juergen, > > [...] >>> I like your original method. >>> So, I try to fix it by moving the native_pv_lock_init() from >>>  native_smp_prepare_boot_cpu() to native_smp_prepare_cpus(). >> >> Hmm, this might work, but the Xen case has to be modified (same for >> my more general solution), as xen_init_spinlocks() is still modifying >> the static key too early. And we can't move xen_init_spinlocks() to >> smp_prepare_cpus() as this would be too late for the alternatives >> patching. >> > > Yes, Right. > >> So let me extend your patch a little bit to cover Xen, too. >> > > Yes! > > How about moving the check of xen_pvspin into native_pv_lock_init() > like below? This would leak xen_pvspin to non-Xen code. I'd rather do the static_branch_disable() in xen_init_lock_cpu() if cpu == 0. Juergen > > Thanks, >     dou. > > ------------------------->8 > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c > index 041096b..b5f3ecb 100644 > --- a/arch/x86/kernel/paravirt.c > +++ b/arch/x86/kernel/paravirt.c > @@ -119,7 +119,7 @@ DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key); > >  void __init native_pv_lock_init(void) >  { > -       if (!static_cpu_has(X86_FEATURE_HYPERVISOR)) > +       if (!static_cpu_has(X86_FEATURE_HYPERVISOR) || !xen_pvspin) >                 static_branch_disable(&virt_spin_lock_key); >  } > > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c > index aed1460..6b1335a 100644 > --- a/arch/x86/kernel/smpboot.c > +++ b/arch/x86/kernel/smpboot.c > @@ -1323,6 +1323,8 @@ void __init native_smp_prepare_cpus(unsigned int > max_cpus) >         pr_info("CPU0: "); >         print_cpu_info(&cpu_data(0)); > > +       native_pv_lock_init(); > + >         uv_system_init(); > >         set_mtrr_aps_delayed_init(); > @@ -1350,7 +1352,6 @@ void __init native_smp_prepare_boot_cpu(void) >         /* already set me in cpu_online_mask in boot_cpu_init() */ >         cpumask_set_cpu(me, cpu_callout_mask); >         cpu_set_state_online(me); > -       native_pv_lock_init(); >  } > >  void __init native_smp_cpus_done(unsigned int max_cpus) > diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c > index 5147140..570b2bc 100644 > --- a/arch/x86/xen/smp_pv.c > +++ b/arch/x86/xen/smp_pv.c > @@ -236,6 +236,8 @@ static void __init xen_pv_smp_prepare_cpus(unsigned > int max_cpus) >                 xen_raw_printk(m); >                 panic(m); >         } > +       native_pv_lock_init(); > + >         xen_init_lock_cpu(0); > >         smp_store_boot_cpu_info(); > diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c > index e8ab80a..8e0ec79 100644 > --- a/arch/x86/xen/spinlock.c > +++ b/arch/x86/xen/spinlock.c > @@ -130,7 +130,6 @@ void __init xen_init_spinlocks(void) > >         if (!xen_pvspin) { >                 printk(KERN_DEBUG "xen: PV spinlocks disabled\n"); > -               static_branch_disable(&virt_spin_lock_key); >                 return; >         } >         printk(KERN_DEBUG "xen: PV spinlocks enabled\n"); > > >>> I hope it's useful to you. >> >> It really is, thanks. >> >> >> Juergen >> >> >> > > >