Re: [git pull] vfs.git get_user_pages_fast() conversion

[Al Viro]

On Fri, Nov 17, 2017 at 09:32:15PM +0000, Al Viro wrote:

> And for get_user_pages() itself it's even more ridiculous - vmalist (the last
> argument) is non-NULL in only one caller.  Which uses it only to check if all
> of the VMAs happen to be hugetlb ones, apparently.
> 
> FWIW, I wanted to trim the users of those two suckers and see what remains.
> And then go through those with maintainers of subsystems in question, to
> see what is really wanted there.  That's for the coming cycle, though...

	Incidentally, why the hell do we need that notify_drop argument of
__get_user_pages_locked()?  Its only use is (and has always been)
        if (notify_drop && lock_dropped && *locked) {
                /*
                 * We must let the caller know we temporarily dropped the lock
                 * and so the critical section protected by it was lost.
                 */
                up_read(&mm->mmap_sem);
                *locked = 0;
        }
in the very end of __get_user_pages_locked().  There are 4 callers:
get_user_pages_locked() and get_user_pages_remote() pass true,
get_user_pages() and __get_user_pages_unlocked() - false.

get_user_pages() passes NULL for 'locked', so we _never_ get
to the body of that if() anyway - lock_dropped is only set if
locked != NULL.

And in __get_user_pages_unlocked() we have
        ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, NULL,
                                      &locked, false, gup_flags | FOLL_TOUCH);
        if (locked)
                up_read(&mm->mmap_sem);

Suppose we passed true instead of false here.  If that if (notify_drop...)
still has not triggered, nothing has changed.  If it *has* triggered, we had
*locked (i.e. locked from the __get_user_pages_unlocked() stack frame)
non-zero, so we'd just have dropped ->mmap_sem just before returning to
__get_user_pages_unlocked() instead of doing that just after.  And set
*locked to zero, so that __get_user_pages_unlocked() won't end up dropping
it.

	Looks like we can bloody well get rid of that argument and do just
        if (lock_dropped && *locked) {
                /*
                 * We must let the caller know we temporarily dropped the lock
                 * and so the critical section protected by it was lost.
                 */
                up_read(&mm->mmap_sem);
                *locked = 0;
        }
in __get_user_pages_locked(), or am I missing something subtle there?  Andrea?