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

Nikita Kalyazin
2026-04-10T15:27:45Z

On 23/03/2026 20:22, Ackerley Tng wrote:
> "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.

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

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