From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49049) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIhNa-0007cp-RE for qemu-devel@nongnu.org; Tue, 03 Feb 2015 12:35:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YIhNV-000058-NV for qemu-devel@nongnu.org; Tue, 03 Feb 2015 12:35:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44478) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIhNV-0008WC-GY for qemu-devel@nongnu.org; Tue, 03 Feb 2015 12:35:09 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t13HZ8TU027496 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 3 Feb 2015 12:35:08 -0500 Message-ID: <1422984907.22865.458.camel@redhat.com> From: Alex Williamson Date: Tue, 03 Feb 2015 10:35:07 -0700 In-Reply-To: <1422984376.22865.456.camel@redhat.com> References: <1422967705-1374-1-git-send-email-pbonzini@redhat.com> <1422976808.22865.452.camel@redhat.com> <54D0F675.8070000@redhat.com> <1422984376.22865.456.camel@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vfio: free dynamically-allocated data in instance_finalize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On Tue, 2015-02-03 at 10:26 -0700, Alex Williamson wrote: > On Tue, 2015-02-03 at 17:25 +0100, Paolo Bonzini wrote: > > > > 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 > > >> Signed-off-by: Paolo Bonzini > > >> --- > > >> 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. > > Hmm, except qemu segfaults in whatever sanity test/capabilities probing > happens when the VM is first opened. I haven't figured out how to > capture that instance in gdb yet. Ah, simply running -device vfio-pci,? causes it: #0 0x000055555567cb30 in vfio_put_base_device (vbasedev=0x55555636e320) at /net/gimli/home/alwillia/Work/qemu.git/hw/vfio/common.c:911 #1 0x00005555556847a2 in vfio_put_device (vdev=0x55555636dab0) at /net/gimli/home/alwillia/Work/qemu.git/hw/vfio/pci.c:3120 #2 0x00005555556853aa in vfio_instance_finalize (obj=0x55555636dab0) at /net/gimli/home/alwillia/Work/qemu.git/hw/vfio/pci.c:3394 #3 0x00005555558c628b in object_deinit (obj=0x55555636dab0, type=0x555556322aa0) at qom/object.c:399 #4 0x00005555558c62fc in object_finalize (data=0x55555636dab0) at qom/object.c:413 #5 0x00005555558c6be9 in object_unref (obj=0x55555636dab0) at qom/object.c:720 #6 0x000055555574b200 in qmp_device_list_properties ( typename=0x55555635f1b0 "vfio-pci", errp=0x7fffffffdde8) at qmp.c:555 #7 0x000055555571ead4 in qdev_device_help (opts=0x55555635f100) at qdev-monitor.c:243 #8 0x0000555555730a1c in device_help_func (opts=0x55555635f100, opaque=0x0) at vl.c:2120 #9 0x00005555559c14d0 in qemu_opts_foreach ( list=0x555555dfd3c0 , func=0x555555730a00 , opaque=0x0, abort_on_failure=0) at util/qemu-option.c:1057 #10 0x000055555573564e in main (argc=13, argv=0x7fffffffe2b8, envp=0x7fffffffe328) at vl.c:4010