On Thu, May 05, 2016 at 04:45:04PM -0600, Alex Williamson wrote: > 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, So, what the kernel does is kind of a red herring, because that's only relevant to the specific case of the VFIO listener, whereas this is a change to the behaviour of all memory listeners. It seems plausible that some memory listeners could have a legitimate reason to want clean up region_del calls when unregistered. But, we know this could be expensive for other listeners, so I don't think we should make that behaviour standard. So I'd be thinking either a special unregister_with_delete() call, or a standalone "delete all" helper function. Assuming this is still needed at all, once the other changes to the reference counting we've discussed have been done. -- 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