qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] vfio: free dynamically-allocated data in instance_finalize
Date: Tue, 03 Feb 2015 17:25:25 +0100	[thread overview]
Message-ID: <54D0F675.8070000@redhat.com> (raw)
In-Reply-To: <1422976808.22865.452.camel@redhat.com>



On 03/02/2015 16:20, Alex Williamson wrote:
> On Tue, 2015-02-03 at 13:48 +0100, Paolo Bonzini wrote:
>> In order to enable out-of-BQL address space lookup, destruction of
>> devices needs to be split in two phases.
>>
>> Unrealize is the first phase; once it complete no new accesses will
>> be started, but there may still be pending memory accesses can still
>> be completed.
>>
>> The second part is freeing the device, which only happens once all memory
>> accesses are complete.  At this point the reference count has dropped to
>> zero, an RCU grace period must have completed (because the RCU-protected
>> FlatViews hold a reference to the device via memory_region_ref).  This is
>> when instance_finalize is called.
>>
>> Freeing data belongs in an instance_finalize callback, because the
>> dynamically allocated memory can still be used after unrealize by the
>> pending memory accesses.
>>
>> In the case of VFIO, the unrealize callback is too early to munmap the
>> BARs.  The munmap must be delayed until memory accesses are complete.
>> To do this, split vfio_unmap_bars in two.  The removal step, now called
>> vfio_unregister_bars, remains in vfio_exitfn.  The reclamation step
>> is vfio_unmap_bars and is moved to the instance_finalize callback.
>>
>> Similarly, quirk MemoryRegions have to be removed during
>> vfio_unregister_bars, but freeing the data structure must be delayed
>> to vfio_unmap_bars.
>>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> 	This patch is part of the third installment 3 of the RCU work.
>> 	Sending it out separately for Alex to review it.
>>
>>  hw/vfio/pci.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 68 insertions(+), 10 deletions(-)
> 
> Looks good to me.  I don't see any external dependencies, so do you want
> me to pull this in through my branch?  Thanks,

Yes, please.

Paolo

  reply	other threads:[~2015-02-03 16:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-03 12:48 [Qemu-devel] [PATCH] vfio: free dynamically-allocated data in instance_finalize Paolo Bonzini
2015-02-03 15:20 ` Alex Williamson
2015-02-03 16:25   ` Paolo Bonzini [this message]
2015-02-03 17:26     ` Alex Williamson
2015-02-03 17:35       ` Alex Williamson
2015-02-03 18:51       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54D0F675.8070000@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).