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
next prev parent 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).