Re: [PATCH RFC v2 2/6] KVM: guest_memfd: Directly allocate folios with filemap_alloc_folio()

[Sean Christopherson <seanjc@google.com>]

On Mon, Mar 02, 2026, Vlastimil Babka wrote:
> On 2/25/26 08:20, Ackerley Tng wrote:
> > __filemap_get_folio_mpol() is parametrized by a bunch of GFP flags, which
> 
>                                                            FGP?
> 
> > adds complexity for the reader. Since guest_memfd doesn't meaningfully use
> > any of the other FGP flags, undo that complexity by directly calling
> > filemap_alloc_folio().
> > 
> > Directly calling filemap_alloc_folio() also allows the order of 0 to be
> > explicitly specified, which is the only order guest_memfd supports. This is
> > easier to understand,

That's debatable.  IMO, one isn't clearly better than the other, especially since
filemap_lock_folio() is itself a wrapper for __filemap_get_folio_mpol().  And there
is a cost to open-coding, as it means we risk missing something if there's a change
in __filemap_get_folio_mpol() that's beneficial to guest_memfd.

As Vlastimil said, if this greatly simplifies accounting, then I'm ok with it.
But the changelog needs to focus on that aspect, because I don't see this as a
clear win versus using __filemap_get_folio_mpol().

And if we go through with this, we should probably revert 16a542e22339 ("mm/filemap:
Extend __filemap_get_folio() to support NUMA memory policies"), because guest_memfd
is/was the only user.

> > +static struct folio *__kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> > +{
> > +	/* TODO: Support huge pages. */
> > +	struct mempolicy *policy;
> > +	struct folio *folio;
> > +	gfp_t gfp;
> > +	int ret;
> > +
> > +	/*
> > +	 * Fast-path: See if folio is already present in mapping to avoid
> > +	 * policy_lookup.
> > +	 */
> > +	folio = filemap_lock_folio(inode->i_mapping, index);
> > +	if (!IS_ERR(folio))
> > +		return folio;
> > +
> > +	gfp = mapping_gfp_mask(inode->i_mapping);
> > +
> > +	policy = mpol_shared_policy_lookup(&GMEM_I(inode)->policy, index);

This is a potential performance regression.  Previously, KVM would do a policy
lookup once per retry loop.  Now KVM will do the lookup 

I doubt it will matter in practice, because on EEXIST filemap_lock_folio() should
be all but guaranteed to find the existing folio.  But it's also something that
should be easy enough to avoid, and it's also another argument for using
__filemap_get_folio_mpol() instead of open coding our own version.