From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41945) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ayS1W-0007fO-MR for qemu-devel@nongnu.org; Thu, 05 May 2016 18:45:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ayS1K-0005WE-Pt for qemu-devel@nongnu.org; Thu, 05 May 2016 18:45:29 -0400 Date: Thu, 5 May 2016 16:45:04 -0600 From: Alex Williamson Message-ID: <20160505164504.0f9d3b03@t450s.home> In-Reply-To: <1462344751-28281-3-git-send-email-aik@ozlabs.ru> References: <1462344751-28281-1-git-send-email-aik@ozlabs.ru> <1462344751-28281-3-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH qemu v16 02/19] memory: Call region_del() callbacks on memory listener unregistering List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Alexander Graf , David Gibson , Paolo Bonzini On Wed, 4 May 2016 16:52:14 +1000 Alexey Kardashevskiy wrote: > When a new memory listener is registered, listener_add_address_space() > is called and which in turn calls region_add() callbacks of memory regions. > However when unregistering the memory listener, it is just removed from > the listening chain and no region_del() is called. > > This adds listener_del_address_space() and uses it in > memory_listener_unregister(). listener_add_address_space() was used as > a template with the following changes: > s/log_global_start/log_global_stop/ > s/log_start/log_stop/ > s/region_add/region_del/ > > This will allow the following patches to add/remove DMA windows > dynamically from VFIO's PCI address space's region_add()/region_del(). Following patch 1 comments, it would be a bug if the kernel actually needed this to do cleanup, we must release everything if QEMU gets shot with a SIGKILL anyway. So what does this cleanup facilitate in QEMU? Having QEMU trigger an unmap for each region_del is not going to be as efficient as just dropping the container and letting the kernel handle the cleanup all in one go. Thanks, Alex > > Signed-off-by: Alexey Kardashevskiy > --- > memory.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/memory.c b/memory.c > index f76f85d..f762a34 100644 > --- a/memory.c > +++ b/memory.c > @@ -2185,6 +2185,49 @@ static void listener_add_address_space(MemoryListener *listener, > flatview_unref(view); > } > > +static void listener_del_address_space(MemoryListener *listener, > + AddressSpace *as) > +{ > + FlatView *view; > + FlatRange *fr; > + > + if (listener->address_space_filter > + && listener->address_space_filter != as) { > + return; > + } > + > + if (listener->begin) { > + listener->begin(listener); > + } > + if (global_dirty_log) { > + if (listener->log_global_stop) { > + listener->log_global_stop(listener); > + } > + } > + > + view = address_space_get_flatview(as); > + FOR_EACH_FLAT_RANGE(fr, view) { > + MemoryRegionSection section = { > + .mr = fr->mr, > + .address_space = as, > + .offset_within_region = fr->offset_in_region, > + .size = fr->addr.size, > + .offset_within_address_space = int128_get64(fr->addr.start), > + .readonly = fr->readonly, > + }; > + if (fr->dirty_log_mask && listener->log_stop) { > + listener->log_stop(listener, §ion, 0, fr->dirty_log_mask); > + } > + if (listener->region_del) { > + listener->region_del(listener, §ion); > + } > + } > + if (listener->commit) { > + listener->commit(listener); > + } > + flatview_unref(view); > +} > + > void memory_listener_register(MemoryListener *listener, AddressSpace *filter) > { > MemoryListener *other = NULL; > @@ -2211,6 +2254,11 @@ void memory_listener_register(MemoryListener *listener, AddressSpace *filter) > > void memory_listener_unregister(MemoryListener *listener) > { > + AddressSpace *as; > + > + QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { > + listener_del_address_space(listener, as); > + } > QTAILQ_REMOVE(&memory_listeners, listener, link); > } >