From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH] Activate Virtualization On Demand v2 Date: Wed, 05 Nov 2008 12:23:12 +0100 Message-ID: <49118220.4070403@suse.de> References: <1225874896-13186-1-git-send-email-agraf@suse.de> <49117015.7040902@redhat.com> <49117548.8030601@suse.de> <4911795A.6020807@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, kraxel@redhat.com, anthony@codemonkey.ws, Sander.Vanleeuwen@sun.com, zach@vmware.com, brogers@novell.com To: Avi Kivity Return-path: Received: from cantor.suse.de ([195.135.220.2]:35397 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755115AbYKELXO (ORCPT ); Wed, 5 Nov 2008 06:23:14 -0500 In-Reply-To: <4911795A.6020807@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > Alexander Graf wrote: [snip] >>>> static int kvm_resume(struct sys_device *dev) >>>> { >>>> - hardware_enable(NULL); >>>> + if (atomic_read(&kvm_usage_count)) >>>> + hardware_enable(NULL); >>>> return 0; >>>> } >>>> >>> Move the test to hardware_enable()? It's repeated too often. >>> >> >> What do we do about the on_each_cpu(hardware_enable) cases? We couldn't >> tell when to activate/deactive virtualization then, as that's >> semantically bound to "amount of VMs". >> > > I don't understand. Moving the test to within the IPI shouldn't > affect anything. > Actually it's there already. Since we have the cpu_isset if here, we won't get disabling when it's disabled or enabling when it's enabled on a per-cpu basis. That code would've just saved us the IPIs :-). So I'll add hardware_{en,dis}able_all functions that do the locking and increase / decrease the counter. Disable is used twice, while enable only once. But for the sake of code readability, I think it might be a good idea to have both of them be functions. Also the locking isn't really needed in most cases (CPU hotplug, suspend, resume, reboot, exit), since we don't schedule there. Alex