On 1/24/23 18:11, 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;
...and I think the same thing happened in internal_get_user_pages_fast().
If you think that FOLL_TOUCH should be added, then it should really be a
separate patch, because it's unrelated to what this patch here is doing.
Looking through the code, I'm on the fence about that. On one hand,
there is an argument that pages that are going to be pinned should get
FOLL_TOUCH. But should pin_user_pages() do that for the caller? I'm
not sure.
thanks,
--
John Hubbard
NVIDIA
