Re: [PATCH v10 04/15] mm/gup: drop local variable in gup_fast_folio_allowed

[Nikita Kalyazin]

On 05/03/2026 19:07, David Hildenbrand (Arm) wrote:
> On 1/26/26 17:47, Kalyazin, Nikita wrote:
> > From: Nikita Kalyazin <kalyazin@xxxxxxxxxx>
> >
> > Move the check for pinning closer to where the result is used.
> > No functional changes.
> >
> > Signed-off-by: Nikita Kalyazin <kalyazin@xxxxxxxxxx>
> > ---
> >   mm/gup.c | 19 ++++++++-----------
> >   1 file changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 9cad53acbc99..e72dacce3e34 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -2737,18 +2737,9 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
> >    */
> >   static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
> >   {
> > -     bool reject_file_backed = false;
> >        struct address_space *mapping;
> >        unsigned long mapping_flags;
> >
> > -     /*
> > -      * If we aren't pinning then no problematic write can occur. A long term
> > -      * pin is the most egregious case so this is the one we disallow.
> > -      */
> > -     if ((flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)) ==
> > -         (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE))
> > -             reject_file_backed = true;
> > -
> >        /* We hold a folio reference, so we can safely access folio fields. */
> >        if (WARN_ON_ONCE(folio_test_slab(folio)))
> >                return false;
> > @@ -2793,8 +2784,14 @@ static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
> >         */
> >        if (secretmem_mapping(mapping))
> >                return false;
> > -     /* The only remaining allowed file system is shmem. */
> > -     return !reject_file_backed || shmem_mapping(mapping);
> > +
> > +     /*
> > +      * If we aren't pinning then no problematic write can occur. A long term
> > +      * pin is the most egregious case so this is the one we disallow.
> > +      * Also check the only remaining allowed file system - shmem.
> > +      */
> > +     return (flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)) !=
> > +         (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE) || shmem_mapping(mapping);
>
> Best to keep this split and a bit more readable.
>
> /*
>   * If we aren't pinning then no problematic write can occur. A writable
>   * long term pin is the most egregious case, so this is the one we
>   * allow only for ...
>   */
> if ((flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)) !=
>      (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)
>          return true;
> /* ... hugetlb (which we allowed above already) and shared memory. */
> return shmem_mapping(mapping);

Makes sense, thank you!

> >   }
> >
> >   static void __maybe_unused gup_fast_undo_dev_pagemap(int *nr, int nr_start,
>
>
> --
> Cheers,
>
> David