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. > > 2. vmf_insert_pfn loops instead of io_remap_pfn_range > > 3. proper undos when some call fails. > > 4. keep the bigger lock range to avoid unessary pte install. > > Why do we need locks at all here? 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, Alex
