Re: [Qemu-devel] [PATCH] vfio: free dynamically-allocated data in instance_finalize

[Paolo Bonzini]

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 <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>>      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