Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964930AbbGCPiy (ORCPT ); Fri, 3 Jul 2015 11:38:54 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:35965 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755283AbbGCPiq (ORCPT ); Fri, 3 Jul 2015 11:38:46 -0400 Subject: Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage To: Peter Zijlstra References: <558BED42.1030000@gmail.com> <20150625120949.GZ3644@twins.programming.kicks-ass.net> <558BF0E4.50602@gmail.com> <20150625125514.GA3644@twins.programming.kicks-ass.net> <55966E0B.7060100@redhat.com> <20150703121907.GH19282@twins.programming.kicks-ass.net> <5596809D.4000905@redhat.com> <20150703131712.GJ19282@twins.programming.kicks-ass.net> <20150703151642.GQ18673@twins.programming.kicks-ass.net> <5596A997.5060705@redhat.com> Cc: Pontus Fuchs , Linus Torvalds , mingo@redhat.com, "linux-kernel@vger.kernel.org" , gleb@kernel.org From: Paolo Bonzini X-Enigmail-Draft-Status: N1110 Message-ID: <5596AC82.6080706@redhat.com> Date: Fri, 3 Jul 2015 17:38:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <5596A997.5060705@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4607 Lines: 129 On 03/07/2015 17:26, Paolo Bonzini wrote: > > > On 03/07/2015 17:16, Peter Zijlstra wrote: >> On Fri, Jul 03, 2015 at 03:17:13PM +0200, Peter Zijlstra wrote: >>> But, could we rework the code so that you register the preempt notifier >>> when creating the vcpu thread and leave it installed forevermore? >> >> OK, it looks like there is no fixed relation between a thread and a vcpu >> :/ >> >> Would something like the below (on top of all the others) work for you? > > This looks fine, but one would do the same thing without the previous > patch, i.e. directly on top of Linus's master---right? The patch in tip > is a red herring, and the hunks below are reverting large parts of it. So basically this. Can you reply with SoB and maybe Acked-by? ------------- 8< --------------- From: Peter Zijlstra Subject: [PATCH] sched, preempt_notifier: separate notifier registration from static_key inc/dec Commit 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers") had two problems. First, the preempt-notifier API needs to sleep with the addition of the static_key, we do however need to hold off preemption while modifying the preempt notifier list, otherwise a preemption could observe an inconsistent list state. KVM correctly registers and unregisters preempt notifiers with preemption disabled, so the sleep caused dmesg splats. Second, KVM registers and unregisters preemption notifiers very often (in vcpu_load/vcpu_put). With a single uniprocessor guest the static key would move between 0 and 1 continuously, hitting the slow path on every userspace exit. To fix this, wrap the static_key inc/dec in a new API, and call it from KVM. Fixes: 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers") Reported-by: Pontus Fuchs Reported-by: Takashi Iwai Not-signed-off-by: Peter Zijlstra Not-acked-by: Peter Zijlstra Not-signed-off-by: Paolo Bonzini link, ¤t->preempt_notifiers); } EXPORT_SYMBOL_GPL(preempt_notifier_register); @@ -2340,7 +2354,6 @@ EXPORT_SYMBOL_GPL(preempt_notifier_register); void preempt_notifier_unregister(struct preempt_notifier *notifier) { hlist_del(¬ifier->link); - static_key_slow_dec(&preempt_notifier_key); } EXPORT_SYMBOL_GPL(preempt_notifier_unregister); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 848af90b8091..8b8a44453670 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -553,6 +553,8 @@ static struct kvm *kvm_create_vm(unsigned long type) list_add(&kvm->vm_list, &vm_list); spin_unlock(&kvm_lock); + preempt_notifier_inc(); + return kvm; out_err: @@ -620,6 +622,7 @@ static void kvm_destroy_vm(struct kvm *kvm) cleanup_srcu_struct(&kvm->irq_srcu); cleanup_srcu_struct(&kvm->srcu); kvm_arch_free_vm(kvm); + preempt_notifier_dec(); hardware_disable_all(); mmdrop(mm); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/