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

David Hildenbrand (Arm)
2026-03-23T17:55:51Z

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.

Something to ponder about :)

--
Cheers,

David