linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lance Yang <lance.yang@linux.dev>
To: luizcap@redhat.com
Cc: lance.yang@linux.dev, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, david@kernel.org,
	baolin.wang@linux.alibaba.com, ryan.roberts@arm.com,
	akpm@linux-foundation.org, ljs@kernel.org, ziy@nvidia.com,
	Liam.Howlett@oracle.com, npache@redhat.com, dev.jain@arm.com,
	baohua@kernel.org
Subject: Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()
Date: Mon, 13 Apr 2026 23:45:14 +0800	[thread overview]
Message-ID: <20260413154514.94193-1-lance.yang@linux.dev> (raw)
In-Reply-To: <119dfe3d-4cfd-46e3-a539-8ac9af90f940@redhat.com>


On Mon, Apr 13, 2026 at 11:24:22AM -0400, Luiz Capitulino wrote:
>On 2026-04-10 04:19, Lance Yang wrote:
>> 
>> On Wed, Apr 08, 2026 at 04:22:57PM -0400, Luiz Capitulino wrote:
>>> Currently, we have two helpers that check for PMD-sized pages but have
>>> different names and slightly different semantics:
>>>
>>> - has_transparent_hugepage(): the name suggests it checks if THP is
>>>   enabled, but when CONFIG_TRANSPARENT_HUGEPAGE=y and the architecture
>>>   implements this helper, it actually checks if the CPU supports
>>>   PMD-sized pages
>>>
>>> - thp_disabled_by_hw(): the name suggests it checks if THP is disabled
>>>   by the hardware, but it just returns a cached value acquired with
>>>   has_transparent_hugepage(). This helper is used in fast paths
>>>
>>> This commit introduces a new helper called pgtable_has_pmd_leaves()
>>> which is intended to replace both has_transparent_hugepage() and
>>> thp_disabled_by_hw(). pgtable_has_pmd_leaves() has very clear semantics:
>>> it returns true if the CPU supports PMD-sized pages and false otherwise.
>>> It always returns a cached value, so it can be used in fast paths.
>>>
>>> The new helper requires an initialization step which is performed by
>>> init_arch_has_pmd_leaves(). We call init_arch_has_pmd_leaves() early
>>> during boot in start_kernel() right after parse_early_param() but before
>>> parse_args(). This allows early_param() handlers to change CPU flags if
>>> needed (eg. parse_memopt() in x86-32) while also allowing users to use
>>> the API from __setup() handlers.
>>>
>>> The next commits will convert users of both has_transparent_hugepage()
>>> and thp_disabled_by_hw() to pgtable_has_pmd_leaves().
>>>
>>> Signed-off-by: Luiz Capitulino <luizcap@redhat.com>
>>> ---
>>> include/linux/pgtable.h | 15 +++++++++++++++
>>> init/main.c             |  1 +
>>> mm/memory.c             |  8 ++++++++
>>> 3 files changed, 24 insertions(+)
>>>
>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>>> index a50df42a893f..c4c5282f795c 100644
>>> --- a/include/linux/pgtable.h
>>> +++ b/include/linux/pgtable.h
>>> @@ -2192,6 +2192,21 @@ static inline const char *pgtable_level_to_str(enum pgtable_level level)
>>> 	}
>>> }
>>>
>>> +#ifdef CONFIG_MMU
>>> +extern bool __arch_has_pmd_leaves;
>>> +static inline bool pgtable_has_pmd_leaves(void)
>>> +{
>>> +	return __arch_has_pmd_leaves;
>>> +}
>>> +void __init init_arch_has_pmd_leaves(void);
>>> +#else
>>> +static inline bool pgtable_has_pmd_leaves(void)
>>> +{
>>> +	return false;
>>> +}
>>> +static inline void __init init_arch_has_pmd_leaves(void) { }
>>> +#endif
>>> +
>>> #endif /* !__ASSEMBLY__ */
>>>
>>> #if !defined(MAX_POSSIBLE_PHYSMEM_BITS) && !defined(CONFIG_64BIT)
>>> diff --git a/init/main.c b/init/main.c
>>> index 1cb395dd94e4..07f2ddbf9677 100644
>>> --- a/init/main.c
>>> +++ b/init/main.c
>>> @@ -1044,6 +1044,7 @@ void start_kernel(void)
>>> 	print_kernel_cmdline(saved_command_line);
>>> 	/* parameters may set static keys */
>>> 	parse_early_param();
>>> +	init_arch_has_pmd_leaves();
>> 
>> One more thought here: I don't see why we need boot-time caching.
>> 
>> has_transparent_hugepage() does *not* look expensive on the common
>> archs. On x86, it is just a CPU feature check. MIPS is different, yes,
>> only the first call there is more involved ...
>
>The caching is not being introduced by this series. thp_disabled_by_hw()
>(which is used in hot paths) is checking a cached value too and this
>cached value is also set at boot-time by hugepage_init(). So, we keep
>the same behavior while extending the use of the cached value in the API
>(which makes sense for consistency).
>
>> But if we *really* want caching, couldn't we just do it lazily instead
>> of adding another early boot init step?
>> 
>> Something like:
>> 
>> bool pgtable_has_pmd_leaves(void)
>> {
>> 	static int __arch_has_pmd_leaves = -1;
>> 
>> 	if (READ_ONCE(__arch_has_pmd_leaves) < 0)
>> 		WRITE_ONCE(__arch_has_pmd_leaves, has_transparent_hugepage());
>> 
>> 	return READ_ONCE(__arch_has_pmd_leaves);
>> }
>> 
>> That would avoid depending on parse_early_param() / parse_args()
>> ordering, and it seems much simpler too :)
>> 
>> The boot-time init step looks a bit shaky to me ...
>
>I understand that caching on first access as you suggest above is an
>alternative way of doing this, but having to properly order
>initialization is very common in start_kernel(). So, unless we identify
>a case where the ordering is tricky or undoable, I don't see an issue
>with it. Also, note that this ordering exists today but it's implicity:
>thp_disabled_by_hw() can only be called after hugepage_init() runs.

