From: John Hubbard <jhubbard@nvidia.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Alistair Popple <apopple@nvidia.com>,
David Hildenbrand <david@redhat.com>,
David Howells <dhowells@redhat.com>,
Christoph Hellwig <hch@infradead.org>, <linux-mm@kvack.org>,
"Mike Rapoport (IBM)" <rppt@kernel.org>
Subject: Re: [PATCH v2 01/13] mm/gup: have internal functions get the mmap_read_lock()
Date: Tue, 24 Jan 2023 18:52:43 -0800 [thread overview]
Message-ID: <fedfe1fe-7a1d-c97e-e195-f83be3edf676@nvidia.com> (raw)
In-Reply-To: <53ac3700-6532-a609-ceff-d0840469e532@nvidia.com>
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
next prev parent reply other threads:[~2023-01-25 2:52 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-24 20:34 [PATCH v2 00/13] Simplify the external interface for GUP Jason Gunthorpe
2023-01-24 20:34 ` [PATCH v2 01/13] mm/gup: have internal functions get the mmap_read_lock() Jason Gunthorpe
2023-01-25 2:11 ` John Hubbard
2023-01-25 2:52 ` John Hubbard [this message]
2023-01-25 16:38 ` Jason Gunthorpe
2023-01-25 18:48 ` John Hubbard
2023-01-24 20:34 ` [PATCH v2 02/13] mm/gup: remove obsolete FOLL_LONGTERM comment Jason Gunthorpe
2023-01-25 2:13 ` John Hubbard
2023-02-08 14:25 ` David Hildenbrand
2023-01-24 20:34 ` [PATCH v2 03/13] mm/gup: don't call __gup_longterm_locked() if FOLL_LONGTERM cannot be set Jason Gunthorpe
2023-02-08 14:26 ` David Hildenbrand
2023-01-24 20:34 ` [PATCH v2 04/13] mm/gup: move try_grab_page() to mm/internal.h Jason Gunthorpe
2023-01-25 2:15 ` John Hubbard
2023-02-08 14:26 ` David Hildenbrand
2023-01-24 20:34 ` [PATCH v2 05/13] mm/gup: simplify the external interface functions and consolidate invariants Jason Gunthorpe
2023-01-25 2:30 ` John Hubbard
2023-01-24 20:34 ` [PATCH v2 06/13] mm/gup: add an assertion that the mmap lock is locked Jason Gunthorpe
2023-01-25 2:34 ` John Hubbard
2023-01-24 20:34 ` [PATCH v2 07/13] mm/gup: remove locked being NULL from faultin_vma_page_range() Jason Gunthorpe
2023-01-25 2:38 ` John Hubbard
2023-01-24 20:34 ` [PATCH v2 08/13] mm/gup: add FOLL_UNLOCKABLE Jason Gunthorpe
2023-01-24 20:34 ` [PATCH v2 09/13] mm/gup: make locked never NULL in the internal GUP functions Jason Gunthorpe
2023-01-25 3:00 ` John Hubbard
2023-01-24 20:34 ` [PATCH v2 10/13] mm/gup: remove pin_user_pages_fast_only() Jason Gunthorpe
2023-01-24 20:34 ` [PATCH v2 11/13] mm/gup: make get_user_pages_fast_only() return the common return value Jason Gunthorpe
2023-01-24 20:34 ` [PATCH v2 12/13] mm/gup: move gup_must_unshare() to mm/internal.h Jason Gunthorpe
2023-01-25 2:41 ` John Hubbard
2023-01-26 11:29 ` David Hildenbrand
2023-01-24 20:34 ` [PATCH v2 13/13] mm/gup: move private gup FOLL_ flags to internal.h Jason Gunthorpe
2023-01-25 2:44 ` John Hubbard
2023-01-26 12:48 ` David Hildenbrand
2023-01-26 12:55 ` Jason Gunthorpe
2023-01-26 13:06 ` David Hildenbrand
2023-01-26 14:41 ` Claudio Imbrenda
2023-01-26 14:46 ` David Hildenbrand
2023-01-26 15:05 ` Jason Gunthorpe
2023-01-26 15:39 ` Claudio Imbrenda
2023-01-26 16:35 ` Jason Gunthorpe
2023-01-26 17:24 ` Claudio Imbrenda
2023-01-30 18:21 ` Claudio Imbrenda
2023-01-30 18:24 ` Jason Gunthorpe
2023-02-07 11:31 ` Claudio Imbrenda
2023-02-07 12:40 ` Jason Gunthorpe
2023-02-06 23:46 ` [PATCH v2 00/13] Simplify the external interface for GUP Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fedfe1fe-7a1d-c97e-e195-f83be3edf676@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=apopple@nvidia.com \
--cc=david@redhat.com \
--cc=dhowells@redhat.com \
--cc=hch@infradead.org \
--cc=jgg@nvidia.com \
--cc=linux-mm@kvack.org \
--cc=rppt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox