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@xxxxxxxxxx>
> > ---
> > 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
