qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH qemu v16 02/19] memory: Call region_del() callba


From: David Gibson
Subject: Re: [Qemu-devel] [PATCH qemu v16 02/19] memory: Call region_del() callbacks on memory listener unregistering
Date: Thu, 26 May 2016 11:48:44 +1000
User-agent: Mutt/1.6.1 (2016-04-27)

On Thu, May 05, 2016 at 04:45:04PM -0600, Alex Williamson wrote:
> On Wed,  4 May 2016 16:52:14 +1000
> Alexey Kardashevskiy <address@hidden> 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

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]