On Tue, Jan 24, 2023 at 06:11:36PM -0800, John Hubbard wrote: > On 1/24/23 12:34, Jason Gunthorpe wrote: > ... > > @@ -3183,11 +3198,13 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages, > > /* FOLL_GET and FOLL_PIN are mutually exclusive. */ > > if (WARN_ON_ONCE(gup_flags & FOLL_GET)) > > return -EINVAL; > > + int locked = 0; > > if (WARN_ON_ONCE(!pages)) > > return -EINVAL; > > - gup_flags |= FOLL_PIN; > > - return get_user_pages_unlocked(start, nr_pages, pages, gup_flags); > > + gup_flags |= FOLL_PIN | FOLL_TOUCH; > > I missed this on my review of v1 of this series: the FOLL_TOUCH change > looks like a mistake, yes? It should just be left as-is: > > gup_flags |= FOLL_PIN; > No, not a mistake. The transformation is this: - return get_user_pages_unlocked(start, nr_pages, pages, gup_flags); + gup_flags |= FOLL_PIN | FOLL_TOUCH; + return __gup_longterm_locked(current->mm, start, nr_pages, pages, NULL, + &locked, gup_flags); Ie we switch get_user_pages_unlocked() for __gup_longterm_locked() Howevr get_user_pages_unlocked() was adding the FOLL_TOUCH before calling __gup_longterm_locked(): - ret = __gup_longterm_locked(mm, start, nr_pages, pages, NULL, &locked, - gup_flags | FOLL_TOUCH); Thus we have to lift the FOLL_TOUCH when we do the function call replacement. Jason
