Skip to contentSkip to conversation

mm: thp: always enable mTHP support

PATCH Superseded Luiz Capitulino 7 participants 49 messages Apr 8 – Apr 20

Subsystems (12)

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v3 cover mm: thp: always enable mTHP support

Today, if an architecture implements has_transparent_hugepage() and the CPU lacks support for PMD-sized pages, the THP code disables all THP, including mTHP. In addition, the kernel lacks a well defined API to check for PMD-sized page support. It currently relies on has_transparent_hugepage() and thp_disabled_by_hw(), but they are not well defined and are tied to THP support.

This series addresses both issues by introducing a new well defined API to query PMD-sized page support: pgtable_has_pmd_leaves(). Using this new helper, we ensure that mTHP remains enabled even when the architecture or CPU doesn't support PMD-sized pages.

Thanks to David Hildenbrand for suggesting this improvement and for providing guidance (all bugs and misconceptions are mine).

This applies to Linus tree bfe62a454542 ("Merge tag 'soc-fixes-7.0-2' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc").

v3
--

- Rebased on top of latest Linus tree
- Removed i915 patch as driver dropped has_transparent_hugepage() usage
- Moved init_arch_has_pmd_leaves() call in start_kernel() to avoid conflict with early_param handlers clearing CPU feature flags
- Fixed build error with CONFIG_MMU=n (kernel test robot)
- Fixed huge_anon_orders_inherit default setting when !pgtable_pmd_leaves() (Baolin)
- Small commit changelog improvements

v2
--
- Added support for always enabling mTHPs for shmem (Baolin)
- Improved commits changelog & added reviewed-by

v1
--
- Call init_arch_has_pmd_leaves() from start_kernel()
- Keep pgtable_has_pmd_leaves() calls tied to CONFIG_TRANSPARENT_HUGEPAGE (David)
- Clear PUD_ORDER when clearing PMD_ORDER (David)
- Small changelog improvements (David)
- Rebased on top of latest mm-new

Luiz Capitulino (10):
docs: tmpfs: remove implementation detail reference
mm: introduce pgtable_has_pmd_leaves()
drivers: dax: use pgtable_has_pmd_leaves()
drivers: nvdimm: use pgtable_has_pmd_leaves()
mm: debug_vm_pgtable: use pgtable_has_pmd_leaves()
mm: shmem: drop has_transparent_hugepage() usage
treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves()
mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves()
mm: thp: always enable mTHP support
mm: thp: x86: cleanup PSE feature bit usage

 Documentation/filesystems/tmpfs.rst           |  5 ++---
 arch/mips/include/asm/pgtable.h               |  4 ++--
 arch/mips/mm/tlb-r4k.c                        |  4 ++--
 arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
 arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
 arch/powerpc/include/asm/book3s/64/pgtable.h  | 10 +++++-----
 arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
 arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
 arch/s390/include/asm/pgtable.h               |  4 ++--
 arch/x86/include/asm/pgtable.h                |  6 ------
 arch/x86/include/asm/pgtable_32.h             |  6 ++++++
 drivers/dax/dax-private.h                     |  2 +-
 drivers/nvdimm/pfn_devs.c                     |  6 ++++--
 include/linux/huge_mm.h                       |  7 -------
 include/linux/pgtable.h                       | 19 ++++++++++++++++--
 init/main.c                                   |  1 +
 mm/debug_vm_pgtable.c                         | 20 +++++++++----------
 mm/huge_memory.c                              | 15 +++++++-------
 mm/memory.c                                   | 10 +++++++++-
 mm/shmem.c                                    | 11 +++++-----
 20 files changed, 80 insertions(+), 60 deletions(-)
Signature · 1 line
2.53.0
#1 lore raw

PATCH v3 1/10 docs: tmpfs: remove implementation detail reference

The tmpfs.rst doc references the has_transparent_hugepage() helper, which is an implementation detail in the kernel and not relevant for users wishing to properly configure THP support for tmpfs. Remove it.

---
 Documentation/filesystems/tmpfs.rst | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Documentation/filesystems/tmpfs.rst +2 -3
index d677e0428c3f..46fc986c3388 100644
@@ -109,9 +109,8 @@ noswap  Disables swap. Remounts must respect the original settings.
 ======  ===========================================================
 
 tmpfs also supports Transparent Huge Pages which requires a kernel
-configured with CONFIG_TRANSPARENT_HUGEPAGE and with huge supported for
-your system (has_transparent_hugepage(), which is architecture specific).
-The mount options for this are:
+configured with CONFIG_TRANSPARENT_HUGEPAGE and with huge pages
+supported for your system. The mount options for this are:
 
 ================ ==============================================================
 huge=never       Do not allocate huge pages.  This is the default.
Signature · 1 line
2.53.0
reviewer Reviewed-by on v3 1/10
#2 lore raw

Re: [PATCH v3 01/10] docs: tmpfs: remove implementation detail reference

Quoted 11 lines · Luiz Capitulino

The tmpfs.rst doc references the has_transparent_hugepage() helper, which is an implementation detail in the kernel and not relevant for users wishing to properly configure THP support for tmpfs. Remove it.

---
 Documentation/filesystems/tmpfs.rst | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

LGTM.

Best Regards,
Yan, Zi

reviewer Reviewed-by on v3 1/10
#3 lore raw

Re: [PATCH v3 01/10] docs: tmpfs: remove implementation detail reference

In reply to Luiz Capitulino
Quoted 8 lines · Luiz Capitulino

The tmpfs.rst doc references the has_transparent_hugepage() helper, which is an implementation detail in the kernel and not relevant for users wishing to properly configure THP support for tmpfs. Remove it.

---

Thanks.

#4 lore raw

PATCH v3 2/10 mm: introduce pgtable_has_pmd_leaves()

In reply to Luiz Capitulino

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().

---
 include/linux/pgtable.h | 15 +++++++++++++++
 init/main.c             |  1 +
 mm/memory.c             |  8 ++++++++
 3 files changed, 24 insertions(+)
include/linux/pgtable.h +15 -0
index a50df42a893f..c4c5282f795c 100644
@@ -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)
init/main.c +1 -0
index 1cb395dd94e4..07f2ddbf9677 100644
@@ -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();
 	after_dashes = parse_args("Booting kernel",
 				  static_command_line, __start___param,
 				  __stop___param - __start___param,
mm/memory.c +8 -0
index c65e82c86fed..5ee312c30f2d 100644
@@ -177,6 +177,14 @@ static int __init init_zero_pfn(void)
 }
 early_initcall(init_zero_pfn);
 
+bool __arch_has_pmd_leaves __read_mostly;
+EXPORT_SYMBOL(__arch_has_pmd_leaves);
+
+void __init init_arch_has_pmd_leaves(void)
+{
+	__arch_has_pmd_leaves = has_transparent_hugepage();
+}
+
 void mm_trace_rss_stat(struct mm_struct *mm, int member)
 {
 	trace_rss_stat(mm, member);
Signature · 1 line
2.53.0
reviewer
#5 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

+Cc THP

Quoted 12 lines · Luiz Capitulino

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

Quoted 5 lines

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.

Set once at boot and then read in hot paths, would a static key be a better fit there instead? Something like:

DEFINE_STATIC_KEY_FALSE(arch_has_pmd_leaves_key);

static inline bool pgtable_has_pmd_leaves(void)

{
	return static_branch_unlikely(&arch_has_pmd_leaves_key);
}

void __init init_arch_has_pmd_leaves(void)

{
	if (has_transparent_hugepage())
		static_branch_enable(&arch_has_pmd_leaves_key);
}

Cheers,
Lance

#6 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

Quoted 15 lines · Lance Yang

+Cc THP

Quoted 12 lines · Luiz Capitulino

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

Quoted 8 lines
Quoted 5 lines

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.

Set once at boot and then read in hot paths, would a static key be a better fit there instead? Something like:

This looks like a good suggestion, if nobody opposes I can do this for v4 (although it may delay inclusion a bit).

Quoted 17 lines

DEFINE_STATIC_KEY_FALSE(arch_has_pmd_leaves_key);

static inline bool pgtable_has_pmd_leaves(void)

{
	return static_branch_unlikely(&arch_has_pmd_leaves_key);
}

void __init init_arch_has_pmd_leaves(void)

{
	if (has_transparent_hugepage())
		static_branch_enable(&arch_has_pmd_leaves_key);
}

Cheers,
Lance

reviewer
#7 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

In reply to Luiz Capitulino
Quoted 66 lines · Luiz Capitulino

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().

---

include/linux/pgtable.h | 15 +++++++++++++++
init/main.c             |  1 +
mm/memory.c             |  8 ++++++++
3 files changed, 24 insertions(+)
include/linux/pgtable.h +15 -0
index a50df42a893f..c4c5282f795c 100644
@@ -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)
init/main.c +0 -0
index 1cb395dd94e4..07f2ddbf9677 100644

init/main.c void start_kernel(void) · line 1047

Quoted 4 lines
init/main.c +1 -0
@@ -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 ...

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 ...

Thanks,
Lance

#8 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

Quoted 75 lines · Lance Yang
Quoted 66 lines · Luiz Capitulino

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().

---

include/linux/pgtable.h | 15 +++++++++++++++
init/main.c             |  1 +
mm/memory.c             |  8 ++++++++
3 files changed, 24 insertions(+)
include/linux/pgtable.h +15 -0
index a50df42a893f..c4c5282f795c 100644
@@ -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)
init/main.c +0 -0
index 1cb395dd94e4..07f2ddbf9677 100644

init/main.c void start_kernel(void) · line 1047

Quoted 4 lines
init/main.c +1 -0
@@ -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.

Quoted 3 lines

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).

Quoted 15 lines

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);
}
Quoted 4 lines

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.

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);
}
reviewer
#9 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

Quoted 104 lines · Luiz Capitulino
Quoted 75 lines · Lance Yang
Quoted 66 lines · Luiz Capitulino

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().

---

include/linux/pgtable.h | 15 +++++++++++++++
init/main.c             |  1 +
mm/memory.c             |  8 ++++++++
3 files changed, 24 insertions(+)
include/linux/pgtable.h +15 -0
index a50df42a893f..c4c5282f795c 100644
@@ -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)
init/main.c +0 -0
index 1cb395dd94e4..07f2ddbf9677 100644

init/main.c void start_kernel(void) · line 1047

Quoted 4 lines
init/main.c +1 -0
@@ -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.

Quoted 3 lines

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).

Quoted 18 lines

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 :)

Quoted 8 lines
Quoted 1 line

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 :)

Quoted 18 lines

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);
}
#10 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

In reply to Lance Yang
Quoted 84 lines · Lance Yang
Quoted 66 lines · Luiz Capitulino

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().

---

include/linux/pgtable.h | 15 +++++++++++++++
init/main.c             |  1 +
mm/memory.c             |  8 ++++++++
3 files changed, 24 insertions(+)
include/linux/pgtable.h +15 -0
index a50df42a893f..c4c5282f795c 100644
@@ -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)
init/main.c +0 -0
index 1cb395dd94e4..07f2ddbf9677 100644

init/main.c void start_kernel(void) · line 1047

Quoted 4 lines
init/main.c +1 -0
@@ -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 ...

But if we *really* want caching, couldn't we just do it lazily instead of adding another early boot init step?

Something like:

Quoted 8 lines

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);

Would look easier when just using two booleans: "has_pmd_leaves" and "probed".

I do like static keys, though. And wonder whether just putting that into hugepage_init() would be sufficient?

Signature · 3 lines
Cheers,

David
#11 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

Quoted 92 lines · David Hildenbrand (Arm)
Quoted 91 lines · Lance Yang
Quoted 66 lines · Luiz Capitulino

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().

---

include/linux/pgtable.h | 15 +++++++++++++++
init/main.c             |  1 +
mm/memory.c             |  8 ++++++++
3 files changed, 24 insertions(+)
include/linux/pgtable.h +15 -0
index a50df42a893f..c4c5282f795c 100644
@@ -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)
init/main.c +0 -0
index 1cb395dd94e4..07f2ddbf9677 100644

init/main.c void start_kernel(void) · line 1047

Quoted 4 lines
init/main.c +1 -0
@@ -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 ...

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());
Quoted 7 lines
Quoted 1 line
	return READ_ONCE(__arch_has_pmd_leaves);

Would look easier when just using two booleans: "has_pmd_leaves" and "probed".

I do like static keys, though. And wonder whether just putting that into hugepage_init() would be sufficient?

Do you mean the static key initialization? I think it's sufficient today, but we'd be tying the new API to THP and the initialization ordering still occurs although it's indirect.

reviewer
#12 lore raw

Re: [PATCH v3 02/10] mm: introduce pgtable_has_pmd_leaves()

In reply to David Hildenbrand (Arm)
Quoted 1 line · David Hildenbrand (Arm)

On 4/10/26 10:19, Lance Yang wrote:

[...]

Quoted 30 lines
Quoted 30 lines

init/main.c void start_kernel(void) · line 1047

Quoted 11 lines

#if !defined(MAX_POSSIBLE_PHYSMEM_BITS) && !defined(CONFIG_64BIT)

init/main.c +1 -0
index 1cb395dd94e4..07f2ddbf9677 100644
@@ -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 ...

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());
Quoted 7 lines
Quoted 1 line
	return READ_ONCE(__arch_has_pmd_leaves);

Would look easier when just using two booleans: "has_pmd_leaves" and "probed".

I do like static keys, though. And wonder whether just putting that into hugepage_init() would be sufficient?

Probably sufficient for the current callers, yes, since they are all THP-related :)

But then the helper may return false simply because THP is not built, rather than because the CPU lacks PMD leaf support, which does not match its semantics very well.

To be clear, I like static keys too, as I suggested earlier ;)

