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.
