On Tue, Mar 09, 2021 at 08:29:51AM -0700, Alex Williamson wrote: > On Tue, 9 Mar 2021 08:46:09 -0400 > Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > > On Tue, Mar 09, 2021 at 03:49:09AM +0000, Zengtao (B) wrote: > > > Hi guys: > > > > > > Thanks for the helpful comments, after rethinking the issue, I have proposed > > > the following change: > > > 1. follow_pte instead of follow_pfn. > > > > Still no on follow_pfn, you don't need it once you use vmf_insert_pfn > > vmf_insert_pfn() only solves the BUG_ON, follow_pte() is being used > here to determine whether the translation is already present to avoid > both duplicate work in inserting the translation and allocating a > duplicate vma tracking structure. Oh.. Doing something stateful in fault is not nice at all I would rather see __vfio_pci_add_vma() search the vma_list for dups than call follow_pfn/pte.. > For the vma tracking and testing whether the fault is already > populated. Once we get rid of the vma list, maybe it makes sense to > only insert the faulting page rather than the entire vma, at which > point I think we'd have no reason to serialize. Thanks, Yes, the address_space stuff is a much better solution to all of this. Jason
