On Fri, Aug 16, 2019 at 06:13:55PM +0200, Jan Kara wrote: > On Fri 16-08-19 11:52:20, Jerome Glisse wrote: > > On Fri, Aug 16, 2019 at 05:44:04PM +0200, Jan Kara wrote: > > > On Fri 16-08-19 10:47:21, Vlastimil Babka wrote: > > > > On 8/15/19 3:35 PM, Jan Kara wrote: > > > > >> > > > > >> So when the GUP user uses MMU notifiers to stop writing to pages whenever > > > > >> they are writeprotected with page_mkclean(), they don't really need page > > > > >> pin - their access is then fully equivalent to any other mmap userspace > > > > >> access and filesystem knows how to deal with those. I forgot out this case > > > > >> when I wrote the above sentence. > > > > >> > > > > >> So to sum up there are three cases: > > > > >> 1) DIO case - GUP references to pages serving as DIO buffers are needed for > > > > >> relatively short time, no special synchronization with page_mkclean() or > > > > >> munmap() => needs FOLL_PIN > > > > >> 2) RDMA case - GUP references to pages serving as DMA buffers needed for a > > > > >> long time, no special synchronization with page_mkclean() or munmap() > > > > >> => needs FOLL_PIN | FOLL_LONGTERM > > > > >> This case has also a special case when the pages are actually DAX. Then > > > > >> the caller additionally needs file lease and additional file_pin > > > > >> structure is used for tracking this usage. > > > > >> 3) ODP case - GUP references to pages serving as DMA buffers, MMU notifiers > > > > >> used to synchronize with page_mkclean() and munmap() => normal page > > > > >> references are fine. > > > > > > > > IMHO the munlock lesson told us about another one, that's in the end equivalent > > > > to 3) > > > > > > > > 4) pinning for struct page manipulation only => normal page references > > > > are fine > > > > > > Right, it's good to have this for clarity. > > > > > > > > I want to add that I'd like to convert users in cases 1) and 2) from using > > > > > GUP to using differently named function. Users in case 3) can stay as they > > > > > are for now although ultimately I'd like to denote such use cases in a > > > > > special way as well... > > > > > > > > So after 1/2/3 is renamed/specially denoted, only 4) keeps the current > > > > interface? > > > > > > Well, munlock() code doesn't even use GUP, just follow_page(). I'd wait to > > > see what's left after handling cases 1), 2), and 3) to decide about the > > > interface for the remainder. > > > > > > > For 3 we do not need to take a reference at all :) So just forget about 3 > > it does not exist. For 3 the reference is the reference the CPU page table > > has on the page and that's it. GUP is no longer involve in ODP or anything > > like that. > > Yes, I understand. But the fact is that GUP calls are currently still there > e.g. in ODP code. If you can make the code work without taking a page > reference at all, I'm only happy :) Already in rdma next AFAIK so in 5.4 it will be gone :) i have been removing all GUP users that do not need reference. Intel i915 driver is a left over i will work some more with them to get rid of it too. Cheers, Jérôme