Messages in this thread | | | Subject | Re: [PATCH] paravirt/locks: avoid modifying static key before jump_label_init() | From | Juergen Gross <> | Date | Wed, 25 Oct 2017 09:51:41 +0200 |
| |
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 >> >> >> > > >
| |