On Wed, Apr 10, 2019 at 07:04:48PM +0200, Cédric Le Goater wrote:
> When a P9 sPAPR VM boots, the CAS negotiation process determines which
> interrupt mode to use (XICS legacy or XIVE native) and invokes a
> machine reset to activate the chosen mode.
>
> To be able to switch from one mode to another, we introduce the
> capability to release a KVM device without destroying the VM. The KVM
> device interface is extended with a new 'release' operation which is
> called when the file descriptor of the device is closed.
Unfortunately, I think there is now a memory leak:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index ea2018ae1cd7..ea2619d5ca98 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2938,6 +2938,19 @@ static int kvm_device_release(struct inode *inode, struct file *filp)
> struct kvm_device *dev = filp->private_data;
> struct kvm *kvm = dev->kvm;
>
> + if (!dev)
> + return -ENODEV;
> +
> + if (dev->kvm != kvm)
> + return -EPERM;
> +
> + if (dev->ops->release) {
> + mutex_lock(&kvm->lock);
> + list_del(&dev->vm_node);
Because the device is now no longer in the kvm->devices list,
kvm_destroy_devices() won't find it there and therefore won't call the
device's destroy method. In fact now the device's destroy method will
never get called; I can't see how kvmppc_xive_free() or
kvmppc_xive_native_free() will ever get called. Thus the memory for
the kvmppc_xive structs will never get freed as far as I can see.
We could fix that by freeing both of the kvm->arch.xive_devices
entries at VM destruction time.
If it is true that any device that has a release method will never see
its destroy method being called, then that needs to be documented
clearly somewhere.
Paul.
