Re: [PATCH 6/8] mm/gup: make locked never NULL in the internal GUP functions

[Jason Gunthorpe <jgg@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