Re: [PATCH v2 01/13] mm/gup: have internal functions get the mmap_read_lock()

[Jason Gunthorpe <jgg@nvidia.com>]

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