Re: [PATCH 1/2] kvm: destroy emulated devices on VM exit

2013-04-25 Thread Alexander Graf

On 26.04.2013, at 02:11, Scott Wood wrote:

> The hassle of getting refcounting right was greater than the hassle
> of keeping a list of devices to destroy on VM exit.
> 
> Signed-off-by: Scott Wood 

Thanks, applied both to my irqfd mpic queue.


Alex

> ---
> arch/powerpc/kvm/mpic.c  |2 --
> include/linux/kvm_host.h |3 ++-
> virt/kvm/kvm_main.c  |   29 -
> 3 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
> index d137df8..4ac98d1 100644
> --- a/arch/powerpc/kvm/mpic.c
> +++ b/arch/powerpc/kvm/mpic.c
> @@ -1788,7 +1788,6 @@ int kvmppc_mpic_connect_vcpu(struct kvm_device *dev, 
> struct kvm_vcpu *vcpu,
>   if (opp->mpic_mode_mask == GCR_MODE_PROXY)
>   vcpu->arch.epr_flags |= KVMPPC_EPR_KERNEL;
> 
> - kvm_device_get(dev);
> out:
>   spin_unlock_irq(&opp->lock);
>   return ret;
> @@ -1804,7 +1803,6 @@ void kvmppc_mpic_disconnect_vcpu(struct openpic *opp, 
> struct kvm_vcpu *vcpu)
>   BUG_ON(!opp->dst[vcpu->arch.irq_cpu_id].vcpu);
> 
>   opp->dst[vcpu->arch.irq_cpu_id].vcpu = NULL;
> - kvm_device_put(opp->dev);
> }
> 
> /*
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index feffbda..36c9776 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -393,6 +393,7 @@ struct kvm {
>   long mmu_notifier_count;
> #endif
>   long tlbs_dirty;
> + struct list_head devices;
> };
> 
> #define kvm_err(fmt, ...) \
> @@ -1069,8 +1070,8 @@ struct kvm_device_ops;
> struct kvm_device {
>   struct kvm_device_ops *ops;
>   struct kvm *kvm;
> - atomic_t users;
>   void *private;
> + struct list_head vm_node;
> };
> 
> /* create, destroy, and name are mandatory */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f6cd14d..5da9f02 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -504,6 +504,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>   mutex_init(&kvm->irq_lock);
>   mutex_init(&kvm->slots_lock);
>   atomic_set(&kvm->users_count, 1);
> + INIT_LIST_HEAD(&kvm->devices);
> 
>   r = kvm_init_mmu_notifier(kvm);
>   if (r)
> @@ -581,6 +582,19 @@ void kvm_free_physmem(struct kvm *kvm)
>   kfree(kvm->memslots);
> }
> 
> +static void kvm_destroy_devices(struct kvm *kvm)
> +{
> + struct list_head *node, *tmp;
> +
> + list_for_each_safe(node, tmp, &kvm->devices) {
> + struct kvm_device *dev =
> + list_entry(node, struct kvm_device, vm_node);
> +
> + list_del(node);
> + dev->ops->destroy(dev);
> + }
> +}
> +
> static void kvm_destroy_vm(struct kvm *kvm)
> {
>   int i;
> @@ -600,6 +614,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>   kvm_arch_flush_shadow_all(kvm);
> #endif
>   kvm_arch_destroy_vm(kvm);
> + kvm_destroy_devices(kvm);
>   kvm_free_physmem(kvm);
>   cleanup_srcu_struct(&kvm->srcu);
>   kvm_arch_free_vm(kvm);
> @@ -2195,23 +2210,11 @@ static long kvm_device_ioctl(struct file *filp, 
> unsigned int ioctl,
>   }
> }
> 
> -void kvm_device_get(struct kvm_device *dev)
> -{
> - atomic_inc(&dev->users);
> -}
> -
> -void kvm_device_put(struct kvm_device *dev)
> -{
> - if (atomic_dec_and_test(&dev->users))
> - dev->ops->destroy(dev);
> -}
> -
> static int kvm_device_release(struct inode *inode, struct file *filp)
> {
>   struct kvm_device *dev = filp->private_data;
>   struct kvm *kvm = dev->kvm;
> 
> - kvm_device_put(dev);
>   kvm_put_kvm(kvm);
>   return 0;
> }
> @@ -2257,7 +2260,6 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
> 
>   dev->ops = ops;
>   dev->kvm = kvm;
> - atomic_set(&dev->users, 1);
> 
>   ret = ops->create(dev, cd->type);
>   if (ret < 0) {
> @@ -2271,6 +2273,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>   return ret;
>   }
> 
> + list_add(&dev->vm_node, &kvm->devices);
>   kvm_get_kvm(kvm);
>   cd->fd = ret;
>   return 0;
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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


