On Wed, May 07, 2025 at 09:36:48AM -0300, Jason Gunthorpe wrote: > On Tue, May 06, 2025 at 01:54:10PM -0700, Nicolin Chen wrote: > > > Now I start to think about the FD situation: either a fault queue or > > an eventq returns an FD and holds a refcount on the event object. So > > the event object can't be destroyed unless the FD is released first. > > Are we doing it incorrectly here? > > Not necessarily, and maybe that is a good point that we are already > doing these cross dependencies. But we could fix the FDs with some > work too. Noted it down. Similar to unmap_mapping_range() in the mmap case, is close_fd() or file_close_fd() the one that we should use in the event object destroy()? Either of which is holding a spinlock, so likely it's safe from a close racing, v.s. the mmap situation? > > I see! It just needs to call that function when we remove the mmap > > for a vIOMMU destroy(). > > It is a little fussy to setup the inode as well, but yes. > > The other small advantage is you don't need to setup a special VMA ops > and do VMA tracking to hold the refcount on the vqueue object. > > But there is also any annoying race with unmap in setting up the mmap > PFNs which is why vfio is doing it from a fault handler.. I see. That's why VFIO is doing vmf_insert_pfn() in the fault op, instead of calling remap_pfn_range() in mmap(). > So maybe refcount is the simplest option for now. We could fix it > later and it won't prevent close() from working right. Ack. I will keep the one that I posted in v3. Thanks Nicolin
