On Tue, Mar 09, 2021 at 08:29:51AM -0700, Alex Williamson wrote:
> On Tue, 9 Mar 2021 08:46:09 -0400
> Jason Gunthorpe <j...@nvidia.com> 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

Reply via email to