Another boot-time init step that depends on specific init ordering seems a bit shaky to me... At the very least, we'd need to document that ordering very clearly.

Cheers,
Lance

#13 lore raw

PATCH v3 3/10 drivers: dax: use pgtable_has_pmd_leaves()

In reply to Luiz Capitulino

dax_align_valid() uses has_transparent_hugepage() to check if PMD-sized pages are supported, use pgtable_has_pmd_leaves() instead.

---
 drivers/dax/dax-private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dax/dax-private.h +1 -1
index c6ae27c982f4..97b577f4107b 100644
@@ -119,7 +119,7 @@ static inline bool dax_align_valid(unsigned long align)
 {
 	if (align == PUD_SIZE && IS_ENABLED(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD))
 		return true;
-	if (align == PMD_SIZE && has_transparent_hugepage())
+	if (align == PMD_SIZE && pgtable_has_pmd_leaves())
 		return true;
 	if (align == PAGE_SIZE)
 		return true;
Signature · 1 line
2.53.0
#14 lore raw

PATCH v3 4/10 drivers: nvdimm: use pgtable_has_pmd_leaves()

In reply to Luiz Capitulino

nd_pfn_supported_alignments() and nd_pfn_default_alignment() use has_transparent_hugepage() to check if THP is supported with PMD-sized pages. Use pgtable_has_pmd_leaves() instead. Also, check for IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) to preserve the current implementation semantics.

---
 drivers/nvdimm/pfn_devs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/nvdimm/pfn_devs.c +4 -2
index 8fa9c16aba7e..457eb54e7ab6 100644
@@ -94,7 +94,8 @@ static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments)
 
 	alignments[0] = PAGE_SIZE;
 
-	if (has_transparent_hugepage()) {
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
+	    pgtable_has_pmd_leaves()) {
 		alignments[1] = HPAGE_PMD_SIZE;
 		if (has_transparent_pud_hugepage())
 			alignments[2] = HPAGE_PUD_SIZE;
@@ -109,7 +110,8 @@ static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments)
 static unsigned long nd_pfn_default_alignment(void)
 {
 
-	if (has_transparent_hugepage())
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
+	    pgtable_has_pmd_leaves())
 		return HPAGE_PMD_SIZE;
 	return PAGE_SIZE;
 }
Signature · 1 line
2.53.0
reviewer Acked-by on v3 4/10
#15 lore raw

Re: [PATCH v3 04/10] drivers: nvdimm: use pgtable_has_pmd_leaves()

Quoted 16 lines · Luiz Capitulino

nd_pfn_supported_alignments() and nd_pfn_default_alignment() use has_transparent_hugepage() to check if THP is supported with PMD-sized pages. Use pgtable_has_pmd_leaves() instead. Also, check for IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) to preserve the current implementation semantics.

---
 drivers/nvdimm/pfn_devs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/nvdimm/pfn_devs.c +0 -0
index 8fa9c16aba7e..457eb54e7ab6 100644

drivers/nvdimm/pfn_devs.c static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments) · line 100

Quoted 8 lines
drivers/nvdimm/pfn_devs.c +2 -1
@@ -94,7 +94,8 @@ static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments)
 
 	alignments[0] = PAGE_SIZE;
 
-	if (has_transparent_hugepage()) {
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
+	    pgtable_has_pmd_leaves()) {
 		alignments[1] = HPAGE_PMD_SIZE;
 		if (has_transparent_pud_hugepage())

Hmm, there is also has_transparent_pud_hugepage(). Should it be converted to pgtable_has_pud_leavs() like has_transparent_hugepage()?

Feel free to defer it to a future patchset.

Quoted 13 lines
drivers/nvdimm/pfn_devs.c +2 -1
 			alignments[2] = HPAGE_PUD_SIZE;
@@ -109,7 +110,8 @@ static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments)
 static unsigned long nd_pfn_default_alignment(void)
 {
 
-	if (has_transparent_hugepage())
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
+	    pgtable_has_pmd_leaves())
 		return HPAGE_PMD_SIZE;
 	return PAGE_SIZE;
 }
Signature · 1 line
2.53.0

Best Regards,
Yan, Zi

#16 lore raw

Re: [PATCH v3 04/10] drivers: nvdimm: use pgtable_has_pmd_leaves()

Quoted 30 lines · Zi Yan
Quoted 16 lines · Luiz Capitulino

nd_pfn_supported_alignments() and nd_pfn_default_alignment() use has_transparent_hugepage() to check if THP is supported with PMD-sized pages. Use pgtable_has_pmd_leaves() instead. Also, check for IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) to preserve the current implementation semantics.

---
  drivers/nvdimm/pfn_devs.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
drivers/nvdimm/pfn_devs.c +0 -0
index 8fa9c16aba7e..457eb54e7ab6 100644

drivers/nvdimm/pfn_devs.c static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments) · line 100

Quoted 8 lines
drivers/nvdimm/pfn_devs.c +2 -1
@@ -94,7 +94,8 @@ static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments)
 
  	alignments[0] = PAGE_SIZE;
 
-	if (has_transparent_hugepage()) {
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
+	    pgtable_has_pmd_leaves()) {
  		alignments[1] = HPAGE_PMD_SIZE;
  		if (has_transparent_pud_hugepage())

Hmm, there is also has_transparent_pud_hugepage(). Should it be converted to pgtable_has_pud_leavs() like has_transparent_hugepage()?

Quoted 1 line

Feel free to defer it to a future patchset.

Yes, I plan to tackle that next once this one is merged. (Thanks for the reviews, btw).

Quoted 21 lines
Quoted 13 lines
drivers/nvdimm/pfn_devs.c +2 -1
  			alignments[2] = HPAGE_PUD_SIZE;
@@ -109,7 +110,8 @@ static unsigned long *nd_pfn_supported_alignments(unsigned long *alignments)
  static unsigned long nd_pfn_default_alignment(void)
  {
 
-	if (has_transparent_hugepage())
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
+	    pgtable_has_pmd_leaves())
  		return HPAGE_PMD_SIZE;
  	return PAGE_SIZE;
  }
Signature · 1 line
2.53.0

Best Regards,
Yan, Zi

#17 lore raw

PATCH v3 5/10 mm: debug_vm_pgtable: use pgtable_has_pmd_leaves()

In reply to Luiz Capitulino

debug_vm_pgtable calls has_transparent_hugepage() in multiple places to check if PMD-sized pages are supported, use pgtable_has_pmd_leaves() instead.

---
 mm/debug_vm_pgtable.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
mm/debug_vm_pgtable.c +10 -10
index 83cf07269f13..b3e30baaeceb 100644
@@ -177,7 +177,7 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx)
 	unsigned long val = idx, *ptr = &val;
 	pmd_t pmd;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD basic (%pGv)\n", ptr);
@@ -222,7 +222,7 @@ static void __init pmd_advanced_tests(struct pgtable_debug_args *args)
 	pmd_t pmd;
 	unsigned long vaddr = args->vaddr;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	page = (args->pmd_pfn != ULONG_MAX) ? pfn_to_page(args->pmd_pfn) : NULL;
@@ -283,7 +283,7 @@ static void __init pmd_leaf_tests(struct pgtable_debug_args *args)
 {
 	pmd_t pmd;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD leaf\n");
@@ -688,7 +688,7 @@ static void __init pmd_protnone_tests(struct pgtable_debug_args *args)
 	if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
 		return;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD protnone\n");
@@ -737,7 +737,7 @@ static void __init pmd_soft_dirty_tests(struct pgtable_debug_args *args)
 	if (!pgtable_supports_soft_dirty())
 		return;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD soft dirty\n");
@@ -754,7 +754,7 @@ static void __init pmd_leaf_soft_dirty_tests(struct pgtable_debug_args *args)
 	    !IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION))
 		return;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD swap soft dirty\n");
@@ -825,7 +825,7 @@ static void __init pmd_softleaf_tests(struct pgtable_debug_args *args)
 	swp_entry_t arch_entry;
 	pmd_t pmd1, pmd2;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD swap\n");
@@ -906,7 +906,7 @@ static void __init pmd_thp_tests(struct pgtable_debug_args *args)
 {
 	pmd_t pmd;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD based THP\n");
@@ -997,7 +997,7 @@ static void __init destroy_args(struct pgtable_debug_args *args)
 	}
 
 	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
-	    has_transparent_hugepage() &&
+	    pgtable_has_pmd_leaves() &&
 	    args->pmd_pfn != ULONG_MAX) {
 		debug_vm_pgtable_free_huge_page(args, args->pmd_pfn, HPAGE_PMD_ORDER);
 		args->pmd_pfn = ULONG_MAX;
@@ -1249,7 +1249,7 @@ static int __init init_args(struct pgtable_debug_args *args)
 	}
 
 	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
-	    has_transparent_hugepage()) {
+	    pgtable_has_pmd_leaves()) {
 		page = debug_vm_pgtable_alloc_huge_page(args, HPAGE_PMD_ORDER);
 		if (page) {
 			args->pmd_pfn = page_to_pfn(page);
Signature · 1 line
2.53.0
reviewer Reviewed-by on v3 5/10
#18 lore raw

Re: [PATCH v3 05/10] mm: debug_vm_pgtable: use pgtable_has_pmd_leaves()

Quoted 77 lines · Luiz Capitulino

debug_vm_pgtable calls has_transparent_hugepage() in multiple places to check if PMD-sized pages are supported, use pgtable_has_pmd_leaves() instead.

---
 mm/debug_vm_pgtable.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
mm/debug_vm_pgtable.c +7 -7
index 83cf07269f13..b3e30baaeceb 100644
@@ -177,7 +177,7 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx)
 	unsigned long val = idx, *ptr = &val;
 	pmd_t pmd;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD basic (%pGv)\n", ptr);
@@ -222,7 +222,7 @@ static void __init pmd_advanced_tests(struct pgtable_debug_args *args)
 	pmd_t pmd;
 	unsigned long vaddr = args->vaddr;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	page = (args->pmd_pfn != ULONG_MAX) ? pfn_to_page(args->pmd_pfn) : NULL;
@@ -283,7 +283,7 @@ static void __init pmd_leaf_tests(struct pgtable_debug_args *args)
 {
 	pmd_t pmd;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD leaf\n");
@@ -688,7 +688,7 @@ static void __init pmd_protnone_tests(struct pgtable_debug_args *args)
 	if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
 		return;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD protnone\n");
@@ -737,7 +737,7 @@ static void __init pmd_soft_dirty_tests(struct pgtable_debug_args *args)
 	if (!pgtable_supports_soft_dirty())
 		return;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD soft dirty\n");
@@ -754,7 +754,7 @@ static void __init pmd_leaf_soft_dirty_tests(struct pgtable_debug_args *args)
 	    !IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION))
 		return;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD swap soft dirty\n");
@@ -825,7 +825,7 @@ static void __init pmd_softleaf_tests(struct pgtable_debug_args *args)
 	swp_entry_t arch_entry;
 	pmd_t pmd1, pmd2;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;
 
 	pr_debug("Validating PMD swap\n");

mm/debug_vm_pgtable.c static void __init pmd_thp_tests(struct pgtable_debug_args *args) · line 910

Quoted 6 lines
mm/debug_vm_pgtable.c +1 -1
@@ -906,7 +906,7 @@ static void __init pmd_thp_tests(struct pgtable_debug_args *args)
 {
 	pmd_t pmd;
 
-	if (!has_transparent_hugepage())
+	if (!pgtable_has_pmd_leaves())
 		return;

Do the above changes require additional IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) like the ones below? In patch 4, you said IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) is needed to preserve the existing semantics.

Never mind, since the above code is guarded by #ifdef CONFIG_TRANSPARENT_HUGEPAGE.

Quoted 22 lines
mm/debug_vm_pgtable.c +2 -2
 	pr_debug("Validating PMD based THP\n");
@@ -997,7 +997,7 @@ static void __init destroy_args(struct pgtable_debug_args *args)
 	}
 
 	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
-	    has_transparent_hugepage() &&
+	    pgtable_has_pmd_leaves() &&
 	    args->pmd_pfn != ULONG_MAX) {
 		debug_vm_pgtable_free_huge_page(args, args->pmd_pfn, HPAGE_PMD_ORDER);
 		args->pmd_pfn = ULONG_MAX;
@@ -1249,7 +1249,7 @@ static int __init init_args(struct pgtable_debug_args *args)
 	}
 
 	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
-	    has_transparent_hugepage()) {
+	    pgtable_has_pmd_leaves()) {
 		page = debug_vm_pgtable_alloc_huge_page(args, HPAGE_PMD_ORDER);
 		if (page) {
 			args->pmd_pfn = page_to_pfn(page);
Signature · 1 line
2.53.0

Best Regards,
Yan, Zi

#20 lore raw

PATCH v3 6/10 mm: shmem: drop has_transparent_hugepage() usage

In reply to Luiz Capitulino

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.

---
 mm/shmem.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
mm/shmem.c +3 -4
index b40f3cd48961..6f8b20d77e07 100644
@@ -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;
 
@@ -4664,8 +4664,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;
@@ -5451,7 +5450,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 */
Signature · 1 line
2.53.0
reviewer Acked-by on v3 6/10
#21 lore raw

Re: [PATCH v3 06/10] mm: shmem: drop has_transparent_hugepage() usage

Quoted 15 lines · Luiz Capitulino

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.

Quoted 5 lines
---
 mm/shmem.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Makes sense to me.

Best Regards,
Yan, Zi

reviewer Reviewed-by on v3 6/10
#22 lore raw

Re: [PATCH v3 06/10] mm: shmem: drop has_transparent_hugepage() usage

In reply to Luiz Capitulino
Quoted 45 lines · Luiz Capitulino

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.

---
mm/shmem.c | 7 +++----

1 file changed, 3 insertions(+), 4 deletions(-)
mm/shmem.c +2 -3
index b40f3cd48961..6f8b20d77e07 100644
@@ -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;
 
@@ -4664,8 +4664,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;

mm/shmem.c void __init shmem_init(void) · line 5456

Quoted 8 lines
mm/shmem.c +1 -1
@@ -5451,7 +5450,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 */

Although this patch simply drops the early has_transparent_hugepage() checks from the shmem parse/init paths, shmem_allowable_huge_orders() still returns 0 when thp_disabled_by_hw() is set.

So on hardware without PMD THP support:

unsigned long shmem_allowable_huge_orders(struct inode *inode,
struct vm_area_struct *vma, pgoff_t index, loff_t write_end, bool shmem_huge_force)
{
...

	if (thp_disabled_by_hw() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
		return 0;

...
}

1) the fault path still falls back to order-0 allocation
2) do_set_pmd() still falls back
3) khugepaged won't collapse it either

