Re: [PATCH v8 07/14] mm: shmem: allow THP support determination at folio allocation time

[Luiz Capitulino]

On 10/2/26 3:23 PM, David Hildenbrand (Arm) wrote:
> On 9/18/26 03:45, Luiz Capitulino wrote:
> > In order to enable THP support in shmem today, besides the user
> > configuration required, the CPU must support PMD-sized pages. This
> > is the case because of the following has_transparent_hugepage()
> > usage:
> >
> > - shmem_parse_one() and shmem_parse_huge(): Check if THP is built-in and
> >    if the CPU supports PMD-sized pages
> >
> > - shmem_init(): Since the CONFIG_TRANSPARENT_HUGEPAGE guard is outside
> >    the code block calling has_transparent_hugepage(), the
> >    has_transparent_hugepage() call is exclusively checking if the CPU
> >    supports PMD-sized pages
> >
> > While it's necessary to check if CONFIG_TRANSPARENT_HUGEPAGE is enabled
> > in all cases, shmem can determine THP size support at folio allocation
> > time. Therefore, drop the has_transparent_hugepage() usage listed above
> > while keeping the CONFIG_TRANSPARENT_HUGEPAGE checks.
> >
> > Additionally, we need to check if PMD size order is supported in
> > shmem_getattr(). Use pgtable_has_pmd_leaves() for that.
> >
> > Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Luiz Capitulino <luizcap@xxxxxxxxxx>
> > ---
> >   mm/shmem.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index 776dff8a848e..930657d05375 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -690,7 +690,7 @@ static int shmem_parse_huge(const char *str)
> >   	else
> >   		return -EINVAL;
> >   
> > -	if (!has_transparent_hugepage() &&
> > +	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> >   	    huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
> >   		return -EINVAL;
> >   
> > @@ -1524,6 +1524,8 @@ static int shmem_getattr(struct mnt_idmap *idmap,
> >   	generic_fillattr(idmap, request_mask, inode, stat);
> >   
> >   	orders = shmem_huge_global_enabled(inode, 0, 0, false, NULL, 0);
>
> I'm curious: why is that not handled inside shmem_huge_global_enabled() ? If PMD
> order is impossible (well, okay, it is possible, but we simply cannot map these
> things through PMDs), I would expect that we never list them as "enabled".

Yes, you're right. What about renaming current shmem_huge_global_enabled() to
__shmem_huge_global_enabled() and then having:

static unsigned int shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
					      loff_t write_end, bool shmem_huge_force,
					      struct vm_area_struct *vma,
					      vm_flags_t vm_flags)
{
	unsigned int orders;

	orders = __shmem_huge_global_enabled(inode, index, write_end,
					     shmem_huge_force, vma, vm_flags);
	if (!pgtable_has_pmd_leaves())
		orders &= ~BIT(PMD_ORDER);

	return orders;
}

Would this be acceptable?

> (while shmem could allocate PMD folios, it would have to map them always through
> PTEs. I think this would be possible through some changes, but we can defer that
> to some future work).