From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33485) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLuep-0008Sh-1b for qemu-devel@nongnu.org; Tue, 17 Sep 2013 08:45:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VLuej-00054u-3F for qemu-devel@nongnu.org; Tue, 17 Sep 2013 08:45:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44897) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLuei-00054l-Rn for qemu-devel@nongnu.org; Tue, 17 Sep 2013 08:45:25 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8HCjOHC018732 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 17 Sep 2013 08:45:24 -0400 Date: Tue, 17 Sep 2013 15:47:24 +0300 From: "Michael S. Tsirkin" Message-ID: <20130917124724.GA18965@redhat.com> References: <1378211609-16121-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1378211609-16121-1-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 00/38] Delay destruction of memory regions to instance_finalize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On Tue, Sep 03, 2013 at 02:32:51PM +0200, Paolo Bonzini wrote: > QOM splits the destruction of a device in two phases: > > - unrealize, also known as "exit" from qdev times, should isolate > the device from the guest. After unrealize returns, the guest > should not be able to issue new requests. > > - instance_finalize will reclaim the memory. This is only called > after all requests terminate and drop the references on the > device. > > Though overlooked, this is important even now: QEMU's little secret is > that devices already do access memory out of the iothread mutex (with > address_space_map/unmap and AIO), and this can be MMIO memory too > through a bounce buffer. This series prepares things so that, once > we'll put the memory_region_ref/unref infrastructure to complete use, > things will just work. > > Of course this split will be particularly important for devices that > will be able to do unlocked MMIO. > > This series changes all PCI devices (the sole to support hotplug _and_ > use MemoryRegions) to do memory_region_del_subregion at unrealize time, > and memory_region_destroy at instance_finalize time. As it is mostly > a PCI patch, it should go through mst's tree. > > Paolo OK so this is the problem. Memory region reference counting actually does not have a reference count per MR. Instead it takes a reference to device: void memory_region_ref(MemoryRegion *mr) { if (mr && mr->owner) { object_ref(mr->owner); } } void memory_region_unref(MemoryRegion *mr) { if (mr && mr->owner) { object_unref(mr->owner); } } Now object_ref only delays finalize. Ergo, to make sure a referenced MR does not get destroyed, we must make sure only finalize calls memory_region_destroy. So I think this patchset should do exactly that, not try to move out more stuff to finalize. -- MST