Re: [PATCH 1/8] mm/gup: have internal functions get the mmap_read_lock()

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Jan 19, 2023 at 01:19:06PM -0800, John Hubbard wrote:
> > diff --git a/mm/gup.c b/mm/gup.c
> > index f45a3a5be53a48..3a9f764165f50b 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -1343,13 +1343,22 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm,
> >   						unsigned int flags)
> >   {
> >   	long ret, pages_done;
> > -	bool lock_dropped;
> > +	bool lock_dropped = false;
> 
> A small thing, but the naming and the associated comments later are now
> inaccurate. Let's add function-level documentation to explain the new
> calling convention for __get_user_pages_locked(), change the name of
> lock_dropped to must_unlock, add a comment mid-function, and correct the
> comment at the end of the function. Like this:

Done

> > @@ -3180,14 +3183,13 @@ EXPORT_SYMBOL(pin_user_pages);
> >   long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> >   			     struct page **pages, unsigned int gup_flags)
> >   {
> > -	/* FOLL_GET and FOLL_PIN are mutually exclusive. */
> > -	if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> > -		return -EINVAL;
> 
> Why are we dropping the assertion check for FOLL_GET?

Hmm, I think this is a rebase error, it should be in the "consolidate
invariants" patch

Thanks,
Jason