From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-170.mta1.migadu.com (out-170.mta1.migadu.com [95.215.58.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6233E36F42B for ; Sun, 17 May 2026 13:32:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779024781; cv=none; b=JAqnQXvy2IAd2I6cOz7MzSh0emE4Qy0GoJ5GfM1nSIMweWIkXzNBTGj8A9qN9lr262jbt3XaTD3s562vHP101c2kfOjVYiWush6ZBOSfTzF6tx+HoLnHoynXL2KO7Hzd/ubu6hXg4pfoz9D/9HyyKnOBNQfE+v17GYAlsl4X9io= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779024781; c=relaxed/simple; bh=V/nQV7c0yUOtOeRt5LlD64XEABWGJH5kdQWTO3gS6JU=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=GAMrbhPcpoEYE2hnSRAN9dWuSSPNGER0zgeYRRpe5gZ3kHTlIHUdFV9VY2/rSjBTiEr1gg/QanHU8lWRZQizkpm6M07yoHd0zhAmstLrL+zFuBNdFYY7o5XtfNECC+hxS2XeV2GIuLJ6YIkEvDstyIzVWVEL8Sg49O2etnP+dPU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=nnQEAqW/; arc=none smtp.client-ip=95.215.58.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="nnQEAqW/" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1779024776; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3TFOcCLVTrzSGMf6OMRn7d9qROQQE6qY9dQm9d0YqOA=; b=nnQEAqW/Lvrruxkg/klvjNyMzNTc5cQnk+U2V6B02n+gcyM9hGIyvLX4f4VcrBQeyA3+8m 8WZZ/MsVvqs/2+FbZkzR2MtX9+FEAQ2gNkLivhfYc2v4WKZt5SmKmlv6qji40NBh28cRP9 QM85UwqXIQIWdbL/DUZaQ9/s+Ujf/bQ= From: Lance Yang To: luizcap@redhat.com, baolin.wang@linux.alibaba.com Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, david@kernel.org, ziy@nvidia.com, lance.yang@linux.dev, corbet@lwn.net, tsbogend@alpha.franken.de, maddy@linux.ibm.com, mpe@ellerman.id.au, agordeev@linux.ibm.com, gerald.schaefer@linux.ibm.com, hca@linux.ibm.com, gor@linux.ibm.com, x86@kernel.org, dave.hansen@linux.intel.com, djbw@kernel.org, vishal.l.verma@intel.com, dave.jiang@intel.com, akpm@linux-foundation.org, lorenzo.stoakes@oracle.com Subject: Re: (sashiko review) Re: [PATCH v4 6/9] mm: shmem: drop has_transparent_hugepage() usage Date: Sun, 17 May 2026 21:32:39 +0800 Message-Id: <20260517133239.26416-1-lance.yang@linux.dev> In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT On Wed, May 06, 2026 at 02:12:41PM -0400, Luiz Capitulino wrote: >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 >> Reviewed-by: Lance Yang >> Acked-by: Zi Yan >> Signed-off-by: Luiz Capitulino >> --- >> 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. Well spotted. @Baolin looks like shmem_getattr() might already be buggy? shmem_huge_global_enabled() returns an order mask. For huge=always and huge=within_size it can return THP_ORDERS_ALL_FILE_DEFAULT, which is not PMD-only and can include smaller file mTHP orders as well ... So shmem_getattr() treating any non-zero mask as HPAGE_PMD_SIZE looks like an over-report? Cheers, Lance