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

Ackerley Tng
2026-03-23T20:22:45Z

"David Hildenbrand (Arm)" <david@kernel.org> writes:

> On 3/17/26 15:11, Kalyazin, Nikita wrote:
>> From: Nikita Kalyazin <kalyazin@amazon.com>
>>
>> Move the check for pinning closer to where the result is used.
>> No functional changes.
>>
>> Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
>> ---
>> 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@kernel.org>
>
> 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.

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