| Message ID | 1343187070-27371-6-git-send-email-qemulist@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 25, 2012 at 4:31 AM, Liu Ping Fan <qemulist@gmail.com> wrote: > From: Liu Ping Fan <pingfank@linux.vnet.ibm.com> > > Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com> > --- > hw/e1000.c | 15 +++++++++++++-- > 1 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/hw/e1000.c b/hw/e1000.c > index 4573f13..4c1e141 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -1192,6 +1192,18 @@ e1000_cleanup(VLANClientState *nc) > s->nic = NULL; > } > > +static void > +pci_e1000_unmap(DeviceState *dev) > +{ > + PCIDevice *p = PCI_DEVICE(dev); > + E1000State *d = DO_UPCAST(E1000State, dev, p); > + > + /* DO NOT FREE anything!until refcnt=0 */ > + /* isolate from memory view */ > + memory_region_destroy(&d->mmio); > + memory_region_destroy(&d->io); > +} It's not obvious to me why a 2-stage cleanup is needed (->unmap(), ->exit()). Explaining things a bit more in the commit description would help. Here's what I'm thinking: We want to remove the memory regions at the same time as removing the device from the tree, but ->exit() is only called when the object is finalized. Because of the object reference held during dispatch, the reference might not reach 0 during hotplug and another thread could still be running this device's code? This series only applies this change to e1000 and piix pci hotplug. How/when will all the other devices be converted? Will it be safe to leave them unconverted once dispatch really happens in parallel? Stefan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/hw/e1000.c b/hw/e1000.c index 4573f13..4c1e141 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -1192,6 +1192,18 @@ e1000_cleanup(VLANClientState *nc) s->nic = NULL; } +static void +pci_e1000_unmap(DeviceState *dev) +{ + PCIDevice *p = PCI_DEVICE(dev); + E1000State *d = DO_UPCAST(E1000State, dev, p); + + /* DO NOT FREE anything!until refcnt=0 */ + /* isolate from memory view */ + memory_region_destroy(&d->mmio); + memory_region_destroy(&d->io); +} + static int pci_e1000_uninit(PCIDevice *dev) { @@ -1199,8 +1211,6 @@ pci_e1000_uninit(PCIDevice *dev) qemu_del_timer(d->autoneg_timer); qemu_free_timer(d->autoneg_timer); - memory_region_destroy(&d->mmio); - memory_region_destroy(&d->io); qemu_del_vlan_client(&d->nic->nc); return 0; } @@ -1283,6 +1293,7 @@ static void e1000_class_init(ObjectClass *klass, void *data) k->class_id = PCI_CLASS_NETWORK_ETHERNET; dc->desc = "Intel Gigabit Ethernet"; dc->reset = qdev_e1000_reset; + dc->unmap = pci_e1000_unmap; dc->vmsd = &vmstate_e1000; dc->props = e1000_properties; }