On 11/17/2010 04:41 PM, Takuya Yoshikawa wrote:
(2010/11/18 11:33), Zachary Amsden wrote:
On 11/17/2010 04:04 PM, Takuya Yoshikawa wrote:
(2010/11/18 10:59), Zachary Amsden wrote:
On 11/15/2010 10:35 PM, Takuya Yoshikawa wrote:
In kvm_cpu_hotplug(), only CPU_STARTING case is protected by kvm_lock.
This patch adds missing protection for CPU_DYING case.

Signed-off-by: Takuya Yoshikawa<yoshikawa.tak...@oss.ntt.co.jp>
---
virt/kvm/kvm_main.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 339dd43..0fdd911 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2148,7 +2148,9 @@ static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned long val,
case CPU_DYING:
printk(KERN_INFO "kvm: disabling virtualization on CPU%d\n",
cpu);
+ spin_lock(&kvm_lock);
hardware_disable(NULL);
+ spin_unlock(&kvm_lock);
break;
case CPU_STARTING:
printk(KERN_INFO "kvm: enabling virtualization on CPU%d\n",

I believe this is correct.

You mean lock is not necessary?

No, I believe your patch is correct and the lock should be there. Did you test with spinlock debugging just to be sure?


Sorry but no.

I have no experience with cpu hotplug.

So I thought it would take too much time to do real test by myself and reported like this this time.

Any easy way to test?

Yes, quite easy. Some systems may not let cpu0 go offline, but you can manually disable and re-enable the other processors:

[r...@mysore ~]# echo "0" > /sys/devices/system/cpu/cpu1/online
[r...@mysore ~]# echo "1" > /sys/devices/system/cpu/cpu1/online

Cheers,

Zach
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to