Nothing jumped out at me, thanks!

reviewer
#23 lore raw

Re: [PATCH v3 06/10] mm: shmem: drop has_transparent_hugepage() usage

Quoted 67 lines · Lance Yang
Quoted 45 lines · Luiz Capitulino

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.

---
mm/shmem.c | 7 +++----

1 file changed, 3 insertions(+), 4 deletions(-)
mm/shmem.c +2 -3
index b40f3cd48961..6f8b20d77e07 100644
@@ -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;
 
@@ -4664,8 +4664,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;

mm/shmem.c void __init shmem_init(void) · line 5456

Quoted 8 lines
mm/shmem.c +1 -1
@@ -5451,7 +5450,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 */

Although this patch simply drops the early has_transparent_hugepage() checks from the shmem parse/init paths, shmem_allowable_huge_orders() still returns 0 when thp_disabled_by_hw() is set.

So on hardware without PMD THP support:

unsigned long shmem_allowable_huge_orders(struct inode *inode,
struct vm_area_struct *vma, pgoff_t index, loff_t write_end, bool shmem_huge_force)
{
...

Quoted 8 lines
	if (thp_disabled_by_hw() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
		return 0;

...
}

1) the fault path still falls back to order-0 allocation
2) do_set_pmd() still falls back
3) khugepaged won't collapse it either

Forgot to add:

4) the buffered write path also falls back for tmpfs mounts

Quoted 4 lines

Nothing jumped out at me, thanks!

reviewer Reviewed-by on v3 6/10
#24 lore raw

Re: [PATCH v3 06/10] mm: shmem: drop has_transparent_hugepage() usage

In reply to Luiz Capitulino
Quoted 10 lines · Luiz Capitulino

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

Quoted 7 lines

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.

---

LGTM. Thanks.

#25 lore raw

PATCH v3 7/10 treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves()

In reply to Luiz Capitulino

Now that all the has_transparent_hugepage() callers have been converted to pgtable_has_pmd_leaves(), rename has_transparent_hugepage() to arch_has_pmd_leaves() since that's what the helper checks for.

arch_has_pmd_leaves() is supposed to be called only by init_arch_has_pmd_leaves(). The remaining exception is hugepage_init() which will be converted in a future commit.

---
 arch/mips/include/asm/pgtable.h               |  4 ++--
 arch/mips/mm/tlb-r4k.c                        |  4 ++--
 arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
 arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
 arch/powerpc/include/asm/book3s/64/pgtable.h  | 10 +++++-----
 arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
 arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
 arch/s390/include/asm/pgtable.h               |  4 ++--
 arch/x86/include/asm/pgtable.h                |  4 ++--
 include/linux/pgtable.h                       |  4 ++--
 mm/huge_memory.c                              |  2 +-
 mm/memory.c                                   |  2 +-
 12 files changed, 22 insertions(+), 22 deletions(-)
arch/mips/include/asm/pgtable.h +2 -2
index fa7b935f947c..a97b788315e2 100644
@@ -615,8 +615,8 @@ unsigned long io_remap_pfn_range_pfn(unsigned long pfn, unsigned long size);
 /* We don't have hardware dirty/accessed bits, generic_pmdp_establish is fine.*/
 #define pmdp_establish generic_pmdp_establish
 
