Re: [PATCH] mm/gup: fix GUP-fast fallback for NULL-mapping order-0 folios

[Alistair Popple]

On 2026-05-15 at 10:09 +1000, Balbir Singh <balbirs@xxxxxxxxxx> wrote...
> On 4/9/26 17:52, David Hildenbrand (Arm) wrote:
> > On 4/9/26 03:46, John Hubbard wrote:
> >> Since commit f002882ca369 ("mm: merge folio_is_secretmem() and
> >> folio_fast_pin_allowed() into gup_fast_folio_allowed()"),
> >> gup_fast_folio_allowed() falls back to the slow path for any order-0
> >> folio with a NULL mapping when CONFIG_SECRETMEM=y. This causes a
> >> performance regression for drivers that allocate pages with alloc_page()
> >> and insert them into VMAs via vm_insert_page(). These pages legitimately
> >> have a NULL folio->mapping, but they cannot be secretmem pages.
> >>
> >> Secretmem pages are always added to the secretmem inode's page cache via
> >> filemap_add_folio(), which sets folio->mapping to the inode's i_mapping.
> >> A folio with a NULL mapping can never be a secretmem folio. The
> >> NULL-mapping check was intended to handle truncated file-backed pages (a
> >> reject_file_backed concern), not secretmem detection.
> >>
> >> When only check_secretmem is true (and reject_file_backed is false), a
> >> NULL mapping is sufficient to prove the folio is not secretmem, so the
> >> fast path can proceed.
> > 
> > Hm, what if secretmem folio just got truncated? I hate to rely on some
> > handling in the caller to detect truncation differently during GUP-fast,
> > but this function returning "true".
> > 
> 
> Can secretmem folios be truncated? I assume you are referring to
> ftruncate(), I am looking at the setattr implementation of secretmem
> and it does not seem like it can be truncated.
> 
> > Zi is working on a way to distinguish folios from non-folio things: that
> > we can identify whatever was added through vm_insert_page().
> > 
> > Because that's really the key problem here: vm_insert_page() pages are
> > not actually folios, they just look like a folio today, but looking at
> > fields like ->mapping does not make any sense.
> > 
> 
> I still think this is a short term fix worth having until we get
> Zi's fixes

Agreed - this clearly results in a performance regression and we have customers
reporting it as such. I think Zi might be out of the office atm but when I last
spoke to him he agreed this fix should go in for now and his approach will come
later.

 - Alistair

> >>
> >> Tested-by: Sourab Gupta <sougupta@xxxxxxxxxx>
> >> Fixes: f002882ca369 ("mm: merge folio_is_secretmem() and folio_fast_pin_allowed() into gup_fast_folio_allowed()")
> >> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> >> ---
> >>  mm/gup.c | 13 +++++++++----
> >>  1 file changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/mm/gup.c b/mm/gup.c
> >> index 8e7dc2c6ee73..3ea661e67eea 100644
> >> --- a/mm/gup.c
> >> +++ b/mm/gup.c
> >> @@ -2784,12 +2784,17 @@ static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
> >>  	mapping = READ_ONCE(folio->mapping);
> >>  
> >>  	/*
> >> -	 * The mapping may have been truncated, in any case we cannot determine
> >> -	 * if this mapping is safe - fall back to slow path to determine how to
> >> -	 * proceed.
> >> +	 * If the mapping is NULL (truncated, or never set), we cannot
> >> +	 * determine whether the folio is file-backed, so a long-term writable
> >> +	 * pin must fall back to the slow path.
> >> +	 *
> >> +	 * Otherwise, a NULL mapping proves this is not a secretmem folio
> >> +	 * (secretmem folios always have a valid mapping to the secretmem
> >> +	 * inode's address_space), so in that case, we can continue with the
> >> +	 * fast path.
> >>  	 */
> >>  	if (!mapping)
> >> -		return false;
> >> +		return !reject_file_backed;
> >>  
> >>  	/* Anonymous folios pose no problem. */
> >>  	mapping_flags = (unsigned long)mapping & FOLIO_MAPPING_FLAGS;
> >>
> >> base-commit: 7f87a5ea75f011d2c9bc8ac0167e5e2d1adb1594
> > 
> > 
> 
>