Re: [PATCH v11 05/16] mm/gup: drop local variable in gup_fast_folio_allowed

[Nikita Kalyazin]

On 23/03/2026 20:22, Ackerley Tng wrote:
> "David Hildenbrand (Arm)" <david@xxxxxxxxxx> writes:
>
> > On 3/17/26 15:11, 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 | 23 ++++++++++++-----------
> > >   1 file changed, 12 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/mm/gup.c b/mm/gup.c
> > > index 5856d35be385..869d79c8daa4 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,18 @@ 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 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);
> >
> > Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> >
> > I'm wondering if it would be a good idea to check for a hugetlb mapping
> > here instead of having the folio_test_hugetlb() check above.
>
> I think it's nice that hugetlb folios are determined immediately to be
> eligible for GUP-fast regardless of whether the folio is file-backed or
> not.

Ok, I'll leave it as is for now.

> > Something to ponder about :)
> >
> > --
> > Cheers,
> >
> > David