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

[Nikita Kalyazin <kalyazin@amazon.com>]

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