-#define has_transparent_hugepage has_transparent_hugepage
-extern int has_transparent_hugepage(void);
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+extern int arch_has_pmd_leaves(void);
 
 static inline int pmd_trans_huge(pmd_t pmd)
 {
arch/mips/mm/tlb-r4k.c +2 -2
index 24fe85fa169d..c423b5784337 100644
@@ -434,7 +434,7 @@ void add_wired_entry(unsigned long entrylo0, unsigned long entrylo1,
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-int has_transparent_hugepage(void)
+int arch_has_pmd_leaves(void)
 {
 	static unsigned int mask = -1;
 
@@ -450,7 +450,7 @@ int has_transparent_hugepage(void)
 	}
 	return mask == PM_HUGE_MASK;
 }
-EXPORT_SYMBOL(has_transparent_hugepage);
+EXPORT_SYMBOL(arch_has_pmd_leaves);
 
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE  */
 
arch/powerpc/include/asm/book3s/64/hash-4k.h +1 -1
index 8e5bd9902bed..6744c2287199 100644
@@ -165,7 +165,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
 extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
 extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
 #endif
 
 #endif /* !__ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -1
index 7deb3a66890b..9392aba5e5dc 100644
@@ -278,7 +278,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
 extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
 extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
 #endif /*  CONFIG_TRANSPARENT_HUGEPAGE */
 
 #endif	/* __ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/pgtable.h +5 -5
index 1a91762b455d..e4d9b884af5c 100644
@@ -1121,14 +1121,14 @@ static inline void update_mmu_cache_pud(struct vm_area_struct *vma,
 {
 }
 
-extern int hash__has_transparent_hugepage(void);
-static inline int has_transparent_hugepage(void)
+extern int hash__arch_has_pmd_leaves(void);
+static inline int arch_has_pmd_leaves(void)
 {
 	if (radix_enabled())
-		return radix__has_transparent_hugepage();
-	return hash__has_transparent_hugepage();
+		return radix__arch_has_pmd_leaves();
+	return hash__arch_has_pmd_leaves();
 }
-#define has_transparent_hugepage has_transparent_hugepage
+#define arch_has_pmd_leaves arch_has_pmd_leaves
 
 static inline int has_transparent_pud_hugepage(void)
 {
arch/powerpc/include/asm/book3s/64/radix.h +1 -1
index da954e779744..c884a119cbd9 100644
@@ -298,7 +298,7 @@ extern pmd_t radix__pmdp_huge_get_and_clear(struct mm_struct *mm,
 pud_t radix__pudp_huge_get_and_clear(struct mm_struct *mm,
 				     unsigned long addr, pud_t *pudp);
 
-static inline int radix__has_transparent_hugepage(void)
+static inline int radix__arch_has_pmd_leaves(void)
 {
 	/* For radix 2M at PMD level means thp */
 	if (mmu_psize_defs[MMU_PAGE_2M].shift == PMD_SHIFT)
arch/powerpc/mm/book3s64/hash_pgtable.c +2 -2
index ac2a24d15d2e..f7933c52cca9 100644
@@ -370,7 +370,7 @@ pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 	return old_pmd;
 }
 
-int hash__has_transparent_hugepage(void)
+int hash__arch_has_pmd_leaves(void)
 {
 
 	if (!mmu_has_feature(MMU_FTR_16M_PAGE))
@@ -399,7 +399,7 @@ int hash__has_transparent_hugepage(void)
 
 	return 1;
 }
-EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
+EXPORT_SYMBOL_GPL(hash__arch_has_pmd_leaves);
 
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
arch/s390/include/asm/pgtable.h +2 -2
index 1c3c3be93be9..e638d914fbad 100644
@@ -1776,8 +1776,8 @@ static inline int pmd_trans_huge(pmd_t pmd)
 	return pmd_leaf(pmd);
 }
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
 {
 	return cpu_has_edat1() ? 1 : 0;
 }
arch/x86/include/asm/pgtable.h +2 -2
index 1662c5a8f445..8fe95270b713 100644
@@ -314,8 +314,8 @@ static inline int pud_trans_huge(pud_t pud)
 }
 #endif
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
 {
 	return boot_cpu_has(X86_FEATURE_PSE);
 }
include/linux/pgtable.h +2 -2
index c4c5282f795c..4b52a3abeab5 100644
@@ -2222,8 +2222,8 @@ static inline void __init init_arch_has_pmd_leaves(void) { }
 #endif
 #endif
 
-#ifndef has_transparent_hugepage
-#define has_transparent_hugepage() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)
+#ifndef arch_has_pmd_leaves
+#define arch_has_pmd_leaves() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)
 #endif
 
 #ifndef has_transparent_pud_hugepage
mm/huge_memory.c +1 -1
index b298cba853ab..9e4889d1673f 100644
@@ -908,7 +908,7 @@ static int __init hugepage_init(void)
 	int err;
 	struct kobject *hugepage_kobj;
 
-	if (!has_transparent_hugepage()) {
+	if (!arch_has_pmd_leaves()) {
 		transparent_hugepage_flags = 1 << TRANSPARENT_HUGEPAGE_UNSUPPORTED;
 		return -EINVAL;
 	}
mm/memory.c +1 -1
index 5ee312c30f2d..87a65f6eabfb 100644
@@ -182,7 +182,7 @@ EXPORT_SYMBOL(__arch_has_pmd_leaves);
 
 void __init init_arch_has_pmd_leaves(void)
 {
-	__arch_has_pmd_leaves = has_transparent_hugepage();
+	__arch_has_pmd_leaves = arch_has_pmd_leaves();
 }
 
 void mm_trace_rss_stat(struct mm_struct *mm, int member)
Signature · 1 line
2.53.0
reviewer
#26 lore raw

Re: [PATCH v3 07/10] treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves()

Quoted 181 lines · Luiz Capitulino

Now that all the has_transparent_hugepage() callers have been converted to pgtable_has_pmd_leaves(), rename has_transparent_hugepage() to arch_has_pmd_leaves() since that's what the helper checks for.

arch_has_pmd_leaves() is supposed to be called only by init_arch_has_pmd_leaves(). The remaining exception is hugepage_init() which will be converted in a future commit.

---
 arch/mips/include/asm/pgtable.h               |  4 ++--
 arch/mips/mm/tlb-r4k.c                        |  4 ++--
 arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
 arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
 arch/powerpc/include/asm/book3s/64/pgtable.h  | 10 +++++-----
 arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
 arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
 arch/s390/include/asm/pgtable.h               |  4 ++--
 arch/x86/include/asm/pgtable.h                |  4 ++--
 include/linux/pgtable.h                       |  4 ++--
 mm/huge_memory.c                              |  2 +-
 mm/memory.c                                   |  2 +-
 12 files changed, 22 insertions(+), 22 deletions(-)
arch/mips/include/asm/pgtable.h +2 -2
index fa7b935f947c..a97b788315e2 100644
@@ -615,8 +615,8 @@ unsigned long io_remap_pfn_range_pfn(unsigned long pfn, unsigned long size);
 /* We don't have hardware dirty/accessed bits, generic_pmdp_establish is fine.*/
 #define pmdp_establish generic_pmdp_establish
 
-#define has_transparent_hugepage has_transparent_hugepage
-extern int has_transparent_hugepage(void);
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+extern int arch_has_pmd_leaves(void);
 
 static inline int pmd_trans_huge(pmd_t pmd)
 {
arch/mips/mm/tlb-r4k.c +2 -2
index 24fe85fa169d..c423b5784337 100644
@@ -434,7 +434,7 @@ void add_wired_entry(unsigned long entrylo0, unsigned long entrylo1,
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-int has_transparent_hugepage(void)
+int arch_has_pmd_leaves(void)
 {
 	static unsigned int mask = -1;
 
@@ -450,7 +450,7 @@ int has_transparent_hugepage(void)
 	}
 	return mask == PM_HUGE_MASK;
 }
-EXPORT_SYMBOL(has_transparent_hugepage);
+EXPORT_SYMBOL(arch_has_pmd_leaves);
 
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE  */
 
arch/powerpc/include/asm/book3s/64/hash-4k.h +1 -1
index 8e5bd9902bed..6744c2287199 100644
@@ -165,7 +165,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
 extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
 extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
 #endif
 
 #endif /* !__ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -1
index 7deb3a66890b..9392aba5e5dc 100644
@@ -278,7 +278,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
 extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
 extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 				       unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
 #endif /*  CONFIG_TRANSPARENT_HUGEPAGE */
 
 #endif	/* __ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/pgtable.h +5 -5
index 1a91762b455d..e4d9b884af5c 100644
@@ -1121,14 +1121,14 @@ static inline void update_mmu_cache_pud(struct vm_area_struct *vma,
 {
 }
 
-extern int hash__has_transparent_hugepage(void);
-static inline int has_transparent_hugepage(void)
+extern int hash__arch_has_pmd_leaves(void);
+static inline int arch_has_pmd_leaves(void)
 {
 	if (radix_enabled())
-		return radix__has_transparent_hugepage();
-	return hash__has_transparent_hugepage();
+		return radix__arch_has_pmd_leaves();
+	return hash__arch_has_pmd_leaves();
 }
-#define has_transparent_hugepage has_transparent_hugepage
+#define arch_has_pmd_leaves arch_has_pmd_leaves
 
 static inline int has_transparent_pud_hugepage(void)
 {
arch/powerpc/include/asm/book3s/64/radix.h +1 -1
index da954e779744..c884a119cbd9 100644
@@ -298,7 +298,7 @@ extern pmd_t radix__pmdp_huge_get_and_clear(struct mm_struct *mm,
 pud_t radix__pudp_huge_get_and_clear(struct mm_struct *mm,
 				     unsigned long addr, pud_t *pudp);
 
-static inline int radix__has_transparent_hugepage(void)
+static inline int radix__arch_has_pmd_leaves(void)
 {
 	/* For radix 2M at PMD level means thp */
 	if (mmu_psize_defs[MMU_PAGE_2M].shift == PMD_SHIFT)
arch/powerpc/mm/book3s64/hash_pgtable.c +2 -2
index ac2a24d15d2e..f7933c52cca9 100644
@@ -370,7 +370,7 @@ pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
 	return old_pmd;
 }
 
-int hash__has_transparent_hugepage(void)
+int hash__arch_has_pmd_leaves(void)
 {
 
 	if (!mmu_has_feature(MMU_FTR_16M_PAGE))
@@ -399,7 +399,7 @@ int hash__has_transparent_hugepage(void)
 
 	return 1;
 }
-EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
+EXPORT_SYMBOL_GPL(hash__arch_has_pmd_leaves);
 
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
arch/s390/include/asm/pgtable.h +2 -2
index 1c3c3be93be9..e638d914fbad 100644
@@ -1776,8 +1776,8 @@ static inline int pmd_trans_huge(pmd_t pmd)
 	return pmd_leaf(pmd);
 }
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
 {
 	return cpu_has_edat1() ? 1 : 0;
 }
arch/x86/include/asm/pgtable.h +2 -2
index 1662c5a8f445..8fe95270b713 100644
@@ -314,8 +314,8 @@ static inline int pud_trans_huge(pud_t pud)
 }
 #endif
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
 {
 	return boot_cpu_has(X86_FEATURE_PSE);
 }
include/linux/pgtable.h +0 -0
index c4c5282f795c..4b52a3abeab5 100644

include/linux/pgtable.h static inline void __init init_arch_has_pmd_leaves(void) { } · line 2226

Quoted 7 lines
include/linux/pgtable.h +2 -2
@@ -2222,8 +2222,8 @@ static inline void __init init_arch_has_pmd_leaves(void) { }
 #endif
 #endif
 
-#ifndef has_transparent_hugepage
-#define has_transparent_hugepage() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)
+#ifndef arch_has_pmd_leaves
+#define arch_has_pmd_leaves() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)

Should this be
#define arch_has_pmd_leaves() 1
?

Since all has_transparent_hugepage() is converted
to IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && pgtable_has_pmd_leaves() and pgtable_has_pmd_leaves() is basically arch_has_pmd_leaves(). The IS_ENABLED() is the same as IS_BUILTIN() here.

Before this patch, if an arch does not define has_transparent_hugepage, has_transparent_hugepage() basically means the arch supports PMD leaves and software controls the availability of THP. Now you rename it to arch_has_pmd_leaves() and setting it to 1 reflects its actual semantics.

Best Regards,
Yan, Zi

#27 lore raw

Re: [PATCH v3 07/10] treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves()

Quoted 200 lines · Zi Yan
Quoted 181 lines · Luiz Capitulino

Now that all the has_transparent_hugepage() callers have been converted to pgtable_has_pmd_leaves(), rename has_transparent_hugepage() to arch_has_pmd_leaves() since that's what the helper checks for.

arch_has_pmd_leaves() is supposed to be called only by init_arch_has_pmd_leaves(). The remaining exception is hugepage_init() which will be converted in a future commit.

---
  arch/mips/include/asm/pgtable.h               |  4 ++--
  arch/mips/mm/tlb-r4k.c                        |  4 ++--
  arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
  arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
  arch/powerpc/include/asm/book3s/64/pgtable.h  | 10 +++++-----
  arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
  arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
  arch/s390/include/asm/pgtable.h               |  4 ++--
  arch/x86/include/asm/pgtable.h                |  4 ++--
  include/linux/pgtable.h                       |  4 ++--
  mm/huge_memory.c                              |  2 +-
  mm/memory.c                                   |  2 +-
  12 files changed, 22 insertions(+), 22 deletions(-)
arch/mips/include/asm/pgtable.h +2 -2
index fa7b935f947c..a97b788315e2 100644
@@ -615,8 +615,8 @@ unsigned long io_remap_pfn_range_pfn(unsigned long pfn, unsigned long size);
  /* We don't have hardware dirty/accessed bits, generic_pmdp_establish is fine.*/
  #define pmdp_establish generic_pmdp_establish
 
-#define has_transparent_hugepage has_transparent_hugepage
-extern int has_transparent_hugepage(void);
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+extern int arch_has_pmd_leaves(void);
 
  static inline int pmd_trans_huge(pmd_t pmd)
  {
arch/mips/mm/tlb-r4k.c +2 -2
index 24fe85fa169d..c423b5784337 100644
@@ -434,7 +434,7 @@ void add_wired_entry(unsigned long entrylo0, unsigned long entrylo1,
 
  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-int has_transparent_hugepage(void)
+int arch_has_pmd_leaves(void)
  {
  	static unsigned int mask = -1;
 
@@ -450,7 +450,7 @@ int has_transparent_hugepage(void)
  	}
  	return mask == PM_HUGE_MASK;
  }
-EXPORT_SYMBOL(has_transparent_hugepage);
+EXPORT_SYMBOL(arch_has_pmd_leaves);
 
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE  */
 
arch/powerpc/include/asm/book3s/64/hash-4k.h +1 -1
index 8e5bd9902bed..6744c2287199 100644
@@ -165,7 +165,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
  				       unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
  #endif
 
  #endif /* !__ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -1
index 7deb3a66890b..9392aba5e5dc 100644
@@ -278,7 +278,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
  				       unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
  #endif /*  CONFIG_TRANSPARENT_HUGEPAGE */
 
  #endif	/* __ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/pgtable.h +5 -5
index 1a91762b455d..e4d9b884af5c 100644
@@ -1121,14 +1121,14 @@ static inline void update_mmu_cache_pud(struct vm_area_struct *vma,
  {
  }
 
-extern int hash__has_transparent_hugepage(void);
-static inline int has_transparent_hugepage(void)
+extern int hash__arch_has_pmd_leaves(void);
+static inline int arch_has_pmd_leaves(void)
  {
  	if (radix_enabled())
-		return radix__has_transparent_hugepage();
-	return hash__has_transparent_hugepage();
+		return radix__arch_has_pmd_leaves();
+	return hash__arch_has_pmd_leaves();
  }
-#define has_transparent_hugepage has_transparent_hugepage
+#define arch_has_pmd_leaves arch_has_pmd_leaves
 
  static inline int has_transparent_pud_hugepage(void)
  {
arch/powerpc/include/asm/book3s/64/radix.h +1 -1
index da954e779744..c884a119cbd9 100644
@@ -298,7 +298,7 @@ extern pmd_t radix__pmdp_huge_get_and_clear(struct mm_struct *mm,
  pud_t radix__pudp_huge_get_and_clear(struct mm_struct *mm,
  				     unsigned long addr, pud_t *pudp);
 
-static inline int radix__has_transparent_hugepage(void)
+static inline int radix__arch_has_pmd_leaves(void)
  {
  	/* For radix 2M at PMD level means thp */
  	if (mmu_psize_defs[MMU_PAGE_2M].shift == PMD_SHIFT)
arch/powerpc/mm/book3s64/hash_pgtable.c +2 -2
index ac2a24d15d2e..f7933c52cca9 100644
@@ -370,7 +370,7 @@ pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
  	return old_pmd;
  }
 
-int hash__has_transparent_hugepage(void)
+int hash__arch_has_pmd_leaves(void)
  {
 
  	if (!mmu_has_feature(MMU_FTR_16M_PAGE))
@@ -399,7 +399,7 @@ int hash__has_transparent_hugepage(void)
 
  	return 1;
  }
-EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
+EXPORT_SYMBOL_GPL(hash__arch_has_pmd_leaves);
 
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
arch/s390/include/asm/pgtable.h +2 -2
index 1c3c3be93be9..e638d914fbad 100644
@@ -1776,8 +1776,8 @@ static inline int pmd_trans_huge(pmd_t pmd)
  	return pmd_leaf(pmd);
  }
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
  {
  	return cpu_has_edat1() ? 1 : 0;
  }
arch/x86/include/asm/pgtable.h +2 -2
index 1662c5a8f445..8fe95270b713 100644
@@ -314,8 +314,8 @@ static inline int pud_trans_huge(pud_t pud)
  }
  #endif
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
  {
  	return boot_cpu_has(X86_FEATURE_PSE);
  }
include/linux/pgtable.h +0 -0
index c4c5282f795c..4b52a3abeab5 100644

include/linux/pgtable.h static inline void __init init_arch_has_pmd_leaves(void) { } · line 2226

Quoted 7 lines
include/linux/pgtable.h +2 -2
@@ -2222,8 +2222,8 @@ static inline void __init init_arch_has_pmd_leaves(void) { }
  #endif
  #endif
 
-#ifndef has_transparent_hugepage
-#define has_transparent_hugepage() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)
+#ifndef arch_has_pmd_leaves
+#define arch_has_pmd_leaves() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)

Should this be
#define arch_has_pmd_leaves() 1
?

Since all has_transparent_hugepage() is converted
to IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && pgtable_has_pmd_leaves() and pgtable_has_pmd_leaves() is basically arch_has_pmd_leaves(). The IS_ENABLED() is the same as IS_BUILTIN() here.

Quoted 4 lines

Before this patch, if an arch does not define has_transparent_hugepage, has_transparent_hugepage() basically means the arch supports PMD leaves and software controls the availability of THP. Now you rename it to arch_has_pmd_leaves() and setting it to 1 reflects its actual semantics.

Yes, you're right that the intended future semantics is: if an arch doesn't define arch_has_pmd_leaves() then the kernel assumes it supports PMD pages.

I originally kept arch_has_pmd_leaves() tied to THP because I was afraid of breaking the current callers who want to use PMD pages with THP. But, as you say, they should all be checking for IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) now.

I'll look into implementing your suggestion for v4.

#28 lore raw

Re: [PATCH v3 07/10] treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves()

Quoted 210 lines · Luiz Capitulino
Quoted 200 lines · Zi Yan
Quoted 181 lines · Luiz Capitulino

Now that all the has_transparent_hugepage() callers have been converted to pgtable_has_pmd_leaves(), rename has_transparent_hugepage() to arch_has_pmd_leaves() since that's what the helper checks for.

arch_has_pmd_leaves() is supposed to be called only by init_arch_has_pmd_leaves(). The remaining exception is hugepage_init() which will be converted in a future commit.

---
  arch/mips/include/asm/pgtable.h               |  4 ++--
  arch/mips/mm/tlb-r4k.c                        |  4 ++--
  arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
  arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
  arch/powerpc/include/asm/book3s/64/pgtable.h  | 10 +++++-----
  arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
  arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
  arch/s390/include/asm/pgtable.h               |  4 ++--
  arch/x86/include/asm/pgtable.h                |  4 ++--
  include/linux/pgtable.h                       |  4 ++--
  mm/huge_memory.c                              |  2 +-
  mm/memory.c                                   |  2 +-
  12 files changed, 22 insertions(+), 22 deletions(-)
arch/mips/include/asm/pgtable.h +2 -2
index fa7b935f947c..a97b788315e2 100644
@@ -615,8 +615,8 @@ unsigned long io_remap_pfn_range_pfn(unsigned long pfn, unsigned long size);
  /* We don't have hardware dirty/accessed bits, generic_pmdp_establish is fine.*/
  #define pmdp_establish generic_pmdp_establish
 
-#define has_transparent_hugepage has_transparent_hugepage
-extern int has_transparent_hugepage(void);
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+extern int arch_has_pmd_leaves(void);
 
  static inline int pmd_trans_huge(pmd_t pmd)
  {
arch/mips/mm/tlb-r4k.c +2 -2
index 24fe85fa169d..c423b5784337 100644
@@ -434,7 +434,7 @@ void add_wired_entry(unsigned long entrylo0, unsigned long entrylo1,
 
  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-int has_transparent_hugepage(void)
+int arch_has_pmd_leaves(void)
  {
      static unsigned int mask = -1;
 
@@ -450,7 +450,7 @@ int has_transparent_hugepage(void)
      }
      return mask == PM_HUGE_MASK;
  }
-EXPORT_SYMBOL(has_transparent_hugepage);
+EXPORT_SYMBOL(arch_has_pmd_leaves);
 
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE  */
 
arch/powerpc/include/asm/book3s/64/hash-4k.h +1 -1
index 8e5bd9902bed..6744c2287199 100644
@@ -165,7 +165,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
                         unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
  #endif
 
  #endif /* !__ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -1
index 7deb3a66890b..9392aba5e5dc 100644
@@ -278,7 +278,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
                         unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
  #endif /*  CONFIG_TRANSPARENT_HUGEPAGE */
 
  #endif    /* __ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/pgtable.h +5 -5
index 1a91762b455d..e4d9b884af5c 100644
@@ -1121,14 +1121,14 @@ static inline void update_mmu_cache_pud(struct vm_area_struct *vma,
  {
  }
 
-extern int hash__has_transparent_hugepage(void);
-static inline int has_transparent_hugepage(void)
+extern int hash__arch_has_pmd_leaves(void);
+static inline int arch_has_pmd_leaves(void)
  {
      if (radix_enabled())
-        return radix__has_transparent_hugepage();
-    return hash__has_transparent_hugepage();
+        return radix__arch_has_pmd_leaves();
+    return hash__arch_has_pmd_leaves();
  }
-#define has_transparent_hugepage has_transparent_hugepage
+#define arch_has_pmd_leaves arch_has_pmd_leaves
 
  static inline int has_transparent_pud_hugepage(void)
  {
arch/powerpc/include/asm/book3s/64/radix.h +1 -1
index da954e779744..c884a119cbd9 100644
@@ -298,7 +298,7 @@ extern pmd_t radix__pmdp_huge_get_and_clear(struct mm_struct *mm,
  pud_t radix__pudp_huge_get_and_clear(struct mm_struct *mm,
                       unsigned long addr, pud_t *pudp);
 
-static inline int radix__has_transparent_hugepage(void)
+static inline int radix__arch_has_pmd_leaves(void)
  {
      /* For radix 2M at PMD level means thp */
      if (mmu_psize_defs[MMU_PAGE_2M].shift == PMD_SHIFT)
arch/powerpc/mm/book3s64/hash_pgtable.c +2 -2
index ac2a24d15d2e..f7933c52cca9 100644
@@ -370,7 +370,7 @@ pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
      return old_pmd;
  }
 
-int hash__has_transparent_hugepage(void)
+int hash__arch_has_pmd_leaves(void)
  {
 
      if (!mmu_has_feature(MMU_FTR_16M_PAGE))
@@ -399,7 +399,7 @@ int hash__has_transparent_hugepage(void)
 
      return 1;
  }
-EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
+EXPORT_SYMBOL_GPL(hash__arch_has_pmd_leaves);
 
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
arch/s390/include/asm/pgtable.h +2 -2
index 1c3c3be93be9..e638d914fbad 100644
@@ -1776,8 +1776,8 @@ static inline int pmd_trans_huge(pmd_t pmd)
      return pmd_leaf(pmd);
  }
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
  {
      return cpu_has_edat1() ? 1 : 0;
  }
arch/x86/include/asm/pgtable.h +2 -2
index 1662c5a8f445..8fe95270b713 100644
@@ -314,8 +314,8 @@ static inline int pud_trans_huge(pud_t pud)
  }
  #endif
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
  {
      return boot_cpu_has(X86_FEATURE_PSE);
  }
include/linux/pgtable.h +0 -0
index c4c5282f795c..4b52a3abeab5 100644

include/linux/pgtable.h static inline void __init init_arch_has_pmd_leaves(void) { } · line 2226

Quoted 7 lines
include/linux/pgtable.h +2 -2
@@ -2222,8 +2222,8 @@ static inline void __init init_arch_has_pmd_leaves(void) { }
  #endif
  #endif
 
-#ifndef has_transparent_hugepage
-#define has_transparent_hugepage() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)
+#ifndef arch_has_pmd_leaves
+#define arch_has_pmd_leaves() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)

Should this be
#define arch_has_pmd_leaves() 1
?

Since all has_transparent_hugepage() is converted
to IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && pgtable_has_pmd_leaves() and pgtable_has_pmd_leaves() is basically arch_has_pmd_leaves(). The IS_ENABLED() is the same as IS_BUILTIN() here.

Quoted 4 lines

Before this patch, if an arch does not define has_transparent_hugepage, has_transparent_hugepage() basically means the arch supports PMD leaves and software controls the availability of THP. Now you rename it to arch_has_pmd_leaves() and setting it to 1 reflects its actual semantics.

Yes, you're right that the intended future semantics is: if an arch doesn't define arch_has_pmd_leaves() then the kernel assumes it supports PMD pages.

Quoted 6 lines

I originally kept arch_has_pmd_leaves() tied to THP because I was afraid of breaking the current callers who want to use PMD pages with THP. But, as you say, they should all be checking for IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) now.

I'll look into implementing your suggestion for v4.

In taking a second look at this, I realized that a better default should be:

#define arch_has_pmd_leaves() IS_ENABLED(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE)

Since archs defining CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE should support PMD pages. This is also consistent with has_transparent_pud_hugepage().

reviewer
#29 lore raw

Re: [PATCH v3 07/10] treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves()

Quoted 218 lines · Luiz Capitulino
Quoted 210 lines · Luiz Capitulino
Quoted 200 lines · Zi Yan
Quoted 181 lines · Luiz Capitulino

Now that all the has_transparent_hugepage() callers have been converted to pgtable_has_pmd_leaves(), rename has_transparent_hugepage() to arch_has_pmd_leaves() since that's what the helper checks for.

arch_has_pmd_leaves() is supposed to be called only by init_arch_has_pmd_leaves(). The remaining exception is hugepage_init() which will be converted in a future commit.

---
  arch/mips/include/asm/pgtable.h               |  4 ++--
  arch/mips/mm/tlb-r4k.c                        |  4 ++--
  arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
  arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
  arch/powerpc/include/asm/book3s/64/pgtable.h  | 10 +++++-----
  arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
  arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
  arch/s390/include/asm/pgtable.h               |  4 ++--
  arch/x86/include/asm/pgtable.h                |  4 ++--
  include/linux/pgtable.h                       |  4 ++--
  mm/huge_memory.c                              |  2 +-
  mm/memory.c                                   |  2 +-
  12 files changed, 22 insertions(+), 22 deletions(-)
arch/mips/include/asm/pgtable.h +2 -2
index fa7b935f947c..a97b788315e2 100644
@@ -615,8 +615,8 @@ unsigned long io_remap_pfn_range_pfn(unsigned long pfn, unsigned long size);
  /* We don't have hardware dirty/accessed bits, generic_pmdp_establish is fine.*/
  #define pmdp_establish generic_pmdp_establish
 
-#define has_transparent_hugepage has_transparent_hugepage
-extern int has_transparent_hugepage(void);
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+extern int arch_has_pmd_leaves(void);
 
  static inline int pmd_trans_huge(pmd_t pmd)
  {
arch/mips/mm/tlb-r4k.c +2 -2
index 24fe85fa169d..c423b5784337 100644
@@ -434,7 +434,7 @@ void add_wired_entry(unsigned long entrylo0, unsigned long entrylo1,
 
  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-int has_transparent_hugepage(void)
+int arch_has_pmd_leaves(void)
  {
      static unsigned int mask = -1;
 
@@ -450,7 +450,7 @@ int has_transparent_hugepage(void)
      }
      return mask == PM_HUGE_MASK;
  }
-EXPORT_SYMBOL(has_transparent_hugepage);
+EXPORT_SYMBOL(arch_has_pmd_leaves);
 
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE  */
 
arch/powerpc/include/asm/book3s/64/hash-4k.h +1 -1
index 8e5bd9902bed..6744c2287199 100644
@@ -165,7 +165,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
                         unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
  #endif
 
  #endif /* !__ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/hash-64k.h +1 -1
index 7deb3a66890b..9392aba5e5dc 100644
@@ -278,7 +278,7 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
                         unsigned long addr, pmd_t *pmdp);
-extern int hash__has_transparent_hugepage(void);
+extern int hash__arch_has_pmd_leaves(void);
  #endif /*  CONFIG_TRANSPARENT_HUGEPAGE */
 
  #endif    /* __ASSEMBLER__ */
arch/powerpc/include/asm/book3s/64/pgtable.h +5 -5
index 1a91762b455d..e4d9b884af5c 100644
@@ -1121,14 +1121,14 @@ static inline void update_mmu_cache_pud(struct vm_area_struct *vma,
  {
  }
 
-extern int hash__has_transparent_hugepage(void);
-static inline int has_transparent_hugepage(void)
+extern int hash__arch_has_pmd_leaves(void);
+static inline int arch_has_pmd_leaves(void)
  {
      if (radix_enabled())
-        return radix__has_transparent_hugepage();
-    return hash__has_transparent_hugepage();
+        return radix__arch_has_pmd_leaves();
+    return hash__arch_has_pmd_leaves();
  }
-#define has_transparent_hugepage has_transparent_hugepage
+#define arch_has_pmd_leaves arch_has_pmd_leaves
 
  static inline int has_transparent_pud_hugepage(void)
  {
arch/powerpc/include/asm/book3s/64/radix.h +1 -1
index da954e779744..c884a119cbd9 100644
@@ -298,7 +298,7 @@ extern pmd_t radix__pmdp_huge_get_and_clear(struct mm_struct *mm,
  pud_t radix__pudp_huge_get_and_clear(struct mm_struct *mm,
                       unsigned long addr, pud_t *pudp);
 
-static inline int radix__has_transparent_hugepage(void)
+static inline int radix__arch_has_pmd_leaves(void)
  {
      /* For radix 2M at PMD level means thp */
      if (mmu_psize_defs[MMU_PAGE_2M].shift == PMD_SHIFT)
arch/powerpc/mm/book3s64/hash_pgtable.c +2 -2
index ac2a24d15d2e..f7933c52cca9 100644
@@ -370,7 +370,7 @@ pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
      return old_pmd;
  }
 
-int hash__has_transparent_hugepage(void)
+int hash__arch_has_pmd_leaves(void)
  {
 
      if (!mmu_has_feature(MMU_FTR_16M_PAGE))
@@ -399,7 +399,7 @@ int hash__has_transparent_hugepage(void)
 
      return 1;
  }
-EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
+EXPORT_SYMBOL_GPL(hash__arch_has_pmd_leaves);
 
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
arch/s390/include/asm/pgtable.h +2 -2
index 1c3c3be93be9..e638d914fbad 100644
@@ -1776,8 +1776,8 @@ static inline int pmd_trans_huge(pmd_t pmd)
      return pmd_leaf(pmd);
  }
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
  {
      return cpu_has_edat1() ? 1 : 0;
  }
arch/x86/include/asm/pgtable.h +2 -2
index 1662c5a8f445..8fe95270b713 100644
@@ -314,8 +314,8 @@ static inline int pud_trans_huge(pud_t pud)
  }
  #endif
 
-#define has_transparent_hugepage has_transparent_hugepage
-static inline int has_transparent_hugepage(void)
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
  {
      return boot_cpu_has(X86_FEATURE_PSE);
  }
include/linux/pgtable.h +0 -0
index c4c5282f795c..4b52a3abeab5 100644

include/linux/pgtable.h static inline void __init init_arch_has_pmd_leaves(void) { } · line 2226

Quoted 7 lines
include/linux/pgtable.h +2 -2
@@ -2222,8 +2222,8 @@ static inline void __init init_arch_has_pmd_leaves(void) { }
  #endif
  #endif
 
-#ifndef has_transparent_hugepage
-#define has_transparent_hugepage() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)
+#ifndef arch_has_pmd_leaves
+#define arch_has_pmd_leaves() IS_BUILTIN(CONFIG_TRANSPARENT_HUGEPAGE)

Should this be
#define arch_has_pmd_leaves() 1
?

Since all has_transparent_hugepage() is converted
to IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && pgtable_has_pmd_leaves() and pgtable_has_pmd_leaves() is basically arch_has_pmd_leaves(). The IS_ENABLED() is the same as IS_BUILTIN() here.

Quoted 4 lines

Before this patch, if an arch does not define has_transparent_hugepage, has_transparent_hugepage() basically means the arch supports PMD leaves and software controls the availability of THP. Now you rename it to arch_has_pmd_leaves() and setting it to 1 reflects its actual semantics.

Yes, you're right that the intended future semantics is: if an arch doesn't define arch_has_pmd_leaves() then the kernel assumes it supports PMD pages.

Quoted 6 lines

I originally kept arch_has_pmd_leaves() tied to THP because I was afraid of breaking the current callers who want to use PMD pages with THP. But, as you say, they should all be checking for IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) now.

I'll look into implementing your suggestion for v4.

Quoted 8 lines

In taking a second look at this, I realized that a better default should be:

#define arch_has_pmd_leaves() IS_ENABLED(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE)

Since archs defining CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE should support PMD pages. This is also consistent with has_transparent_pud_hugepage().

Sounds good to me. Thank you for improving the code.

Best Regards,
Yan, Zi

#30 lore raw

PATCH v3 8/10 mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves()

In reply to Luiz Capitulino

Despite its name, thp_disabled_by_hw() just checks whether the architecture supports PMD-sized pages. It returns true when TRANSPARENT_HUGEPAGE_UNSUPPORTED is set in transparent_hugepage_flags, this only occurs if the architecture implements arch_has_pmd_leaves() and that function returns false.

Since pgtable_has_pmd_leaves() provides the same semantics, use it instead.

---
 include/linux/huge_mm.h | 7 -------
 mm/huge_memory.c        | 6 ++----
 mm/memory.c             | 2 +-
 mm/shmem.c              | 2 +-
 4 files changed, 4 insertions(+), 13 deletions(-)
include/linux/huge_mm.h +0 -7
index a4d9f964dfde..e291a650b10f 100644
@@ -47,7 +47,6 @@ vm_fault_t vmf_insert_folio_pud(struct vm_fault *vmf, struct folio *folio,
 				bool write);
 
 enum transparent_hugepage_flag {
-	TRANSPARENT_HUGEPAGE_UNSUPPORTED,
 	TRANSPARENT_HUGEPAGE_FLAG,
 	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
 	TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG,
@@ -352,12 +351,6 @@ static inline bool vma_thp_disabled(struct vm_area_struct *vma,
 	return mm_flags_test(MMF_DISABLE_THP_EXCEPT_ADVISED, vma->vm_mm);
 }
 
-static inline bool thp_disabled_by_hw(void)
-{
-	/* If the hardware/firmware marked hugepage support disabled. */
-	return transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_UNSUPPORTED);
-}
-
 unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
 		unsigned long len, unsigned long pgoff, unsigned long flags);
 unsigned long thp_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
mm/huge_memory.c +2 -4
index 9e4889d1673f..86e489c0a150 100644
@@ -125,7 +125,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
 	if (!vma->vm_mm)		/* vdso */
 		return 0;
 
-	if (thp_disabled_by_hw() || vma_thp_disabled(vma, vm_flags, forced_collapse))
+	if (!pgtable_has_pmd_leaves() || vma_thp_disabled(vma, vm_flags, forced_collapse))
 		return 0;
 
 	/* khugepaged doesn't collapse DAX vma, but page fault is fine. */
@@ -908,10 +908,8 @@ static int __init hugepage_init(void)
 	int err;
 	struct kobject *hugepage_kobj;
 
-	if (!arch_has_pmd_leaves()) {
-		transparent_hugepage_flags = 1 << TRANSPARENT_HUGEPAGE_UNSUPPORTED;
+	if (!pgtable_has_pmd_leaves())
 		return -EINVAL;
-	}
 
 	/*
 	 * hugepages can't be allocated by the buddy allocator
mm/memory.c +1 -1
index 87a65f6eabfb..29cb4ea33ba0 100644
@@ -5427,7 +5427,7 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *pa
 	 * PMD mappings if THPs are disabled. As we already have a THP,
 	 * behave as if we are forcing a collapse.
 	 */
-	if (thp_disabled_by_hw() || vma_thp_disabled(vma, vma->vm_flags,
+	if (!pgtable_has_pmd_leaves() || vma_thp_disabled(vma, vma->vm_flags,
 						     /* forced_collapse=*/ true))
 		return ret;
 
mm/shmem.c +1 -1
index 6f8b20d77e07..613393eae5a9 100644
@@ -1839,7 +1839,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
 	vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
 	unsigned int global_orders;
 
-	if (thp_disabled_by_hw() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
+	if (!pgtable_has_pmd_leaves() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
 		return 0;
 
 	global_orders = shmem_huge_global_enabled(inode, index, write_end,
Signature · 1 line
2.53.0
reviewer Acked-by on v3 8/10
#31 lore raw

Re: [PATCH v3 08/10] mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves()

Quoted 9 lines · Luiz Capitulino

Despite its name, thp_disabled_by_hw() just checks whether the architecture supports PMD-sized pages. It returns true when TRANSPARENT_HUGEPAGE_UNSUPPORTED is set in transparent_hugepage_flags, this only occurs if the architecture implements arch_has_pmd_leaves() and that function returns false.

Since pgtable_has_pmd_leaves() provides the same semantics, use it instead.

Quoted 8 lines
---
 include/linux/huge_mm.h | 7 -------
 mm/huge_memory.c        | 6 ++----
 mm/memory.c             | 2 +-
 mm/shmem.c              | 2 +-
 4 files changed, 4 insertions(+), 13 deletions(-)

LGTM.

Best Regards,
Yan, Zi

reviewer Reviewed-by on v3 8/10
#32 lore raw

Re: [PATCH v3 08/10] mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves()

In reply to Luiz Capitulino
Quoted 11 lines · Luiz Capitulino

Despite its name, thp_disabled_by_hw() just checks whether the architecture supports PMD-sized pages. It returns true when TRANSPARENT_HUGEPAGE_UNSUPPORTED is set in transparent_hugepage_flags, this only occurs if the architecture implements arch_has_pmd_leaves() and that function returns false.

Since pgtable_has_pmd_leaves() provides the same semantics, use it instead.

---

LGTM.

reviewer Reviewed-by on v3 8/10
#33 lore raw

Re: [PATCH v3 08/10] mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves()

In reply to Luiz Capitulino
Quoted 11 lines · Luiz Capitulino

Despite its name, thp_disabled_by_hw() just checks whether the architecture supports PMD-sized pages. It returns true when TRANSPARENT_HUGEPAGE_UNSUPPORTED is set in transparent_hugepage_flags, this only occurs if the architecture implements arch_has_pmd_leaves() and that function returns false.

Since pgtable_has_pmd_leaves() provides the same semantics, use it instead.

---

Nice cleanup, thanks!

#34 lore raw

PATCH v3 9/10 mm: thp: always enable mTHP support

In reply to Luiz Capitulino

If PMD-sized pages are not supported on an architecture (ie. the arch implements arch_has_pmd_leaves() and it returns false) then the current code disables all THP, including mTHP.

This commit fixes this by allowing mTHP to be always enabled for all archs. When PMD-sized pages are not supported, its sysfs entry won't be created and their mapping will be disallowed at page-fault time.

Similarly, this commit implements the following changes for shmem:

- In shmem_allowable_huge_orders(): drop the pgtable_has_pmd_leaves() check so that mTHP sizes are considered
- In shmem_alloc_and_add_folio(): don't consider PMD and PUD orders when PMD-sized pages are not supported by the CPU

---
 mm/huge_memory.c | 13 ++++++++-----
 mm/shmem.c       |  4 +++-
 2 files changed, 11 insertions(+), 6 deletions(-)
mm/huge_memory.c +8 -5
index 86e489c0a150..6de3d8ebc35c 100644
@@ -118,6 +118,9 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
 	else
 		supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
 
+	if (!pgtable_has_pmd_leaves())
+		supported_orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+
 	orders &= supported_orders;
 	if (!orders)
 		return 0;
@@ -125,7 +128,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
 	if (!vma->vm_mm)		/* vdso */
 		return 0;
 
-	if (!pgtable_has_pmd_leaves() || vma_thp_disabled(vma, vm_flags, forced_collapse))
+	if (vma_thp_disabled(vma, vm_flags, forced_collapse))
 		return 0;
 
 	/* khugepaged doesn't collapse DAX vma, but page fault is fine. */
@@ -787,7 +790,7 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
 	 * disable all other sizes. powerpc's PMD_ORDER isn't a compile-time
 	 * constant so we have to do this here.
 	 */
-	if (!anon_orders_configured)
+	if (!anon_orders_configured && pgtable_has_pmd_leaves())
 		huge_anon_orders_inherit = BIT(PMD_ORDER);
 
 	*hugepage_kobj = kobject_create_and_add("transparent_hugepage", mm_kobj);
@@ -809,6 +812,9 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
 	}
 
 	orders = THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_FILE_DEFAULT;
+	if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+
 	order = highest_order(orders);
 	while (orders) {
 		thpsize = thpsize_create(order, *hugepage_kobj);
@@ -908,9 +914,6 @@ static int __init hugepage_init(void)
 	int err;
 	struct kobject *hugepage_kobj;
 
-	if (!pgtable_has_pmd_leaves())
-		return -EINVAL;
-
 	/*
 	 * hugepages can't be allocated by the buddy allocator
 	 */
mm/shmem.c +3 -1
index 613393eae5a9..b49a30475cb0 100644
@@ -1839,7 +1839,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
 	vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
 	unsigned int global_orders;
 
-	if (!pgtable_has_pmd_leaves() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
+	if (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force))
 		return 0;
 
 	global_orders = shmem_huge_global_enabled(inode, index, write_end,
@@ -1947,6 +1947,8 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 
 	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
 		orders = 0;
+	else if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
 
 	if (orders > 0) {
 		suitable_orders = shmem_suitable_orders(inode, vmf,
Signature · 1 line
2.53.0
reviewer
#35 lore raw
Quoted 26 lines · Luiz Capitulino

If PMD-sized pages are not supported on an architecture (ie. the arch implements arch_has_pmd_leaves() and it returns false) then the current code disables all THP, including mTHP.

This commit fixes this by allowing mTHP to be always enabled for all archs. When PMD-sized pages are not supported, its sysfs entry won't be created and their mapping will be disallowed at page-fault time.

Similarly, this commit implements the following changes for shmem:

- In shmem_allowable_huge_orders(): drop the pgtable_has_pmd_leaves() check so that mTHP sizes are considered
- In shmem_alloc_and_add_folio(): don't consider PMD and PUD orders when PMD-sized pages are not supported by the CPU

---
 mm/huge_memory.c | 13 ++++++++-----
 mm/shmem.c       |  4 +++-
 2 files changed, 11 insertions(+), 6 deletions(-)
mm/huge_memory.c +0 -0
index 86e489c0a150..6de3d8ebc35c 100644

mm/huge_memory.c unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma, · line 122

Quoted 5 lines
mm/huge_memory.c +2 -0
@@ -118,6 +118,9 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
 	else
 		supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
 
+	if (!pgtable_has_pmd_leaves())
+		supported_orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Why is BIT(PUD_ORDER) also removed? I thought PMD THP support and PUD THP support are separate. Here the code implies PUD THP relies on PMD THP. Is that the case?

Quoted 23 lines
mm/huge_memory.c +3 -2
+
 	orders &= supported_orders;
 	if (!orders)
 		return 0;
@@ -125,7 +128,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
 	if (!vma->vm_mm)		/* vdso */
 		return 0;
 
-	if (!pgtable_has_pmd_leaves() || vma_thp_disabled(vma, vm_flags, forced_collapse))
+	if (vma_thp_disabled(vma, vm_flags, forced_collapse))
 		return 0;
 
 	/* khugepaged doesn't collapse DAX vma, but page fault is fine. */
@@ -787,7 +790,7 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
 	 * disable all other sizes. powerpc's PMD_ORDER isn't a compile-time
 	 * constant so we have to do this here.
 	 */
-	if (!anon_orders_configured)
+	if (!anon_orders_configured && pgtable_has_pmd_leaves())
 		huge_anon_orders_inherit = BIT(PMD_ORDER);
 
 	*hugepage_kobj = kobject_create_and_add("transparent_hugepage", mm_kobj);

mm/huge_memory.c static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj) · line 817

Quoted 6 lines
mm/huge_memory.c +3 -0
@@ -809,6 +812,9 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
 	}
 
 	orders = THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_FILE_DEFAULT;
+	if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+

Ditto.

mm/huge_memory.c static int __init hugepage_init(void) · line 919

Quoted 13 lines
mm/huge_memory.c +0 -3
 	order = highest_order(orders);
 	while (orders) {
 		thpsize = thpsize_create(order, *hugepage_kobj);
@@ -908,9 +914,6 @@ static int __init hugepage_init(void)
 	int err;
 	struct kobject *hugepage_kobj;
 
-	if (!pgtable_has_pmd_leaves())
-		return -EINVAL;
-
 	/*
 	 * hugepages can't be allocated by the buddy allocator
 	 */

The code after is:

MAYBE_BUILD_BUG_ON(HPAGE_PMD_ORDER > MAX_PAGE_ORDER);

Should this check be removed or only performed when pgtable_has_pmd_leaves()?

I do not know if there is a possible Kconfig that lowers MAX_PAGE_ORDER below PMD_ORDER and enables THP. After this patchset, that might be valid if people do not want to use mTHP but not PMD THP.

Quoted 14 lines
mm/shmem.c +1 -1
index 613393eae5a9..b49a30475cb0 100644
@@ -1839,7 +1839,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
 	vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
 	unsigned int global_orders;
 
-	if (!pgtable_has_pmd_leaves() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
+	if (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force))
 		return 0;
 
 	global_orders = shmem_huge_global_enabled(inode, index, write_end,

mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1951

Quoted 5 lines
mm/shmem.c +2 -0
@@ -1947,6 +1947,8 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 
 	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
 		orders = 0;
+	else if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Same question as the first one.

Quoted 5 lines
mm/shmem.c +0 -0
 	if (orders > 0) {
 		suitable_orders = shmem_suitable_orders(inode, vmf,
Signature · 1 line
2.53.0

Best Regards,
Yan, Zi

#36 lore raw
Quoted 31 lines · Zi Yan
Quoted 29 lines · Luiz Capitulino

If PMD-sized pages are not supported on an architecture (ie. the arch implements arch_has_pmd_leaves() and it returns false) then the current code disables all THP, including mTHP.

This commit fixes this by allowing mTHP to be always enabled for all archs. When PMD-sized pages are not supported, its sysfs entry won't be created and their mapping will be disallowed at page-fault time.

Similarly, this commit implements the following changes for shmem:

- In shmem_allowable_huge_orders(): drop the pgtable_has_pmd_leaves() check so that mTHP sizes are considered
- In shmem_alloc_and_add_folio(): don't consider PMD and PUD orders when PMD-sized pages are not supported by the CPU

---
  mm/huge_memory.c | 13 ++++++++-----
  mm/shmem.c       |  4 +++-
  2 files changed, 11 insertions(+), 6 deletions(-)
mm/huge_memory.c +0 -0
index 86e489c0a150..6de3d8ebc35c 100644
@@ -118,6 +118,9 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
  	else
  		supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
 
Quoted 5 lines

mm/huge_memory.c unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma, · line 122

Quoted 2 lines
mm/huge_memory.c +2 -0
+	if (!pgtable_has_pmd_leaves())
+		supported_orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Why is BIT(PUD_ORDER) also removed? I thought PMD THP support and PUD THP support are separate. Here the code implies PUD THP relies on PMD THP. Is that the case?

This was a suggestion from David to an earlier version:

https://lore.kernel.org/linux-mm/[email protected]/

My understanding was that if an arch doesn't support PMD pages then it probably doesn't support PUD pages either.

Quoted 47 lines
Quoted 23 lines
mm/huge_memory.c +3 -2
+
  	orders &= supported_orders;
  	if (!orders)
  		return 0;
@@ -125,7 +128,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
  	if (!vma->vm_mm)		/* vdso */
  		return 0;
 
-	if (!pgtable_has_pmd_leaves() || vma_thp_disabled(vma, vm_flags, forced_collapse))
+	if (vma_thp_disabled(vma, vm_flags, forced_collapse))
  		return 0;
 
  	/* khugepaged doesn't collapse DAX vma, but page fault is fine. */
@@ -787,7 +790,7 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
  	 * disable all other sizes. powerpc's PMD_ORDER isn't a compile-time
  	 * constant so we have to do this here.
  	 */
-	if (!anon_orders_configured)
+	if (!anon_orders_configured && pgtable_has_pmd_leaves())
  		huge_anon_orders_inherit = BIT(PMD_ORDER);
 
  	*hugepage_kobj = kobject_create_and_add("transparent_hugepage", mm_kobj);

mm/huge_memory.c static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj) · line 817

Quoted 6 lines
mm/huge_memory.c +3 -0
@@ -809,6 +812,9 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
  	}
 
  	orders = THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_FILE_DEFAULT;
+	if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+

Ditto.

mm/huge_memory.c static int __init hugepage_init(void) · line 919

Quoted 13 lines
mm/huge_memory.c +0 -3
  	order = highest_order(orders);
  	while (orders) {
  		thpsize = thpsize_create(order, *hugepage_kobj);
@@ -908,9 +914,6 @@ static int __init hugepage_init(void)
  	int err;
  	struct kobject *hugepage_kobj;
 
-	if (!pgtable_has_pmd_leaves())
-		return -EINVAL;
-
  	/*
  	 * hugepages can't be allocated by the buddy allocator
  	 */

The code after is:

Quoted 7 lines

MAYBE_BUILD_BUG_ON(HPAGE_PMD_ORDER > MAX_PAGE_ORDER);

Should this check be removed or only performed when pgtable_has_pmd_leaves()?

I do not know if there is a possible Kconfig that lowers MAX_PAGE_ORDER below PMD_ORDER and enables THP. After this patchset, that might be valid if people do not want to use mTHP but not PMD THP.

I need to look into this more carefully to be able to answer this, I'll get back to you.

Quoted 32 lines
Quoted 14 lines
mm/shmem.c +1 -1
index 613393eae5a9..b49a30475cb0 100644
@@ -1839,7 +1839,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
  	vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
  	unsigned int global_orders;
 
-	if (!pgtable_has_pmd_leaves() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
+	if (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force))
  		return 0;
 
  	global_orders = shmem_huge_global_enabled(inode, index, write_end,

mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1951

Quoted 5 lines
mm/shmem.c +2 -0
@@ -1947,6 +1947,8 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 
  	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
  		orders = 0;
+	else if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Same question as the first one.

Quoted 5 lines
mm/shmem.c +0 -0
  	if (orders > 0) {
  		suitable_orders = shmem_suitable_orders(inode, vmf,
Signature · 1 line
2.53.0

Best Regards,
Yan, Zi

reviewer
#37 lore raw
Quoted 38 lines · Luiz Capitulino
Quoted 31 lines · Zi Yan
Quoted 29 lines · Luiz Capitulino

If PMD-sized pages are not supported on an architecture (ie. the arch implements arch_has_pmd_leaves() and it returns false) then the current code disables all THP, including mTHP.

This commit fixes this by allowing mTHP to be always enabled for all archs. When PMD-sized pages are not supported, its sysfs entry won't be created and their mapping will be disallowed at page-fault time.

Similarly, this commit implements the following changes for shmem:

- In shmem_allowable_huge_orders(): drop the pgtable_has_pmd_leaves() check so that mTHP sizes are considered
- In shmem_alloc_and_add_folio(): don't consider PMD and PUD orders when PMD-sized pages are not supported by the CPU

---
  mm/huge_memory.c | 13 ++++++++-----
  mm/shmem.c       |  4 +++-
  2 files changed, 11 insertions(+), 6 deletions(-)
mm/huge_memory.c +0 -0
index 86e489c0a150..6de3d8ebc35c 100644
@@ -118,6 +118,9 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
  	else
  		supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
 
Quoted 5 lines

mm/huge_memory.c unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma, · line 122

Quoted 2 lines
mm/huge_memory.c +2 -0
+	if (!pgtable_has_pmd_leaves())
+		supported_orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Why is BIT(PUD_ORDER) also removed? I thought PMD THP support and PUD THP support are separate. Here the code implies PUD THP relies on PMD THP. Is that the case?

Quoted 6 lines

This was a suggestion from David to an earlier version:

https://lore.kernel.org/linux-mm/[email protected]/

My understanding was that if an arch doesn't support PMD pages then it probably doesn't support PUD pages either.

Got it. Can you add a comment on “No PMD leaves -> No PUD leaves” for pgtable_has_pmd_leaves(), so that no one would ask the same question again?

Thanks.

Best Regards,
Yan, Zi

#38 lore raw
Quoted 42 lines · Zi Yan
Quoted 40 lines · Luiz Capitulino
Quoted 31 lines · Zi Yan
Quoted 29 lines · Luiz Capitulino

If PMD-sized pages are not supported on an architecture (ie. the arch implements arch_has_pmd_leaves() and it returns false) then the current code disables all THP, including mTHP.

This commit fixes this by allowing mTHP to be always enabled for all archs. When PMD-sized pages are not supported, its sysfs entry won't be created and their mapping will be disallowed at page-fault time.

Similarly, this commit implements the following changes for shmem:

- In shmem_allowable_huge_orders(): drop the pgtable_has_pmd_leaves() check so that mTHP sizes are considered
- In shmem_alloc_and_add_folio(): don't consider PMD and PUD orders when PMD-sized pages are not supported by the CPU

---
   mm/huge_memory.c | 13 ++++++++-----
   mm/shmem.c       |  4 +++-
   2 files changed, 11 insertions(+), 6 deletions(-)
mm/huge_memory.c +0 -0
index 86e489c0a150..6de3d8ebc35c 100644
@@ -118,6 +118,9 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
   	else
   		supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
 
Quoted 5 lines

mm/huge_memory.c unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma, · line 122

Quoted 2 lines
mm/huge_memory.c +2 -0
+	if (!pgtable_has_pmd_leaves())
+		supported_orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Why is BIT(PUD_ORDER) also removed? I thought PMD THP support and PUD THP support are separate. Here the code implies PUD THP relies on PMD THP. Is that the case?

This was a suggestion from David to an earlier version:

Quoted 7 lines
Quoted 4 lines

https://lore.kernel.org/linux-mm/[email protected]/

My understanding was that if an arch doesn't support PMD pages then it probably doesn't support PUD pages either.

Got it. Can you add a comment on “No PMD leaves -> No PUD leaves” for pgtable_has_pmd_leaves(), so that no one would ask the same question again?

Sure, I can do that.

Quoted 6 lines

Thanks.

Best Regards,
Yan, Zi

reviewer
#39 lore raw
In reply to Luiz Capitulino
Quoted 87 lines · Luiz Capitulino

If PMD-sized pages are not supported on an architecture (ie. the arch implements arch_has_pmd_leaves() and it returns false) then the current code disables all THP, including mTHP.

This commit fixes this by allowing mTHP to be always enabled for all archs. When PMD-sized pages are not supported, its sysfs entry won't be created and their mapping will be disallowed at page-fault time.

Similarly, this commit implements the following changes for shmem:

- In shmem_allowable_huge_orders(): drop the pgtable_has_pmd_leaves() check so that mTHP sizes are considered
- In shmem_alloc_and_add_folio(): don't consider PMD and PUD orders when PMD-sized pages are not supported by the CPU

---
  mm/huge_memory.c | 13 ++++++++-----
  mm/shmem.c       |  4 +++-
  2 files changed, 11 insertions(+), 6 deletions(-)
mm/huge_memory.c +8 -5
index 86e489c0a150..6de3d8ebc35c 100644
@@ -118,6 +118,9 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
  	else
  		supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
 
+	if (!pgtable_has_pmd_leaves())
+		supported_orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+
  	orders &= supported_orders;
  	if (!orders)
  		return 0;
@@ -125,7 +128,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
  	if (!vma->vm_mm)		/* vdso */
  		return 0;
 
-	if (!pgtable_has_pmd_leaves() || vma_thp_disabled(vma, vm_flags, forced_collapse))
+	if (vma_thp_disabled(vma, vm_flags, forced_collapse))
  		return 0;
 
  	/* khugepaged doesn't collapse DAX vma, but page fault is fine. */
@@ -787,7 +790,7 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
  	 * disable all other sizes. powerpc's PMD_ORDER isn't a compile-time
  	 * constant so we have to do this here.
  	 */
-	if (!anon_orders_configured)
+	if (!anon_orders_configured && pgtable_has_pmd_leaves())
  		huge_anon_orders_inherit = BIT(PMD_ORDER);
 
  	*hugepage_kobj = kobject_create_and_add("transparent_hugepage", mm_kobj);
@@ -809,6 +812,9 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
  	}
 
  	orders = THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_FILE_DEFAULT;
+	if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+
  	order = highest_order(orders);
  	while (orders) {
  		thpsize = thpsize_create(order, *hugepage_kobj);
@@ -908,9 +914,6 @@ static int __init hugepage_init(void)
  	int err;
  	struct kobject *hugepage_kobj;
 
-	if (!pgtable_has_pmd_leaves())
-		return -EINVAL;
-
  	/*
  	 * hugepages can't be allocated by the buddy allocator
  	 */
mm/shmem.c +1 -1
index 613393eae5a9..b49a30475cb0 100644
@@ -1839,7 +1839,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
  	vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
  	unsigned int global_orders;
 
-	if (!pgtable_has_pmd_leaves() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
+	if (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force))
  		return 0;
 
  	global_orders = shmem_huge_global_enabled(inode, index, write_end,

mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1951

Quoted 5 lines
mm/shmem.c +2 -0
@@ -1947,6 +1947,8 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 
  	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
  		orders = 0;
+	else if (!pgtable_has_pmd_leaves())
+		orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Sorry, I still don't like the changes here, because shmem_allowable_huge_orders() is meant to determine which large orders are allowed. Something like below (untested):

@@ -1834,6 +1834,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
struct vm_area_struct *vma, pgoff_t index, loff_t write_end, bool shmem_huge_force)
{
+ unsigned int filter_orders = pgtable_has_pmd_leaves() ? (BIT(PMD_ORDER) | BIT(PUD_ORDER)) : 0;
unsigned long mask = READ_ONCE(huge_shmem_orders_always); unsigned long within_size_orders =

READ_ONCE(huge_shmem_orders_within_size);
        vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
@@ -1846,7 +1847,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
                                                  shmem_huge_force, 
vma, vm_flags);
        /* Tmpfs huge pages allocation */
        if (!vma || !vma_is_anon_shmem(vma))

- return global_orders;
+ return global_orders & ~filter_orders;

/*
* Following the 'deny' semantics of the top level, force the huge
@@ -1871,6 +1872,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
if (global_orders > 0)
mask |= READ_ONCE(huge_shmem_orders_inherit);

+       mask &= ~filter_orders;
        return THP_ORDERS_ALL_FILE_DEFAULT & mask;

}

Additionally, we also need a pgtable_has_pmd_leaves() check before setting huge_shmem_orders_inherit as well.

@@ -5428,7 +5430,7 @@ void __init shmem_init(void)
* Default to setting PMD-sized THP to inherit the global
setting and
* disable all other multi-size THPs.
*/
- if (!shmem_orders_configured)
+ if (!shmem_orders_configured && pgtable_has_pmd_leaves())

                huge_shmem_orders_inherit = BIT(HPAGE_PMD_ORDER);
 #endif
        return;
#40 lore raw
Quoted 79 lines · Baolin Wang
Quoted 76 lines · Luiz Capitulino

If PMD-sized pages are not supported on an architecture (ie. the arch implements arch_has_pmd_leaves() and it returns false) then the current code disables all THP, including mTHP.

This commit fixes this by allowing mTHP to be always enabled for all archs. When PMD-sized pages are not supported, its sysfs entry won't be created and their mapping will be disallowed at page-fault time.

Similarly, this commit implements the following changes for shmem:

- In shmem_allowable_huge_orders(): drop the pgtable_has_pmd_leaves() check so that mTHP sizes are considered
- In shmem_alloc_and_add_folio(): don't consider PMD and PUD orders when PMD-sized pages are not supported by the CPU

---
  mm/huge_memory.c | 13 ++++++++-----
  mm/shmem.c       |  4 +++-
  2 files changed, 11 insertions(+), 6 deletions(-)
mm/huge_memory.c +8 -5
index 86e489c0a150..6de3d8ebc35c 100644
@@ -118,6 +118,9 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
      else
          supported_orders = THP_ORDERS_ALL_FILE_DEFAULT;
+    if (!pgtable_has_pmd_leaves())
+        supported_orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+
      orders &= supported_orders;
      if (!orders)
          return 0;
@@ -125,7 +128,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
      if (!vma->vm_mm)        /* vdso */
          return 0;
-    if (!pgtable_has_pmd_leaves() || vma_thp_disabled(vma, vm_flags, forced_collapse))
+    if (vma_thp_disabled(vma, vm_flags, forced_collapse))
          return 0;
      /* khugepaged doesn't collapse DAX vma, but page fault is fine. */
@@ -787,7 +790,7 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
       * disable all other sizes. powerpc's PMD_ORDER isn't a compile-time
       * constant so we have to do this here.
       */
-    if (!anon_orders_configured)
+    if (!anon_orders_configured && pgtable_has_pmd_leaves())
          huge_anon_orders_inherit = BIT(PMD_ORDER);
      *hugepage_kobj = kobject_create_and_add("transparent_hugepage", mm_kobj);
@@ -809,6 +812,9 @@ static int __init hugepage_init_sysfs(struct kobject **hugepage_kobj)
      }
      orders = THP_ORDERS_ALL_ANON | THP_ORDERS_ALL_FILE_DEFAULT;
+    if (!pgtable_has_pmd_leaves())
+        orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));
+
      order = highest_order(orders);
      while (orders) {
          thpsize = thpsize_create(order, *hugepage_kobj);
@@ -908,9 +914,6 @@ static int __init hugepage_init(void)
      int err;
      struct kobject *hugepage_kobj;
-    if (!pgtable_has_pmd_leaves())
-        return -EINVAL;
-
      /*
       * hugepages can't be allocated by the buddy allocator
       */
mm/shmem.c +1 -1
index 613393eae5a9..b49a30475cb0 100644
@@ -1839,7 +1839,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
      vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
      unsigned int global_orders;
-    if (!pgtable_has_pmd_leaves() || (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force)))
+    if (vma && vma_thp_disabled(vma, vm_flags, shmem_huge_force))
Quoted 8 lines

mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1950

Quoted 7 lines
mm/shmem.c +2 -0
          return 0;
      global_orders = shmem_huge_global_enabled(inode, index, write_end,
@@ -1947,6 +1947,8 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
      if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
          orders = 0;
+    else if (!pgtable_has_pmd_leaves())
+        orders &= ~(BIT(PMD_ORDER) | BIT(PUD_ORDER));

Sorry, I still don't like the changes here, because shmem_allowable_huge_orders() is meant to determine which large orders are allowed. Something like below (untested):

Sure, I can do something like this.

Quoted 39 lines
(quoted diff) +3 -1
@@ -1834,6 +1834,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
                                 struct vm_area_struct *vma, pgoff_t index,
                                 loff_t write_end, bool shmem_huge_force)
  {
+       unsigned int filter_orders = pgtable_has_pmd_leaves() ? (BIT(PMD_ORDER) | BIT(PUD_ORDER)) : 0;
         unsigned long mask = READ_ONCE(huge_shmem_orders_always);
         unsigned long within_size_orders = READ_ONCE(huge_shmem_orders_within_size);
         vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
@@ -1846,7 +1847,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
                                                   shmem_huge_force, vma, vm_flags);
         /* Tmpfs huge pages allocation */
         if (!vma || !vma_is_anon_shmem(vma))
-               return global_orders;
+               return global_orders & ~filter_orders;
 
         /*
          * Following the 'deny' semantics of the top level, force the huge
@@ -1871,6 +1872,7 @@ unsigned long shmem_allowable_huge_orders(struct inode *inode,
         if (global_orders > 0)
                 mask |= READ_ONCE(huge_shmem_orders_inherit);
 
+       mask &= ~filter_orders;
         return THP_ORDERS_ALL_FILE_DEFAULT & mask;
  }
 

Additionally, we also need a pgtable_has_pmd_leaves() check before setting huge_shmem_orders_inherit as well.

(quoted diff) +1 -1
@@ -5428,7 +5430,7 @@ void __init shmem_init(void)
          * Default to setting PMD-sized THP to inherit the global setting and
          * disable all other multi-size THPs.
          */
-       if (!shmem_orders_configured)
+       if (!shmem_orders_configured && pgtable_has_pmd_leaves())
                 huge_shmem_orders_inherit = BIT(HPAGE_PMD_ORDER);
  #endif
         return;
#41 lore raw

PATCH v3 10/10 mm: thp: x86: cleanup PSE feature bit usage

In reply to Luiz Capitulino

Historically, THP support on x86 checked the PSE feature bit to enable THP. On 64-bit, this check is redundant since PSE is always enabled by default for compatibility. On 32-bit, PSE can enable 2 MiB or 4 MiB page sizes so it must be checked. To clean this up, this commit:

1. Drops arch_has_pmd_leaves() from common x86 code. For 64-bit, we assume PMD-sized pages are always supported

2. Checks for PSE only on 32-bit by implementing arch_has_pmd_leaves()

---
 arch/x86/include/asm/pgtable.h    | 6 ------
 arch/x86/include/asm/pgtable_32.h | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)
arch/x86/include/asm/pgtable.h +0 -6
index 8fe95270b713..f45568f9513c 100644
@@ -314,12 +314,6 @@ static inline int pud_trans_huge(pud_t pud)
 }
 #endif
 
-#define arch_has_pmd_leaves arch_has_pmd_leaves
-static inline int arch_has_pmd_leaves(void)
-{
-	return boot_cpu_has(X86_FEATURE_PSE);
-}
-
 #ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
 static inline bool pmd_special(pmd_t pmd)
 {
arch/x86/include/asm/pgtable_32.h +6 -0
index acea0cfa2460..1db3214cfb45 100644
@@ -44,6 +44,12 @@ do {						\
 	flush_tlb_one_kernel((vaddr));		\
 } while (0)
 
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
+{
+	return boot_cpu_has(X86_FEATURE_PSE);
+}
+
 #endif /* !__ASSEMBLER__ */
 
 /*
Signature · 1 line
2.53.0
reviewer
#42 lore raw

Re: [PATCH v3 10/10] mm: thp: x86: cleanup PSE feature bit usage

+X86 maintainers

Quoted 52 lines · Luiz Capitulino

Historically, THP support on x86 checked the PSE feature bit to enable THP. On 64-bit, this check is redundant since PSE is always enabled by default for compatibility. On 32-bit, PSE can enable 2 MiB or 4 MiB page sizes so it must be checked. To clean this up, this commit:

1. Drops arch_has_pmd_leaves() from common x86 code. For 64-bit, we assume PMD-sized pages are always supported

2. Checks for PSE only on 32-bit by implementing arch_has_pmd_leaves()

---
 arch/x86/include/asm/pgtable.h    | 6 ------
 arch/x86/include/asm/pgtable_32.h | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)
arch/x86/include/asm/pgtable.h +0 -6
index 8fe95270b713..f45568f9513c 100644
@@ -314,12 +314,6 @@ static inline int pud_trans_huge(pud_t pud)
 }
 #endif
 
-#define arch_has_pmd_leaves arch_has_pmd_leaves
-static inline int arch_has_pmd_leaves(void)
-{
-	return boot_cpu_has(X86_FEATURE_PSE);
-}
-
 #ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
 static inline bool pmd_special(pmd_t pmd)
 {
arch/x86/include/asm/pgtable_32.h +6 -0
index acea0cfa2460..1db3214cfb45 100644
@@ -44,6 +44,12 @@ do {						\
 	flush_tlb_one_kernel((vaddr));		\
 } while (0)
 
+#define arch_has_pmd_leaves arch_has_pmd_leaves
+static inline int arch_has_pmd_leaves(void)
+{
+	return boot_cpu_has(X86_FEATURE_PSE);
+}
+
 #endif /* !__ASSEMBLER__ */
 
 /*
Signature · 1 line
2.53.0

Best Regards,
Yan, Zi

#43 lore raw

Re: [PATCH v3 10/10] mm: thp: x86: cleanup PSE feature bit usage

Quoted 3 lines · Zi Yan

Historically, THP support on x86 checked the PSE feature bit to enable THP. On 64-bit, this check is redundant since PSE is always enabled by default for compatibility.

What do you mean by "PSE is always enabled by default"?

A hypervisor is completely free to not enumerate PSE support to a guest. Any kernel can disable PSE with clearcpuid=3 on the command-line. PSE is also disabled for at least one erratum (which I think includes some 64-bit CPUs).

Here's a snippet in a 64-bit VM of /proc/cpuinfo on a normal boot:

DirectMap4k:       83836 kB
DirectMap2M:     1939456 kB
DirectMap1G:     1048576 kB

and with clearcpuid=3:

DirectMap4k:     3071868 kB
DirectMap2M:           0 kB

Just to show that it's possible.

So perhaps I'm reading into the changelog too much, but this doesn't quite seem right to me.

#44 lore raw

Re: [PATCH v3 10/10] mm: thp: x86: cleanup PSE feature bit usage

Quoted 11 lines · Dave Hansen
Quoted 3 lines · Zi Yan

Historically, THP support on x86 checked the PSE feature bit to enable THP. On 64-bit, this check is redundant since PSE is always enabled by default for compatibility.

What do you mean by "PSE is always enabled by default"?

A hypervisor is completely free to not enumerate PSE support to a guest. Any kernel can disable PSE with clearcpuid=3 on the command-line. PSE is also disabled for at least one erratum (which I think includes some 64-bit CPUs).

You're right, I was wrong to assume that PSE could be ignored on 64-bit. Thanks for taking the time to show it. I'll drop this patch for v4.

Quoted 17 lines

Here's a snippet in a 64-bit VM of /proc/cpuinfo on a normal boot:

DirectMap4k:       83836 kB
DirectMap2M:     1939456 kB
DirectMap1G:     1048576 kB

and with clearcpuid=3:

DirectMap4k:     3071868 kB
DirectMap2M:           0 kB

Just to show that it's possible.

So perhaps I'm reading into the changelog too much, but this doesn't quite seem right to me.

reviewer
#45 lore raw
In reply to Luiz Capitulino

+Cc THP

Some folks seem to be missing from CC for this series. Please run scripts/get_maintainer.pl; it's your friend :)

Thanks,
Lance

Quoted 80 lines · Luiz Capitulino

Today, if an architecture implements has_transparent_hugepage() and the CPU lacks support for PMD-sized pages, the THP code disables all THP, including mTHP. In addition, the kernel lacks a well defined API to check for PMD-sized page support. It currently relies on has_transparent_hugepage() and thp_disabled_by_hw(), but they are not well defined and are tied to THP support.

This series addresses both issues by introducing a new well defined API to query PMD-sized page support: pgtable_has_pmd_leaves(). Using this new helper, we ensure that mTHP remains enabled even when the architecture or CPU doesn't support PMD-sized pages.

Thanks to David Hildenbrand for suggesting this improvement and for providing guidance (all bugs and misconceptions are mine).

This applies to Linus tree bfe62a454542 ("Merge tag 'soc-fixes-7.0-2' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc").

v3
--

- Rebased on top of latest Linus tree
- Removed i915 patch as driver dropped has_transparent_hugepage() usage
- Moved init_arch_has_pmd_leaves() call in start_kernel() to avoid conflict with early_param handlers clearing CPU feature flags
- Fixed build error with CONFIG_MMU=n (kernel test robot)
- Fixed huge_anon_orders_inherit default setting when !pgtable_pmd_leaves() (Baolin)
- Small commit changelog improvements

v2
--
- Added support for always enabling mTHPs for shmem (Baolin)
- Improved commits changelog & added reviewed-by

v1
--
- Call init_arch_has_pmd_leaves() from start_kernel()
- Keep pgtable_has_pmd_leaves() calls tied to CONFIG_TRANSPARENT_HUGEPAGE (David)
- Clear PUD_ORDER when clearing PMD_ORDER (David)
- Small changelog improvements (David)
- Rebased on top of latest mm-new

Luiz Capitulino (10):
docs: tmpfs: remove implementation detail reference
mm: introduce pgtable_has_pmd_leaves()
drivers: dax: use pgtable_has_pmd_leaves()
drivers: nvdimm: use pgtable_has_pmd_leaves()
mm: debug_vm_pgtable: use pgtable_has_pmd_leaves()
mm: shmem: drop has_transparent_hugepage() usage
treewide: rename has_transparent_hugepage() to arch_has_pmd_leaves()
mm: replace thp_disabled_by_hw() with pgtable_has_pmd_leaves()
mm: thp: always enable mTHP support
mm: thp: x86: cleanup PSE feature bit usage

Documentation/filesystems/tmpfs.rst           |  5 ++---
arch/mips/include/asm/pgtable.h               |  4 ++--
arch/mips/mm/tlb-r4k.c                        |  4 ++--
arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
arch/powerpc/include/asm/book3s/64/pgtable.h  | 10 +++++-----
arch/powerpc/include/asm/book3s/64/radix.h    |  2 +-
arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
arch/s390/include/asm/pgtable.h               |  4 ++--
arch/x86/include/asm/pgtable.h                |  6 ------
arch/x86/include/asm/pgtable_32.h             |  6 ++++++
drivers/dax/dax-private.h                     |  2 +-
drivers/nvdimm/pfn_devs.c                     |  6 ++++--
include/linux/huge_mm.h                       |  7 -------
include/linux/pgtable.h                       | 19 ++++++++++++++++--
init/main.c                                   |  1 +
mm/debug_vm_pgtable.c                         | 20 +++++++++----------
mm/huge_memory.c                              | 15 +++++++-------
mm/memory.c                                   | 10 +++++++++-
mm/shmem.c                                    | 11 +++++-----
20 files changed, 80 insertions(+), 60 deletions(-)
Signature · 1 line
2.53.0
#46 lore raw
Quoted 4 lines · Lance Yang

+Cc THP

Some folks seem to be missing from CC for this series. Please run scripts/get_maintainer.pl; it's your friend :)

Please also use [email protected] for me, I'm now ignoring mails that are sent to my old address, so you won't get replies if you send it to the wrong one :)

(My upstream bandwidth is dramatically reduced and I can't justify checking both places.)

I will try to take a look at this when I get time, but really this is something to queue up for next cycle clearly at this point!

I really do appreciate work that pays down technical debt however, so just to be clear, I'm glad you're doing that :>)

Cheers, Lorenzo

#47 lore raw
Quoted 8 lines · Lorenzo Stoakes
Quoted 4 lines · Lance Yang

+Cc THP

Some folks seem to be missing from CC for this series. Please run scripts/get_maintainer.pl; it's your friend :)

Please also use [email protected] for me, I'm now ignoring mails that are sent to my old address, so you won't get replies if you send it to the wrong one :)

Thanks Lance and Lorenzo, I'll make sure to CC the right people if I need to post a new version.

Quoted 12 lines

(My upstream bandwidth is dramatically reduced and I can't justify checking both places.)

I will try to take a look at this when I get time, but really this is something to queue up for next cycle clearly at this point!

I really do appreciate work that pays down technical debt however, so just to be clear, I'm glad you're doing that :>)

Cheers, Lorenzo

reviewer
#48 lore raw
In reply to Lance Yang
Quoted 4 lines · Lance Yang

+Cc THP

Some folks seem to be missing from CC for this series. Please run scripts/get_maintainer.pl; it's your friend :)

Ouch... just realized I missed one more. +Cc Barry.