From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM breaks CPU hotplug Date: Mon, 26 Mar 2007 11:11:08 +0200 Message-ID: <46078E2C.9010100@qumranet.com> References: <1174898893.14063.7.camel@sli10-conroe.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel , Andrew Morton To: Shaohua Li Return-path: In-reply-to: <1174898893.14063.7.camel-yAZKuqJtXNMXR+D7ky4Foa2pdiUAq4bhAL8bYrjMMd8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Shaohua Li wrote: > When testing CPU hotplug, I found cpu can't be onlined with kvm enabled > sometimes. The reason is smp_call_function_single is a nop if the thread > is running on the target cpu. I think CPU_ONLINE case doesn't require > the fix as the online CPU isn't plugged into sheduler yet. > > I think this is not enough, because: - this path is preemptible code, so the test (this_cpu == cpu) can run one on cpu and execute later code on another - a virtual machine might be scheduled later on this cpu and oops when executing a vmx/svm instruction I'm not sure how to fix. Perhaps have _cpu_down() remove the cpu it is downing from its affinity mask earlier? the code is already there, a few lines below. > Signed-off-by: Shaohua Li > > diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c > index dc7a8c7..806a931 100644 > --- a/drivers/kvm/kvm_main.c > +++ b/drivers/kvm/kvm_main.c > @@ -2360,6 +2360,7 @@ static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned long val, > void *v) > { > int cpu = (long)v; > + int this_cpu; > > switch (val) { > case CPU_DOWN_PREPARE: > @@ -2367,8 +2368,13 @@ static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned long val, > printk(KERN_INFO "kvm: disabling virtualization on CPU%d\n", > cpu); > decache_vcpus_on_cpu(cpu); > - smp_call_function_single(cpu, kvm_arch_ops->hardware_disable, > - NULL, 0, 1); > + this_cpu = get_cpu(); > + if (this_cpu == cpu) > + kvm_arch_ops->hardware_disable(NULL); > + else > + smp_call_function_single(cpu, > + kvm_arch_ops->hardware_disable, NULL, 0, 1); > + put_cpu(); > break; > case CPU_ONLINE: > printk(KERN_INFO "kvm: enabling virtualization on CPU%d\n", > -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV