Re: [PATCH v11 04/16] mm/gup: drop secretmem optimization from gup_fast_folio_allowed

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

On 23/03/2026 18:31, David Hildenbrand (Arm) wrote:
> On 3/17/26 15:11, Kalyazin, Nikita wrote:
>> From: Patrick Roy <patrick.roy@linux.dev>
>>
>> This drops an optimization in gup_fast_folio_allowed() where
>> secretmem_mapping() was only called if CONFIG_SECRETMEM=y. secretmem is
>> enabled by default since commit b758fe6df50d ("mm/secretmem: make it on
>> by default"), so the secretmem check did not actually end up elided in
>> most cases anymore anyway.
>>
>> This is in preparation of the generalization of handling mappings where
>> direct map entries of folios are set to not present. Currently,
>> mappings that match this description are secretmem mappings
>> (memfd_secret()). Later, some guest_memfd configurations will also fall
>> into this category.
>>
>> Signed-off-by: Patrick Roy <patrick.roy@linux.dev>
>> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
>> Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
>> ---
>> mm/gup.c | 11 +----------
>> 1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/mm/gup.c b/mm/gup.c
>> index 8e7dc2c6ee73..5856d35be385 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -2739,7 +2739,6 @@ static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
>> {
>> bool reject_file_backed = false;
>> struct address_space *mapping;
>> - bool check_secretmem = false;
>> unsigned long mapping_flags;
>>
>> /*
>> @@ -2751,14 +2750,6 @@ static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
>> reject_file_backed = true;
>>
>> /* We hold a folio reference, so we can safely access folio fields. */
>> -
>> - /* secretmem folios are always order-0 folios. */
>> - if (IS_ENABLED(CONFIG_SECRETMEM) && !folio_test_large(folio))
>> - check_secretmem = true;
>> -
>> - if (!reject_file_backed && !check_secretmem)
>> - return true;
>> -
>
> The AI review says that this will force all small folios through the
> mapping check (which we obviously need later :) ).
>
> It brings up two cases where page->mapping is not set up:
>
> 1) ZONE_DEVICE pages (like Device DAX and PCI P2PDMA)
>
> 2) large shmem folios in the swap cache
>
>
> 2) doesn't make sense, because the folio cannot be mapped in user space
> when that happens.
>
> I am also skeptical about 1), especially as large folios are also
> supported for device dax and would be problematic here.
> __dev_dax_pte_fault() clearly sets folio->mapping through dax_set_mapping().
>
>
> If 1) is ever a case we could allow them by checking for
> folio_is_zone_device(). But I am not sure if that is really required.
> Sounds weird.

I went ahead and added a check for folio_is_zone_device().

>
>
> --
> Cheers,
>
> David