From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38325) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UYCPE-0006Yt-Uy for qemu-devel@nongnu.org; Fri, 03 May 2013 05:35:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UYCPD-0000Ps-Iy for qemu-devel@nongnu.org; Fri, 03 May 2013 05:35:56 -0400 Received: from mail-we0-x231.google.com ([2a00:1450:400c:c03::231]:39593) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UYCPC-0000Pi-WF for qemu-devel@nongnu.org; Fri, 03 May 2013 05:35:55 -0400 Received: by mail-we0-f177.google.com with SMTP id u3so1131074wey.36 for ; Fri, 03 May 2013 02:35:54 -0700 (PDT) Date: Fri, 3 May 2013 11:35:50 +0200 From: Stefan Hajnoczi Message-ID: <20130503093550.GC24864@stefanha-thinkpad.redhat.com> References: <1367549122-2948-1-git-send-email-qemulist@gmail.com> <1367549122-2948-5-git-send-email-qemulist@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1367549122-2948-5-git-send-email-qemulist@gmail.com> Subject: Re: [Qemu-devel] [PATCH v2 4/6] hostmem: hostmem listener pin RAM-Device by refcnt List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liu Ping Fan Cc: Peter Maydell , Anthony Liguori , Jan Kiszka , qemu-devel@nongnu.org, Vasilis Liaskovitis , Stefan Hajnoczi , Paolo Bonzini On Fri, May 03, 2013 at 10:45:20AM +0800, Liu Ping Fan wrote: > From: Liu Ping Fan > > With ref()/unref() interface of MemoryRegion, we can pin RAM-Device > when using its memory, and release it when done. > > Signed-off-by: Liu Ping Fan > --- > hw/virtio/dataplane/hostmem.c | 44 +++++++++++++++++++++++++++----- > hw/virtio/dataplane/vring.c | 8 +++--- > include/hw/virtio/dataplane/hostmem.h | 4 ++- > 3 files changed, 44 insertions(+), 12 deletions(-) > > diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c > index 1fd3e06..0e28dfc 100644 > --- a/hw/virtio/dataplane/hostmem.c > +++ b/hw/virtio/dataplane/hostmem.c > @@ -42,18 +42,28 @@ static void hostmem_ref(HostMem *hostmem) > > static void hostmem_unref(HostMem *hostmem) > { > - int t; > + int i, t; > + HostMemRegion *hmr; > > t = __sync_sub_and_fetch(&hostmem->ref, 1); > assert(t >= 0); > if (!t) { > + for (i = 0; i < hostmem->num_current_regions; i++) { > + hmr = &hostmem->current_regions[i]; > + /* Fix me, when memory hotunplug implement > + * assert(hmr->mr_ops->unref) > + */ > + if (hmr->mr->ops && hmr->mr->ops->unref) { > + hmr->mr->ops->unref(); > + } This patch should use memory_region_ref()/unref() which you introduced in the previous patch instead of open-coding this. > @@ -158,6 +176,18 @@ static void hostmem_listener_append_region(MemoryListener *listener, > hostmem_append_new_region(as_mem->next_hostmem, section); > } > > +static void hostmem_listener_append_region(MemoryListener *listener, > + MemoryRegionSection *section) > +{ > + hostmem_listener_nop_region(listener, section); > + /* Fix me, when memory hotunplug implement > + * assert(section->mr->ops->ref) > + */ > + if (section->mr->ops && section->mr->ops->ref) { > + section->mr->ops->ref(); > + } > +} Why does append increment the refcount while nop does not? It seems that we always need to increment the memory region refcount since we're building a completely new hostmem. The refcount ownership is not passed from the current hostmen to the next hostmem, all memory regions are released in hostmem_unref().