From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Hajnoczi Subject: Re: [PATCH 5/5] e1000: using new interface--unmap to unplug Date: Wed, 25 Jul 2012 08:12:13 +0100 Message-ID: References: <1343187070-27371-1-git-send-email-qemulist@gmail.com> <1343187070-27371-6-git-send-email-qemulist@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, Anthony Liguori , Avi Kivity , Jan Kiszka , Marcelo Tosatti To: Liu Ping Fan Return-path: Received: from mail-lpp01m010-f46.google.com ([209.85.215.46]:40400 "EHLO mail-lpp01m010-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751858Ab2GYHMP (ORCPT ); Wed, 25 Jul 2012 03:12:15 -0400 Received: by lahd3 with SMTP id d3so308793lah.19 for ; Wed, 25 Jul 2012 00:12:14 -0700 (PDT) In-Reply-To: <1343187070-27371-6-git-send-email-qemulist@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Jul 25, 2012 at 4:31 AM, Liu Ping Fan wrote: > From: Liu Ping Fan > > Signed-off-by: Liu Ping Fan > --- > 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