Re: [PATCH 1/2] mm/gup: introduce pin_user_pages_locked()

[Souptick Joarder]

On Sun, May 31, 2020 at 12:34 PM Souptick Joarder <jrdr.linux@xxxxxxxxx> wrote:
>
> On Thu, May 28, 2020 at 4:02 AM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
> >
> > Introduce pin_user_pages_locked(), which is nearly identical to
> > get_user_pages_locked() except that it sets FOLL_PIN and rejects
> > FOLL_GET.

Forget to ask, is it fine to add this new pin_user_pages_locked() in
Documentation/core-api/pin_user_pages.rst ?

> >
> > Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> > ---
> >  include/linux/mm.h |  2 ++
> >  mm/gup.c           | 30 ++++++++++++++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 98be7289d7e9..d16951087c93 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1700,6 +1700,8 @@ long pin_user_pages(unsigned long start, unsigned long nr_pages,
> >                     struct vm_area_struct **vmas);
> >  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> >                     unsigned int gup_flags, struct page **pages, int *locked);
> > +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > +                   unsigned int gup_flags, struct page **pages, int *locked);
> >  long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> >                     struct page **pages, unsigned int gup_flags);
> >  long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 2f0a0b497c23..17418a949067 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -2992,3 +2992,33 @@ long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> >         return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
> >  }
> >  EXPORT_SYMBOL(pin_user_pages_unlocked);
> > +
> > +/*
> > + * pin_user_pages_locked() is the FOLL_PIN variant of get_user_pages_locked().
> > + * Behavior is the same, except that this one sets FOLL_PIN and rejects
> > + * FOLL_GET.
> > + */
> > +long pin_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > +                          unsigned int gup_flags, struct page **pages,
> > +                          int *locked)
> > +{
> > +       /*
> > +        * FIXME: Current FOLL_LONGTERM behavior is incompatible with
> > +        * FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> > +        * vmas.  As there are no users of this flag in this call we simply
> > +        * disallow this option for now.
> > +        */
> > +       if (WARN_ON_ONCE(gup_flags & FOLL_LONGTERM))
> > +               return -EINVAL;
> > +
> > +       /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> > +       if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> > +               return -EINVAL;
> > +
> > +       gup_flags |= FOLL_PIN;
>
> Right now get_user_pages_locked() doesn't have similar check for FOLL_PIN
> and also not setting FOLL_GET internally irrespective of gup_flags
> passed by user.
> Do we need to add the same in get_user_pages_locked() ?
>
> > +       return __get_user_pages_locked(current, current->mm, start, nr_pages,
> > +                                      pages, NULL, locked,
> > +                                      gup_flags | FOLL_TOUCH);
> > +}
> > +EXPORT_SYMBOL(pin_user_pages_locked);
> > +
> > --
> > 2.26.2
> >
> >