(sashiko review) Re: [PATCH v4 6/9] mm: shmem: drop has_transparent_hugepage() usage

[Luiz Capitulino <luizcap@redhat.com>]

On 2026-05-01 15:18, Luiz Capitulino wrote:
> Shmem uses has_transparent_hugepage() in the following ways:
> 
> - 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 mTHP size support at folio allocation
> time. Therefore, drop has_transparent_hugepage() usage while keeping the
> CONFIG_TRANSPARENT_HUGEPAGE checks.
> 
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Reviewed-by: Lance Yang <lance.yang@linux.dev>
> Acked-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: Luiz Capitulino <luizcap@redhat.com>
> ---
>   mm/shmem.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 3b5dc21b323c..1948d73fb1e3 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -689,7 +689,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;
>   
> @@ -4656,8 +4656,7 @@ static int shmem_parse_one(struct fs_context *fc, struct fs_parameter *param)
>   	case Opt_huge:
>   		ctx->huge = result.uint_32;
>   		if (ctx->huge != SHMEM_HUGE_NEVER &&
> -		    !(IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> -		      has_transparent_hugepage()))
> +		    !IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
>   			goto unsupported_parameter;
>   		ctx->seen |= SHMEM_SEEN_HUGE;
>   		break;

"""
By dropping the has_transparent_hugepage() check, will mount -t tmpfs
-o huge=always now succeed on hardware lacking PMD support?

If so, since hugepage_init() still sets the TRANSPARENT_HUGEPAGE_UNSUPPORTED
flag, thp_disabled_by_hw() will unconditionally block all large folio
allocations in shmem_allowable_huge_orders().

Does this create an intermediate state where the mount silently succeeds
but no huge pages of any size can actually be allocated?

I see this is resolved later in the series by commit cd27430097e8
("mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves()") and
commit 641a20ae032f ("mm: thp: always enable mTHP support").
"""

The mount -t tmpfs -o huge=always succeeding on hardware without PMD
support can happen in this patch, yes. But this seems very minor, the
impact seems to be someone doing bisection, landing on this patch and
their reproducer is depedent on mounting tmpfs with -o huge=always on
hardware without PMD size support? I can fix it if others feel strong
about this.

> @@ -5449,7 +5448,7 @@ void __init shmem_init(void)
>   #endif
>   
>   #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -	if (has_transparent_hugepage() && shmem_huge > SHMEM_HUGE_DENY)
> +	if (shmem_huge > SHMEM_HUGE_DENY)
>   		SHMEM_SB(shm_mnt->mnt_sb)->huge = shmem_huge;
>   	else
>   		shmem_huge = SHMEM_HUGE_NEVER; /* just in case it was patched */

"""
Also, by allowing shmem_huge to be set to SHMEM_HUGE_ALWAYS on systems
without PMD support, does this incorrectly affect shmem_getattr()?

shmem_getattr() relies on shmem_huge_global_enabled(), which only checks
the software configuration and not hardware PMD support. Consequently,
shmem_getattr() will erroneously report stat->blksize = HPAGE_PMD_SIZE
to userspace.

Since subsequent patches in the series do not appear to update
shmem_getattr(), could this misleading block size cause userspace tools
to over-allocate IO buffers on hardware where PMD-sized pages are
structurally impossible?
"""

This a real issue (albeit small one), the problem is this check in
shmem_getattr():

	if (shmem_huge_global_enabled(inode, 0, 0, false, NULL, 0))
		stat->blksize = HPAGE_PMD_SIZE;

So, we may report HPAGE_PMD_SIZE even when PMD size is not supported.
Looks like we may over-report today as well for the
SHMEM_HUGE_WITHIN_SIZE case? In any case, I'll fix this.