Re: [Qemu-devel] [PATCH v5 01/11] memory: Sanity check that no listeners remain on a destroyed AddressSpace

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

On Thu, Mar 20, 2014 at 11:20:09AM +0100, Paolo Bonzini wrote:
> Il 12/03/2014 06:52, Alexey Kardashevskiy ha scritto:
> >From: David Gibson <david@gibson.dropbear.id.au>
> >
> >At the moment, most AddressSpace objects last as long as the guest system
> >in practice, but that could well change in future.  In addition, for VFIO
> >we will be introducing some private per-AdressSpace information, which must
> >be disposed of before the AddressSpace itself is destroyed.
> >
> >To reduce the chances of subtle bugs in this area, this patch adds
> >asssertions to ensure that when an AddressSpace is destroyed, there are no
> >remaining MemoryListeners using that AS as a filter.
> >
> >Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >---
> > memory.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> >diff --git a/memory.c b/memory.c
> >index 3f1df23..678661e 100644
> >--- a/memory.c
> >+++ b/memory.c
> >@@ -1722,12 +1722,19 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
> >
> > void address_space_destroy(AddressSpace *as)
> > {
> >+    MemoryListener *listener;
> >+
> >     /* Flush out anything from MemoryListeners listening in on this */
> >     memory_region_transaction_begin();
> >     as->root = NULL;
> >     memory_region_transaction_commit();
> >     QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
> >     address_space_destroy_dispatch(as);
> >+
> >+    QTAILQ_FOREACH(listener, &memory_listeners, link) {
> >+        assert(listener->address_space_filter != as);
> >+    }
> >+
> >     flatview_unref(as->current_map);
> >     g_free(as->name);
> >     g_free(as->ioeventfds);
> >
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> An alternative is to add a count of listeners to the address space
> and assert that it is 0.

Address space destruction should be pretty rare, so I don't think it's
worth the bother of adding a count, which just adds the possibility of
bugs updating it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]