From: Jason Gunthorpe <jgg@nvidia.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: Alistair Popple <apopple@nvidia.com>, linux-mm@kvack.org
Subject: Re: [PATCH 6/8] mm/gup: make locked never NULL in the internal GUP functions
Date: Fri, 20 Jan 2023 20:21:10 -0400 [thread overview]
Message-ID: <Y8sv9okbfrm7sJJz@nvidia.com> (raw)
In-Reply-To: <4f052c76-6e29-24d7-3d7a-79d7aad69364@nvidia.com>
On Fri, Jan 20, 2023 at 11:19:58AM -0800, John Hubbard wrote:
> On 1/17/23 07:58, Jason Gunthorpe wrote:
> > Now that NULL locked doesn't have a special meaning we can just make it
> > non-NULL in all cases and remove the special tests.
> >
> > get_user_pages() and pin_user_pages() can safely pass in a locked = 1
> >
> > get_user_pages_remote) and pin_user_pages_remote() can swap in a local
> > variable for locked if NULL is passed.
> >
> > Remove all the NULL checks.
> >
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> > mm/gup.c | 30 ++++++++++++++++++++----------
> > 1 file changed, 20 insertions(+), 10 deletions(-)
> >
> ...
>
> Looks correct, just a few remaining comments that could be
> removed or fixed up:
>
> > @@ -1121,7 +1121,7 @@ static long __get_user_pages(struct mm_struct *mm,
> > i = follow_hugetlb_page(mm, vma, pages, vmas,
> > &start, &nr_pages, i,
> > gup_flags, locked);
> > - if (locked && *locked == 0) {
> > + if (!*locked) {
>
>
> Just above this function, there is a comment that can
> be adjusted to remove the reference to possibly NULL locked
> arg. This one:
>
> * If @locked != NULL, *@locked will be set to 0 when mmap_lock is
> * released by an up_read(). That can happen if @gup_flags does not
> * have FOLL_NOWAIT.
OK
> There's another one above populate_vma_page_range():
>
> * If @locked is NULL, it may be held for read or write and will
> * be unperturbed.
> *
> * If @locked is non-NULL, it must held for read only and may be
> * released. If it's released, *@locked will be set to 0.
>
> ...and another set, above faultin_vma_page_range():
>
> * If @locked is NULL, it may be held for read or write and will be unperturbed.
> *
> * If @locked is non-NULL, it must held for read only and may be released. If
> * it's released, *@locked will be set to 0.
ahh, I missed these two functions, they need to set FOLL_UNLOCK and
de-null locked too. I didn't check things that were calling
__get_user_pages() :\
Thanks,
Jason
next prev parent reply other threads:[~2023-01-21 0:21 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-17 15:58 [PATCH 0/8] Simplify the external interface for GUP Jason Gunthorpe
2023-01-17 15:58 ` [PATCH 1/8] mm/gup: have internal functions get the mmap_read_lock() Jason Gunthorpe
2023-01-19 11:16 ` Mike Rapoport
2023-01-19 21:19 ` John Hubbard
2023-01-19 22:40 ` John Hubbard
2023-01-20 14:47 ` Jason Gunthorpe
2023-01-17 15:58 ` [PATCH 2/8] mm/gup: don't call __gup_longterm_locked() if FOLL_LONGTERM cannot be set Jason Gunthorpe
2023-01-19 22:24 ` John Hubbard
2023-01-17 15:58 ` [PATCH 3/8] mm/gup: simplify the external interface functions and consolidate invariants Jason Gunthorpe
2023-01-19 11:17 ` Mike Rapoport
2023-01-20 2:51 ` John Hubbard
2023-01-20 14:58 ` Jason Gunthorpe
2023-01-20 18:45 ` John Hubbard
2023-01-17 15:58 ` [PATCH 4/8] mm/gup: add an assertion that the mmap lock is locked Jason Gunthorpe
2023-01-20 3:08 ` John Hubbard
2023-01-20 15:44 ` Jason Gunthorpe
2023-01-17 15:58 ` [PATCH 5/8] mm/gup: add FOLL_UNLOCK Jason Gunthorpe
2023-01-19 11:18 ` Mike Rapoport
2023-01-20 13:45 ` Jason Gunthorpe
2023-01-20 14:58 ` Mike Rapoport
2023-01-20 19:02 ` John Hubbard
2023-01-23 11:37 ` David Hildenbrand
2023-01-23 17:58 ` Jason Gunthorpe
2023-01-23 22:22 ` John Hubbard
2023-01-24 13:08 ` David Hildenbrand
2023-01-17 15:58 ` [PATCH 6/8] mm/gup: make locked never NULL in the internal GUP functions Jason Gunthorpe
2023-01-20 19:19 ` John Hubbard
2023-01-21 0:21 ` Jason Gunthorpe [this message]
2023-01-23 11:35 ` David Hildenbrand
2023-01-23 12:44 ` Jason Gunthorpe
2023-01-23 12:59 ` David Hildenbrand
2023-01-23 18:07 ` Jason Gunthorpe
2023-01-17 15:58 ` [PATCH 7/8] mm/gup: remove pin_user_pages_fast_only() Jason Gunthorpe
2023-01-20 19:23 ` John Hubbard
2023-01-23 11:31 ` David Hildenbrand
2023-01-17 15:58 ` [PATCH 8/8] mm/gup: make get_user_pages_fast_only() return the common return value Jason Gunthorpe
2023-01-20 19:27 ` John Hubbard
2023-01-23 11:33 ` David Hildenbrand
2023-01-19 11:18 ` [PATCH 0/8] Simplify the external interface for GUP Mike Rapoport
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=Y8sv9okbfrm7sJJz@nvidia.com \
--to=jgg@nvidia.com \
--cc=apopple@nvidia.com \
--cc=jhubbard@nvidia.com \
--cc=linux-mm@kvack.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;
as well as URLs for NNTP newsgroup(s).