Yeah, I see that, but I'm not a big fan of carrying that over into a new
generic API :)

The existing implict dependency does not really look like a good reason
to add another boot ordering assumption in start_kernel() if we can
avoid it.

I'd rather have the new helper reduce such assumption, not depend on
another init point ...

So if we want caching there, I still lean toward doing it lazily. But
let's see what folks think :)

>In terms of simplicity, look at how the resulting code (using an
>untested version of your static key suggestion) looks in comparison to
>caching at first use:
>
>DEFINE_STATIC_KEY_TRUE(__arch_has_pmd_leaves_key);
>
>static inline bool pgtable_has_pmd_leaves(void)
>{
>	return static_branch_likely(&__arch_has_pmd_leaves_key);
>}
>
>void __init init_arch_has_pmd_leaves(void)
>{
>	if (!arch_has_pmd_leaves())
>		static_branch_disable(&__arch_has_pmd_leaves_key);
>}
>
>


  reply	other threads:[~2026-04-13 15:45 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-08 20:22 [PATCH v3 00/10] mm: thp: always enable mTHP support Luiz Capitulino
2026-04-08 20:22 ` [PATCH v3 01/10] docs: tmpfs: remove implementation detail reference Luiz Capitulino
2026-04-09 15:11   ` Zi Yan
2026-04-10 16:00   ` Lance Yang
2026-04-08 20:22 ` [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves() Luiz Capitulino
2026-04-09 12:26   ` Lance Yang
2026-04-09 18:22     ` Luiz Capitulino
2026-04-10  8:19   ` Lance Yang
2026-04-13 15:24     ` Luiz Capitulino
2026-04-13 15:45       ` Lance Yang [this message]
2026-04-08 20:22 ` [PATCH v3 03/10] drivers: dax: use pgtable_has_pmd_leaves() Luiz Capitulino
2026-04-08 20:22 ` [PATCH v3 04/10] drivers: nvdimm: " Luiz Capitulino
2026-04-09 15:21   ` Zi Yan
2026-04-09 18:51     ` Luiz Capitulino
2026-04-08 20:23 ` [PATCH v3 05/10] mm: debug_vm_pgtable: " Luiz Capitulino
2026-04-09 15:25   ` Zi Yan
2026-04-10 16:09   ` Lance Yang
2026-04-08 20:23 ` [PATCH v3 06/10] mm: shmem: drop has_transparent_hugepage() usage Luiz Capitulino
2026-04-09 15:26   ` Zi Yan
2026-04-10 15:59   ` Lance Yang
2026-04-11  4:00     ` Lance Yang
2026-04-11  6:56   ` Baolin Wang
2026-04-08 20:23 ` [PATCH v3 07/10] treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves() Luiz Capitulino
2026-04-09 15:41   ` Zi Yan
2026-04-09 19:43     ` Luiz Capitulino
2026-04-13 15:32       ` Luiz Capitulino
2026-04-13 15:34         ` Zi Yan
2026-04-08 20:23 ` [PATCH v3 08/10] mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves() Luiz Capitulino
2026-04-09 15:43   ` Zi Yan
2026-04-11  7:01   ` Baolin Wang
2026-04-12 14:44   ` Lance Yang
2026-04-08 20:23 ` [PATCH v3 09/10] mm: thp: always enable mTHP support Luiz Capitulino
2026-04-09 15:55   ` Zi Yan
2026-04-09 20:07     ` Luiz Capitulino
2026-04-09 20:10       ` Zi Yan
2026-04-09 21:19         ` Luiz Capitulino
2026-04-11  7:22   ` Baolin Wang
2026-04-13 15:39     ` Luiz Capitulino
2026-04-08 20:23 ` [PATCH v3 10/10] mm: thp: x86: cleanup PSE feature bit usage Luiz Capitulino
2026-04-09 15:57   ` Zi Yan
2026-04-09 20:10     ` Dave Hansen
2026-04-09 21:24       ` Luiz Capitulino
2026-04-09  8:29 ` [PATCH v3 00/10] mm: thp: always enable mTHP support Lance Yang
2026-04-09  8:36   ` Lorenzo Stoakes
2026-04-09 18:18     ` Luiz Capitulino
2026-04-09 12:35   ` Lance Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260413154514.94193-1-lance.yang@linux.dev \
    --to=lance.yang@linux.dev \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=luizcap@redhat.com \
    --cc=npache@redhat.com \
    --cc=ryan.roberts@arm.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox