Re: [PATCH] KVM: Destroy target device if coalesced MMIO unregistration fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/20/2022 11:04 AM, Wang, Wei W wrote:
On Tuesday, December 20, 2022 1:19 AM, Sean Christopherson wrote:
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c index
0be80c213f7f..5ef88f5a0864 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -187,15 +187,17 @@ int
kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
  			r = kvm_io_bus_unregister_dev(kvm,
  				zone->pio ? KVM_PIO_BUS : KVM_MMIO_BUS, &dev->dev);

+			kvm_iodevice_destructor(&dev->dev);
+
  			/*
  			 * On failure, unregister destroys all devices on the
  			 * bus _except_ the target device, i.e. coalesced_zones
-			 * has been modified.  No need to restart the walk as
-			 * there aren't any zones left.
+			 * has been modified.  Bail after destroying the target
+			 * device, there's no need to restart the walk as there
+			 * aren't any zones left.
  			 */
  			if (r)
  				break;
-			kvm_iodevice_destructor(&dev->dev);
  		}
  	}
Another option is to let kvm_io_bus_unregister_dev handle this, and no need for callers
to make the extra kvm_iodevice_destructor() call. This simplifies the usage for callers
(e.g. reducing LOCs and no leakages like this):

One vote for this option : )



diff --git a/include/kvm/iodev.h b/include/kvm/iodev.h
index d75fc4365746..56619e33251e 100644
--- a/include/kvm/iodev.h
+++ b/include/kvm/iodev.h
@@ -55,10 +55,4 @@ static inline int kvm_iodevice_write(struct kvm_vcpu *vcpu,
                                  : -EOPNOTSUPP;
  }

-static inline void kvm_iodevice_destructor(struct kvm_io_device *dev)
-{
-       if (dev->ops->destructor)
-               dev->ops->destructor(dev);
-}
-
  #endif /* __KVM_IODEV_H__ */
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 0be80c213f7f..d7135a5e76f8 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -195,7 +195,6 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
                          */
                         if (r)
                                 break;
-                       kvm_iodevice_destructor(&dev->dev);
                 }
         }

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 2a3ed401ce46..1b277afb545b 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -898,7 +898,6 @@ kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
                 bus = kvm_get_bus(kvm, bus_idx);
                 if (bus)
                         bus->ioeventfd_count--;
-               ioeventfd_release(p);
                 ret = 0;
                 break;
         }
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 13e88297f999..582757ebdce6 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5200,6 +5200,12 @@ static struct notifier_block kvm_reboot_notifier = {
         .priority = 0,
  };

+static void kvm_iodevice_destructor(struct kvm_io_device *dev)
+{
+       if (dev->ops->destructor)
+               dev->ops->destructor(dev);
+}
+
  static void kvm_io_bus_destroy(struct kvm_io_bus *bus)
  {
         int i;
@@ -5423,7 +5429,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
  int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
                               struct kvm_io_device *dev)
  {
-       int i, j;
+       int i;
         struct kvm_io_bus *new_bus, *bus;

         lockdep_assert_held(&kvm->slots_lock);
@@ -5453,18 +5459,18 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
         rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
         synchronize_srcu_expedited(&kvm->srcu);

-       /* Destroy the old bus _after_ installing the (null) bus. */
+       /*
+        * If (null) bus is installed, destroy the old bus, including all the
+        * attached devices. Otherwise, destroy the caller's device only.
+        */
         if (!new_bus) {
                 pr_err("kvm: failed to shrink bus, removing it completely\n");
-               for (j = 0; j < bus->dev_count; j++) {
-                       if (j == i)
-                               continue;
-                       kvm_iodevice_destructor(bus->range[j].dev);
-               }
+               kvm_io_bus_destroy(bus);
+               return -ENOMEM;
         }

-       kfree(bus);
-       return new_bus ? 0 : -ENOMEM;
+       kvm_iodevice_destructor(dev);
+       return 0;
  }

  struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux