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

Nikita Kalyazin
2026-03-10T00:00:46Z

On 05/03/2026 19:07, David Hildenbrand (Arm) wrote:
> On 1/26/26 17:47, 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 | 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