On Tue, Feb 23, 2021 at 02:48:20PM -0800, Dan Williams wrote: > On Tue, Feb 23, 2021 at 10:54 AM Jason Gunthorpe <jgg@xxxxxxxx> wrote: > > > > On Tue, Feb 23, 2021 at 08:44:52AM -0800, Dan Williams wrote: > > > > > > The downside would be one extra lookup in dev_pagemap tree > > > > for other pgmap->types (P2P, FSDAX, PRIVATE). But just one > > > > per gup-fast() call. > > > > > > I'd guess a dev_pagemap lookup is faster than a get_user_pages slow > > > path. It should be measurable that this change is at least as fast or > > > faster than falling back to the slow path, but it would be good to > > > measure. > > > > What is the dev_pagemap thing doing in gup fast anyhow? > > > > I've been wondering for a while.. > > It's there to synchronize against dax-device removal. The device will > suspend removal awaiting all page references to be dropped, but > gup-fast could be racing device removal. So gup-fast checks for > pte_devmap() to grab a live reference to the device before assuming it > can pin a page. >From the perspective of CPU A it can't tell if CPU B is doing a HW page table walk or a GUP fast when it invalidates a page table. The design of gup-fast is supposed to be the same as the design of a HW page table walk, and the tlb invalidate CPU A does when removing a page from a page table is supposed to serialize against both a HW page table walk and gup-fast. Given that the HW page table walker does not do dev_pagemap stuff, why does gup-fast? Can you sketch the exact race this is protecting against? Jason