[PATCH 1/2] kvm: destroy emulated devices on VM exit

2013-04-25 Thread Scott Wood
The hassle of getting refcounting right was greater than the hassle
of keeping a list of devices to destroy on VM exit.

Signed-off-by: Scott Wood 
---
 arch/powerpc/kvm/mpic.c  |2 --
 include/linux/kvm_host.h |3 ++-
 virt/kvm/kvm_main.c  |   29 -
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
index d137df8..4ac98d1 100644
--- a/arch/powerpc/kvm/mpic.c
+++ b/arch/powerpc/kvm/mpic.c
@@ -1788,7 +1788,6 @@ int kvmppc_mpic_connect_vcpu(struct kvm_device *dev, 
struct kvm_vcpu *vcpu,
if (opp->mpic_mode_mask == GCR_MODE_PROXY)
vcpu->arch.epr_flags |= KVMPPC_EPR_KERNEL;
 
-   kvm_device_get(dev);
 out:
spin_unlock_irq(&opp->lock);
return ret;
@@ -1804,7 +1803,6 @@ void kvmppc_mpic_disconnect_vcpu(struct openpic *opp, 
struct kvm_vcpu *vcpu)
BUG_ON(!opp->dst[vcpu->arch.irq_cpu_id].vcpu);
 
opp->dst[vcpu->arch.irq_cpu_id].vcpu = NULL;
-   kvm_device_put(opp->dev);
 }
 
 /*
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index feffbda..36c9776 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -393,6 +393,7 @@ struct kvm {
long mmu_notifier_count;
 #endif
long tlbs_dirty;
+   struct list_head devices;
 };
 
 #define kvm_err(fmt, ...) \
@@ -1069,8 +1070,8 @@ struct kvm_device_ops;
 struct kvm_device {
struct kvm_device_ops *ops;
struct kvm *kvm;
-   atomic_t users;
void *private;
+   struct list_head vm_node;
 };
 
 /* create, destroy, and name are mandatory */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f6cd14d..5da9f02 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -504,6 +504,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
mutex_init(&kvm->irq_lock);
mutex_init(&kvm->slots_lock);
atomic_set(&kvm->users_count, 1);
+   INIT_LIST_HEAD(&kvm->devices);
 
r = kvm_init_mmu_notifier(kvm);
if (r)
@@ -581,6 +582,19 @@ void kvm_free_physmem(struct kvm *kvm)
kfree(kvm->memslots);
 }
 
+static void kvm_destroy_devices(struct kvm *kvm)
+{
+   struct list_head *node, *tmp;
+
+   list_for_each_safe(node, tmp, &kvm->devices) {
+   struct kvm_device *dev =
+   list_entry(node, struct kvm_device, vm_node);
+
+   list_del(node);
+   dev->ops->destroy(dev);
+   }
+}
+
 static void kvm_destroy_vm(struct kvm *kvm)
 {
int i;
@@ -600,6 +614,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
kvm_arch_flush_shadow_all(kvm);
 #endif
kvm_arch_destroy_vm(kvm);
+   kvm_destroy_devices(kvm);
kvm_free_physmem(kvm);
cleanup_srcu_struct(&kvm->srcu);
kvm_arch_free_vm(kvm);
@@ -2195,23 +2210,11 @@ static long kvm_device_ioctl(struct file *filp, 
unsigned int ioctl,
}
 }
 
-void kvm_device_get(struct kvm_device *dev)
-{
-   atomic_inc(&dev->users);
-}
-
-void kvm_device_put(struct kvm_device *dev)
-{
-   if (atomic_dec_and_test(&dev->users))
-   dev->ops->destroy(dev);
-}
-
 static int kvm_device_release(struct inode *inode, struct file *filp)
 {
struct kvm_device *dev = filp->private_data;
struct kvm *kvm = dev->kvm;
 
-   kvm_device_put(dev);
kvm_put_kvm(kvm);
return 0;
 }
@@ -2257,7 +2260,6 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
 
dev->ops = ops;
dev->kvm = kvm;
-   atomic_set(&dev->users, 1);
 
ret = ops->create(dev, cd->type);
if (ret < 0) {
@@ -2271,6 +2273,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
return ret;
}
 
+   list_add(&dev->vm_node, &kvm->devices);
kvm_get_kvm(kvm);
cd->fd = ret;
return 0;
-- 
1.7.10.4


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