From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH RFC v5 8/8] pvqspinlock, x86: Enable KVM to use qspinlock's PV support Date: Thu, 27 Feb 2014 10:31:51 +0100 Message-ID: <530F0607.1020705@redhat.com> References: <1393427668-60228-1-git-send-email-Waiman.Long@hp.com> <1393427668-60228-9-git-send-email-Waiman.Long@hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1393427668-60228-9-git-send-email-Waiman.Long@hp.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Waiman Long , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Arnd Bergmann , Peter Zijlstra Cc: Jeremy Fitzhardinge , Raghavendra K T , virtualization@lists.linux-foundation.org, Andi Kleen , Michel Lespinasse , Boris Ostrovsky , linux-arch@vger.kernel.org, x86@kernel.org, Scott J Norton , xen-devel@lists.xenproject.org, "Paul E. McKenney" , Alexander Fyodorov , Rik van Riel , Konrad Rzeszutek Wilk , Daniel J Blueman , Oleg Nesterov , Steven Rostedt , Chris Wright , George Spelvin , Alok Kataria , Aswin Chandramouleeswaran , Chegu Vinod , Linus Torvalds li List-Id: linux-arch.vger.kernel.org Il 26/02/2014 16:14, Waiman Long ha scritto: > This patch enables KVM to use the queue spinlock's PV support code > when the PARAVIRT_SPINLOCKS kernel config option is set. However, > PV support for Xen is not ready yet and so the queue spinlock will > still have to be disabled when PARAVIRT_SPINLOCKS config option is > on with Xen. > > Signed-off-by: Waiman Long > --- > arch/x86/kernel/kvm.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ > kernel/Kconfig.locks | 2 +- > 2 files changed, 55 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > index f318e78..3ddc436 100644 > --- a/arch/x86/kernel/kvm.c > +++ b/arch/x86/kernel/kvm.c > @@ -568,6 +568,7 @@ static void kvm_kick_cpu(int cpu) > kvm_hypercall2(KVM_HC_KICK_CPU, flags, apicid); > } > > +#ifndef CONFIG_QUEUE_SPINLOCK > enum kvm_contention_stat { > TAKEN_SLOW, > TAKEN_SLOW_PICKUP, > @@ -795,6 +796,55 @@ static void kvm_unlock_kick(struct arch_spinlock *lock, __ticket_t ticket) > } > } > } > +#else /* !CONFIG_QUEUE_SPINLOCK */ > + > +#ifdef CONFIG_KVM_DEBUG_FS > +static struct dentry *d_spin_debug; > +static struct dentry *d_kvm_debug; > +static u32 lh_kick_stats; /* Lock holder kick count */ > +static u32 qh_kick_stats; /* Queue head kick count */ > +static u32 nn_kick_stats; /* Next node kick count */ > + > +static int __init kvm_spinlock_debugfs(void) > +{ > + d_kvm_debug = debugfs_create_dir("kvm-guest", NULL); > + if (!d_kvm_debug) { > + printk(KERN_WARNING > + "Could not create 'kvm' debugfs directory\n"); > + return -ENOMEM; > + } > + d_spin_debug = debugfs_create_dir("spinlocks", d_kvm_debug); > + > + debugfs_create_u32("lh_kick_stats", 0644, d_spin_debug, &lh_kick_stats); > + debugfs_create_u32("qh_kick_stats", 0644, d_spin_debug, &qh_kick_stats); > + debugfs_create_u32("nn_kick_stats", 0644, d_spin_debug, &nn_kick_stats); > + > + return 0; > +} > + > +static inline void inc_kick_stats(enum pv_kick_type type) > +{ > + if (type == PV_KICK_LOCK_HOLDER) > + add_smp(&lh_kick_stats, 1); > + else if (type == PV_KICK_QUEUE_HEAD) > + add_smp(&qh_kick_stats, 1); > + else > + add_smp(&nn_kick_stats, 1); > +} > +fs_initcall(kvm_spinlock_debugfs); > + > +#else /* CONFIG_KVM_DEBUG_FS */ > +static inline void inc_kick_stats(enum pv_kick_type type) > +{ > +} > +#endif /* CONFIG_KVM_DEBUG_FS */ > + > +static void kvm_kick_cpu_type(int cpu, enum pv_kick_type type) > +{ > + kvm_kick_cpu(cpu); > + inc_kick_stats(type); > +} > +#endif /* !CONFIG_QUEUE_SPINLOCK */ > > /* > * Setup pv_lock_ops to exploit KVM_FEATURE_PV_UNHALT if present. > @@ -807,8 +857,12 @@ void __init kvm_spinlock_init(void) > if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)) > return; > > +#ifdef CONFIG_QUEUE_SPINLOCK > + pv_lock_ops.kick_cpu = kvm_kick_cpu_type; > +#else > pv_lock_ops.lock_spinning = PV_CALLEE_SAVE(kvm_lock_spinning); > pv_lock_ops.unlock_kick = kvm_unlock_kick; > +#endif > } > > static __init int kvm_spinlock_init_jump(void) > diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks > index f185584..a70fdeb 100644 > --- a/kernel/Kconfig.locks > +++ b/kernel/Kconfig.locks > @@ -229,4 +229,4 @@ config ARCH_USE_QUEUE_SPINLOCK > > config QUEUE_SPINLOCK > def_bool y if ARCH_USE_QUEUE_SPINLOCK > - depends on SMP && !PARAVIRT_SPINLOCKS > + depends on SMP && (!PARAVIRT_SPINLOCKS || !XEN) > Should this rather be def_bool y if ARCH_USE_QUEUE_SPINLOCK && (!PARAVIRT_SPINLOCKS || !XEN) ? PARAVIRT_SPINLOCKS + XEN + QUEUE_SPINLOCK + PARAVIRT_UNFAIR_LOCKS is a valid combination, but it's impossible to choose PARAVIRT_UNFAIR_LOCKS if QUEUE_SPINLOCK is unavailable. Paolo