[PATCH v2 0/4] mm: add more kernel parameters to control mTHP

[PATCH v2 0/4] mm: add more kernel parameters to control mTHP

[Maíra Canal]

This series introduces three patches related to the kernel parameters
controlling mTHP. The first patch is a straightforward documentation update,
correcting the format of the kernel parameter ``thp_anon=``.

The second and third patches focus on controlling THP support for shmem
via the kernel command line. The second patch introduces a parameter to
control the global default huge page allocation policy for the internal
shmem mount. The third patch implements a parameter similar to ``thp_anon=``,
but for shmem.

The goal of these changes is to simplify the configuration of systems that
rely on mTHP support for shmem. For instance, a platform with a GPU that
benefits from huge pages may want to enable huge pages for shmem. Having
these kernel parameters streamlines the configuration process and ensures
consistency across setups.

v1 -> v2: https://lore.kernel.org/linux-mm/20241027175743.1056710-1-mcanal@igalia.com/T/

* [1/4] s/fix the format/fix the doc in the commit's subject (Barry Song & David Hildenbrand)
* [1/4] Add Barry's A-b to PATCH 1/3 (Barry Song)
* [1/4] s/64KB/64K (David Hildenbrand)
* [1/4] Add David's A-b to PATCH 1/3 (David Hildenbrand)
* [2/4] Create the function `shmem_valid_huge()` to reduce code-duplication (Baolin Wang)
* [3/4] New PATCH: generalize the function `setup_thp_anon()` and add it to common file
* [4/4] Fix typo in the documentation: s/shmem_anon/thp_shmem (Barry Song)
* [4/4] Reduce code-duplication (Barry Song)

Best Regards,
- Maíra

Maíra Canal (4):
  mm: fix docs for the kernel parameter ``thp_anon=``
  mm: shmem: control THP support through the kernel command line
  mm: generalize the implementation of ``thp_anon=``
  mm: shmem: override mTHP shmem default with a kernel parameter

 .../admin-guide/kernel-parameters.txt         |  19 ++-
 Documentation/admin-guide/mm/transhuge.rst    |  25 +++-
 include/linux/huge_mm.h                       |  29 +++++
 mm/huge_memory.c                              | 109 ++++++++----------
 mm/shmem.c                                    |  91 +++++++++++++--
 5 files changed, 201 insertions(+), 72 deletions(-)

-- 
2.46.2

[PATCH v2 1/4] mm: fix docs for the kernel parameter ``thp_anon=``

[Maíra Canal]

If we add ``thp_anon=32,64K:always`` to the kernel command line, we
will see the following error:

[    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting

This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
as [KMG] must follow each number to especify its unit. So, the correct
format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.

Therefore, adjust the documentation to reflect the correct format of the
parameter ``thp_anon=``.

Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Acked-by: Barry Song <baohua@kernel.org>
Acked-by: David Hildenbrand <david@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 2 +-
 Documentation/admin-guide/mm/transhuge.rst      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1518343bbe22..1666576acc0e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6688,7 +6688,7 @@
 			0: no polling (default)
 
 	thp_anon=	[KNL]
-			Format: <size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>
+			Format: <size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>
 			state is one of "always", "madvise", "never" or "inherit".
 			Control the default behavior of the system with respect
 			to anonymous transparent hugepages.
diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
index 203ba7aaf5fc..745055c3dc09 100644
--- a/Documentation/admin-guide/mm/transhuge.rst
+++ b/Documentation/admin-guide/mm/transhuge.rst
@@ -303,7 +303,7 @@ control by passing the parameter ``transparent_hugepage=always`` or
 kernel command line.
 
 Alternatively, each supported anonymous THP size can be controlled by
-passing ``thp_anon=<size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>``,
+passing ``thp_anon=<size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>``,
 where ``<size>`` is the THP size (must be a power of 2 of PAGE_SIZE and
 supported anonymous THP)  and ``<state>`` is one of ``always``, ``madvise``,
 ``never`` or ``inherit``.
-- 
2.46.2

[PATCH v2 2/4] mm: shmem: control THP support through the kernel command line

[Maíra Canal]

Add a new kernel command line to control the hugepage allocation policy
for the internal shmem mount, ``transparent_hugepage_shmem``. The
parameter is similar to ``transparent_hugepage`` and has the following
format:

transparent_hugepage_shmem=<policy>

where ``<policy>`` is one of the seven valid policies available for
shmem.

By configuring the default hugepage allocation policy for the internal
shmem mount, applications that use shmem, such as the DRM GEM objects,
can take advantage of mTHP before it's been configured through sysfs.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 .../admin-guide/kernel-parameters.txt         |  7 +++
 Documentation/admin-guide/mm/transhuge.rst    |  6 +++
 mm/shmem.c                                    | 53 +++++++++++++++----
 3 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1666576acc0e..acabb04d0dd4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6926,6 +6926,13 @@
 			See Documentation/admin-guide/mm/transhuge.rst
 			for more details.
 
+	transparent_hugepage_shmem= [KNL]
+			Format: [always|within_size|advise|never|deny|force]
+			Can be used to control the hugepage allocation policy for
+			the internal shmem mount.
+			See Documentation/admin-guide/mm/transhuge.rst
+			for more details.
+
 	trusted.source=	[KEYS]
 			Format: <string>
 			This parameter identifies the trust source as a backend
diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
index 745055c3dc09..9b5b02c4d1ab 100644
--- a/Documentation/admin-guide/mm/transhuge.rst
+++ b/Documentation/admin-guide/mm/transhuge.rst
@@ -326,6 +326,12 @@ PMD_ORDER THP policy will be overridden. If the policy for PMD_ORDER
 is not defined within a valid ``thp_anon``, its policy will default to
 ``never``.
 
+Similarly to ``transparent_hugepage``, you can control the hugepage
+allocation policy for the internal shmem mount by using the kernel parameter
+``transparent_hugepage_shmem=<policy>``, where ``<policy>`` is one of the
+seven valid policies for shmem (``always``, ``within_size``, ``advise``,
+``never``, ``deny``, and ``force``).
+
 Hugepages in tmpfs/shmem
 ========================
 
diff --git a/mm/shmem.c b/mm/shmem.c
index 44282a296c33..26c1eb1b4b1d 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -582,7 +582,6 @@ static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
 	}
 }
 
-#if defined(CONFIG_SYSFS)
 static int shmem_parse_huge(const char *str)
 {
 	if (!strcmp(str, "never"))
@@ -599,7 +598,6 @@ static int shmem_parse_huge(const char *str)
 		return SHMEM_HUGE_FORCE;
 	return -EINVAL;
 }
-#endif
 
 #if defined(CONFIG_SYSFS) || defined(CONFIG_TMPFS)
 static const char *shmem_format_huge(int huge)
@@ -624,6 +622,20 @@ static const char *shmem_format_huge(int huge)
 }
 #endif
 
+static int shmem_valid_huge(int huge)
+{
+	if (!has_transparent_hugepage() &&
+			huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
+		return -EINVAL;
+
+	/* Do not override huge allocation policy with non-PMD sized mTHP */
+	if (huge == SHMEM_HUGE_FORCE &&
+	    huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
+		return -EINVAL;
+
+	return 0;
+}
+
 static unsigned long shmem_unused_huge_shrink(struct shmem_sb_info *sbinfo,
 		struct shrink_control *sc, unsigned long nr_to_free)
 {
@@ -5070,14 +5082,10 @@ static ssize_t shmem_enabled_store(struct kobject *kobj,
 	huge = shmem_parse_huge(tmp);
 	if (huge == -EINVAL)
 		return -EINVAL;
-	if (!has_transparent_hugepage() &&
-			huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
-		return -EINVAL;
 
-	/* Do not override huge allocation policy with non-PMD sized mTHP */
-	if (huge == SHMEM_HUGE_FORCE &&
-	    huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
-		return -EINVAL;
+	err = shmem_valid_huge(huge);
+	if (err)
+		return err;
 
 	shmem_huge = huge;
 	if (shmem_huge > SHMEM_HUGE_DENY)
@@ -5174,6 +5182,33 @@ struct kobj_attribute thpsize_shmem_enabled_attr =
 	__ATTR(shmem_enabled, 0644, thpsize_shmem_enabled_show, thpsize_shmem_enabled_store);
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE && CONFIG_SYSFS */
 
+#if defined(CONFIG_TRANSPARENT_HUGEPAGE)
+
+static int __init setup_transparent_hugepage_shmem(char *str)
+{
+	int huge, ret = 0;
+
+	if (!str)
+		goto out;
+
+	huge = shmem_parse_huge(str);
+	if (huge == -EINVAL)
+		goto out;
+
+	ret = shmem_valid_huge(huge);
+	if (ret)
+		goto out;
+
+	shmem_huge = huge;
+	return 1;
+out:
+	pr_warn("transparent_hugepage_shmem= cannot parse, ignored\n");
+	return ret;
+}
+__setup("transparent_hugepage_shmem=", setup_transparent_hugepage_shmem);
+
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+
 #else /* !CONFIG_SHMEM */
 
 /*
-- 
2.46.2

[PATCH v2 3/4] mm: generalize the implementation of ``thp_anon=``

[Maíra Canal]

Currently, the function ``setup_thp_anon()`` is extremely tied to the
policies and bitmaps that currently exist for anon THP. This means that
we can't reuse the function if we implement a ``thp_shmem=``.

This commit extracts the behavior of the function to a new generic
function. The new function is exposed in common headers for future use
by mm/shmem.c.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 include/linux/huge_mm.h |  29 +++++++++++
 mm/huge_memory.c        | 109 ++++++++++++++++++----------------------
 2 files changed, 78 insertions(+), 60 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index b94c2e8ee918..b82e9379f2bf 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -471,6 +471,35 @@ void split_huge_pmd_locked(struct vm_area_struct *vma, unsigned long address,
 bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
 			   pmd_t *pmdp, struct folio *folio);
 
+static inline int get_order_from_str(const char *size_str,
+				     const unsigned long thp_orders)
+{
+	unsigned long size;
+	char *endptr;
+	int order;
+
+	size = memparse(size_str, &endptr);
+
+	if (!is_power_of_2(size))
+		goto err;
+
+	order = get_order(size);
+	if (BIT(order) & ~thp_orders)
+		goto err;
+
+	return order;
+err:
+	pr_err("invalid size %s in boot parameter\n", size_str);
+	return -EINVAL;
+}
+
+struct thp_policy_bitmap {
+	const char *policy;
+	unsigned long bitmap;
+};
+
+int parse_huge_orders(char *p, struct thp_policy_bitmap *policies,
+		      const int num_policies, const unsigned long thp_orders);
 #else /* CONFIG_TRANSPARENT_HUGEPAGE */
 
 static inline bool folio_test_pmd_mappable(struct folio *folio)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 832ca761b4c3..c61f4481cb6a 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -958,91 +958,80 @@ static int __init setup_transparent_hugepage(char *str)
 }
 __setup("transparent_hugepage=", setup_transparent_hugepage);
 
-static inline int get_order_from_str(const char *size_str)
-{
-	unsigned long size;
-	char *endptr;
-	int order;
-
-	size = memparse(size_str, &endptr);
-
-	if (!is_power_of_2(size))
-		goto err;
-	order = get_order(size);
-	if (BIT(order) & ~THP_ORDERS_ALL_ANON)
-		goto err;
-
-	return order;
-err:
-	pr_err("invalid size %s in thp_anon boot parameter\n", size_str);
-	return -EINVAL;
-}
-
-static char str_dup[PAGE_SIZE] __initdata;
-static int __init setup_thp_anon(char *str)
+int parse_huge_orders(char *p, struct thp_policy_bitmap *policies,
+		      const int num_policies, const unsigned long thp_orders)
 {
 	char *token, *range, *policy, *subtoken;
-	unsigned long always, inherit, madvise;
 	char *start_size, *end_size;
-	int start, end, nr;
-	char *p;
+	int start, end, nr, i;
+	bool policy_set;
 
-	if (!str || strlen(str) + 1 > PAGE_SIZE)
-		goto err;
-	strcpy(str_dup, str);
-
-	always = huge_anon_orders_always;
-	madvise = huge_anon_orders_madvise;
-	inherit = huge_anon_orders_inherit;
-	p = str_dup;
 	while ((token = strsep(&p, ";")) != NULL) {
 		range = strsep(&token, ":");
 		policy = token;
 
 		if (!policy)
-			goto err;
+			return 0;
 
 		while ((subtoken = strsep(&range, ",")) != NULL) {
+			policy_set = false;
+
 			if (strchr(subtoken, '-')) {
 				start_size = strsep(&subtoken, "-");
 				end_size = subtoken;
 
-				start = get_order_from_str(start_size);
-				end = get_order_from_str(end_size);
+				start = get_order_from_str(start_size, thp_orders);
+				end = get_order_from_str(end_size, thp_orders);
 			} else {
-				start = end = get_order_from_str(subtoken);
+				start = end = get_order_from_str(subtoken, thp_orders);
 			}
 
 			if (start < 0 || end < 0 || start > end)
-				goto err;
+				return 0;
 
 			nr = end - start + 1;
-			if (!strcmp(policy, "always")) {
-				bitmap_set(&always, start, nr);
-				bitmap_clear(&inherit, start, nr);
-				bitmap_clear(&madvise, start, nr);
-			} else if (!strcmp(policy, "madvise")) {
-				bitmap_set(&madvise, start, nr);
-				bitmap_clear(&inherit, start, nr);
-				bitmap_clear(&always, start, nr);
-			} else if (!strcmp(policy, "inherit")) {
-				bitmap_set(&inherit, start, nr);
-				bitmap_clear(&madvise, start, nr);
-				bitmap_clear(&always, start, nr);
-			} else if (!strcmp(policy, "never")) {
-				bitmap_clear(&inherit, start, nr);
-				bitmap_clear(&madvise, start, nr);
-				bitmap_clear(&always, start, nr);
-			} else {
-				pr_err("invalid policy %s in thp_anon boot parameter\n", policy);
-				goto err;
+
+			for (i = 0; i < num_policies; i++) {
+				if (!strcmp(policy, policies[i].policy)) {
+					bitmap_set(&policies[i].bitmap, start, nr);
+					policy_set = true;
+				} else
+					bitmap_clear(&policies[i].bitmap, start, nr);
+			}
+
+			if (!policy_set && strcmp(policy, "never")) {
+				pr_err("invalid policy %s in boot parameter\n", policy);
+				return 0;
 			}
 		}
 	}
 
-	huge_anon_orders_always = always;
-	huge_anon_orders_madvise = madvise;
-	huge_anon_orders_inherit = inherit;
+	return 1;
+}
+
+static char str_dup[PAGE_SIZE] __initdata;
+static int __init setup_thp_anon(char *str)
+{
+	struct thp_policy_bitmap policies[] = {
+		{ "always",  huge_anon_orders_always },
+		{ "madvise",  huge_anon_orders_madvise },
+		{ "inherit",  huge_anon_orders_inherit },
+	};
+	char *p;
+
+	if (!str || strlen(str) + 1 > PAGE_SIZE)
+		goto err;
+
+	strscpy(str_dup, str);
+	p = str_dup;
+
+	if (!parse_huge_orders(p, policies, ARRAY_SIZE(policies),
+			      THP_ORDERS_ALL_ANON))
+		goto err;
+
+	huge_anon_orders_always = policies[0].bitmap;
+	huge_anon_orders_madvise = policies[1].bitmap;
+	huge_anon_orders_inherit = policies[2].bitmap;
 	anon_orders_configured = true;
 	return 1;
 
-- 
2.46.2

[PATCH v2 4/4] mm: shmem: override mTHP shmem default with a kernel parameter

[Maíra Canal]

Add the ``thp_shmem=`` kernel command line to allow specifying the
default policy of each supported shmem hugepage size. The kernel parameter
accepts the following format:

  thp_shmem=<size>[KMG],<size>[KMG]:<policy>;<size>[KMG]-<size>[KMG]:<policy>

For example,

  thp_shmem=16K-64K:always;128K,512K:inherit;256K:advise;1M-2M:never;4M-8M:within_size

By configuring the default policy of several shmem hugepages, the user
can take advantage of mTHP before it's been configured through sysfs.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 .../admin-guide/kernel-parameters.txt         | 10 +++++
 Documentation/admin-guide/mm/transhuge.rst    | 17 +++++++++
 mm/shmem.c                                    | 38 ++++++++++++++++++-
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index acabb04d0dd4..b48d744d99b0 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6700,6 +6700,16 @@
 			Force threading of all interrupt handlers except those
 			marked explicitly IRQF_NO_THREAD.
 
+	thp_shmem=	[KNL]
+			Format: <size>[KMG],<size>[KMG]:<policy>;<size>[KMG]-<size>[KMG]:<policy>
+			Control the default policy of each hugepage size for the
+			internal shmem mount. <policy> is one of policies available
+			for the shmem mount ("always", "inherit", "never", "within_size",
+			and "advise").
+			It can be used multiple times for multiple shmem THP sizes.
+			See Documentation/admin-guide/mm/transhuge.rst for more
+			details.
+
 	topology=	[S390,EARLY]
 			Format: {off | on}
 			Specify if the kernel should make use of the cpu
diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
index 9b5b02c4d1ab..47e7fc30e22d 100644
--- a/Documentation/admin-guide/mm/transhuge.rst
+++ b/Documentation/admin-guide/mm/transhuge.rst
@@ -332,6 +332,23 @@ allocation policy for the internal shmem mount by using the kernel parameter
 seven valid policies for shmem (``always``, ``within_size``, ``advise``,
 ``never``, ``deny``, and ``force``).
 
+In the same manner as ``thp_anon`` controls each supported anonymous THP
+size, ``thp_shmem`` controls each supported shmem THP size. ``thp_shmem``
+has the same format as ``thp_anon``, but also supports the policy
+``within_size``.
+
+``thp_shmem=`` may be specified multiple times to configure all THP sizes
+as required. If ``thp_shmem=`` is specified at least once, any shmem THP
+sizes not explicitly configured on the command line are implicitly set to
+``never``.
+
+``transparent_hugepage_shmem`` setting only affects the global toggle. If
+``thp_shmem`` is not specified, PMD_ORDER hugepage will default to
+``inherit``. However, if a valid ``thp_shmem`` setting is provided by the
+user, the PMD_ORDER hugepage policy will be overridden. If the policy for
+PMD_ORDER is not defined within a valid ``thp_shmem``, its policy will
+default to ``never``.
+
 Hugepages in tmpfs/shmem
 ========================
 
diff --git a/mm/shmem.c b/mm/shmem.c
index 26c1eb1b4b1d..37951bb863dd 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -136,6 +136,7 @@ static unsigned long huge_shmem_orders_always __read_mostly;
 static unsigned long huge_shmem_orders_madvise __read_mostly;
 static unsigned long huge_shmem_orders_inherit __read_mostly;
 static unsigned long huge_shmem_orders_within_size __read_mostly;
+static bool shmem_orders_configured __initdata;
 #endif
 
 #ifdef CONFIG_TMPFS
@@ -5027,7 +5028,8 @@ void __init shmem_init(void)
 	 * Default to setting PMD-sized THP to inherit the global setting and
 	 * disable all other multi-size THPs.
 	 */
-	huge_shmem_orders_inherit = BIT(HPAGE_PMD_ORDER);
+	if (!shmem_orders_configured)
+		huge_shmem_orders_inherit = BIT(HPAGE_PMD_ORDER);
 #endif
 	return;
 
@@ -5184,6 +5186,7 @@ struct kobj_attribute thpsize_shmem_enabled_attr =
 
 #if defined(CONFIG_TRANSPARENT_HUGEPAGE)
 
+
 static int __init setup_transparent_hugepage_shmem(char *str)
 {
 	int huge, ret = 0;
@@ -5207,6 +5210,39 @@ static int __init setup_transparent_hugepage_shmem(char *str)
 }
 __setup("transparent_hugepage_shmem=", setup_transparent_hugepage_shmem);
 
+static char str_dup[PAGE_SIZE] __initdata;
+static int __init setup_thp_shmem(char *str)
+{
+	struct thp_policy_bitmap policies[] = {
+		{ "always",  huge_shmem_orders_always },
+		{ "inherit",  huge_shmem_orders_inherit },
+		{ "advise",  huge_shmem_orders_madvise },
+		{ "within_size",  huge_shmem_orders_within_size }
+	};
+	char *p;
+
+	if (!str || strlen(str) + 1 > PAGE_SIZE)
+		goto err;
+
+	strscpy(str_dup, str);
+	p = str_dup;
+
+	if (!parse_huge_orders(p, policies, ARRAY_SIZE(policies),
+			      THP_ORDERS_ALL_FILE_DEFAULT))
+		goto err;
+
+	huge_shmem_orders_always = policies[0].bitmap;
+	huge_shmem_orders_inherit = policies[1].bitmap;
+	huge_shmem_orders_madvise = policies[2].bitmap;
+	huge_shmem_orders_within_size = policies[3].bitmap;
+	shmem_orders_configured = true;
+	return 1;
+err:
+	pr_warn("thp_shmem=%s: error parsing string, ignoring setting\n", str);
+	return 0;
+}
+__setup("thp_shmem=", setup_thp_shmem);
+
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
 #else /* !CONFIG_SHMEM */
-- 
2.46.2

Re: [PATCH v2 1/4] mm: fix docs for the kernel parameter ``thp_anon=``

[Barry Song]

On Tue, Oct 29, 2024 at 8:24 AM Maíra Canal <mcanal@igalia.com> wrote:
>
> If we add ``thp_anon=32,64K:always`` to the kernel command line, we
> will see the following error:
>
> [    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting
>
> This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
> as [KMG] must follow each number to especify its unit. So, the correct
> format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.
>
> Therefore, adjust the documentation to reflect the correct format of the
> parameter ``thp_anon=``.
>
> Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> Acked-by: Barry Song <baohua@kernel.org>
> Acked-by: David Hildenbrand <david@redhat.com>

Can we separate this and apply it to v6.12-rc? If Andrew doesn't require a new
version for the separation, can we extract it from this series and
apply it to mm?

> ---
>  Documentation/admin-guide/kernel-parameters.txt | 2 +-
>  Documentation/admin-guide/mm/transhuge.rst      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1518343bbe22..1666576acc0e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6688,7 +6688,7 @@
>                         0: no polling (default)
>
>         thp_anon=       [KNL]
> -                       Format: <size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>
> +                       Format: <size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>
>                         state is one of "always", "madvise", "never" or "inherit".
>                         Control the default behavior of the system with respect
>                         to anonymous transparent hugepages.
> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
> index 203ba7aaf5fc..745055c3dc09 100644
> --- a/Documentation/admin-guide/mm/transhuge.rst
> +++ b/Documentation/admin-guide/mm/transhuge.rst
> @@ -303,7 +303,7 @@ control by passing the parameter ``transparent_hugepage=always`` or
>  kernel command line.
>
>  Alternatively, each supported anonymous THP size can be controlled by
> -passing ``thp_anon=<size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>``,
> +passing ``thp_anon=<size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>``,
>  where ``<size>`` is the THP size (must be a power of 2 of PAGE_SIZE and
>  supported anonymous THP)  and ``<state>`` is one of ``always``, ``madvise``,
>  ``never`` or ``inherit``.
> --
> 2.46.2
>

Thanks
Barry

Re: [PATCH v2 3/4] mm: generalize the implementation of ``thp_anon=``

[Barry Song]

On Tue, Oct 29, 2024 at 8:24 AM Maíra Canal <mcanal@igalia.com> wrote:
>
> Currently, the function ``setup_thp_anon()`` is extremely tied to the
> policies and bitmaps that currently exist for anon THP. This means that
> we can't reuse the function if we implement a ``thp_shmem=``.
>
> This commit extracts the behavior of the function to a new generic
> function. The new function is exposed in common headers for future use
> by mm/shmem.c.
>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>

Sorry, Maíra, my mistake. I don't see much value in making
get_order_from_str() a common
API since bootcmd such a small thing. I'd prefer to stick with your
previous code with
just a rename—unless, at some point, we move both shmem and anon controls into
mm/huge_memory.c.

> ---
>  include/linux/huge_mm.h |  29 +++++++++++
>  mm/huge_memory.c        | 109 ++++++++++++++++++----------------------
>  2 files changed, 78 insertions(+), 60 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index b94c2e8ee918..b82e9379f2bf 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -471,6 +471,35 @@ void split_huge_pmd_locked(struct vm_area_struct *vma, unsigned long address,
>  bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>                            pmd_t *pmdp, struct folio *folio);
>
> +static inline int get_order_from_str(const char *size_str,
> +                                    const unsigned long thp_orders)
> +{
> +       unsigned long size;
> +       char *endptr;
> +       int order;
> +
> +       size = memparse(size_str, &endptr);
> +
> +       if (!is_power_of_2(size))
> +               goto err;
> +
> +       order = get_order(size);
> +       if (BIT(order) & ~thp_orders)
> +               goto err;
> +
> +       return order;
> +err:
> +       pr_err("invalid size %s in boot parameter\n", size_str);
> +       return -EINVAL;
> +}
> +
> +struct thp_policy_bitmap {
> +       const char *policy;
> +       unsigned long bitmap;
> +};
> +
> +int parse_huge_orders(char *p, struct thp_policy_bitmap *policies,
> +                     const int num_policies, const unsigned long thp_orders);
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>
>  static inline bool folio_test_pmd_mappable(struct folio *folio)
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 832ca761b4c3..c61f4481cb6a 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -958,91 +958,80 @@ static int __init setup_transparent_hugepage(char *str)
>  }
>  __setup("transparent_hugepage=", setup_transparent_hugepage);
>
> -static inline int get_order_from_str(const char *size_str)
> -{
> -       unsigned long size;
> -       char *endptr;
> -       int order;
> -
> -       size = memparse(size_str, &endptr);
> -
> -       if (!is_power_of_2(size))
> -               goto err;
> -       order = get_order(size);
> -       if (BIT(order) & ~THP_ORDERS_ALL_ANON)
> -               goto err;
> -
> -       return order;
> -err:
> -       pr_err("invalid size %s in thp_anon boot parameter\n", size_str);
> -       return -EINVAL;
> -}
> -
> -static char str_dup[PAGE_SIZE] __initdata;
> -static int __init setup_thp_anon(char *str)
> +int parse_huge_orders(char *p, struct thp_policy_bitmap *policies,
> +                     const int num_policies, const unsigned long thp_orders)
>  {
>         char *token, *range, *policy, *subtoken;
> -       unsigned long always, inherit, madvise;
>         char *start_size, *end_size;
> -       int start, end, nr;
> -       char *p;
> +       int start, end, nr, i;
> +       bool policy_set;
>
> -       if (!str || strlen(str) + 1 > PAGE_SIZE)
> -               goto err;
> -       strcpy(str_dup, str);
> -
> -       always = huge_anon_orders_always;
> -       madvise = huge_anon_orders_madvise;
> -       inherit = huge_anon_orders_inherit;
> -       p = str_dup;
>         while ((token = strsep(&p, ";")) != NULL) {
>                 range = strsep(&token, ":");
>                 policy = token;
>
>                 if (!policy)
> -                       goto err;
> +                       return 0;
>
>                 while ((subtoken = strsep(&range, ",")) != NULL) {
> +                       policy_set = false;
> +
>                         if (strchr(subtoken, '-')) {
>                                 start_size = strsep(&subtoken, "-");
>                                 end_size = subtoken;
>
> -                               start = get_order_from_str(start_size);
> -                               end = get_order_from_str(end_size);
> +                               start = get_order_from_str(start_size, thp_orders);
> +                               end = get_order_from_str(end_size, thp_orders);
>                         } else {
> -                               start = end = get_order_from_str(subtoken);
> +                               start = end = get_order_from_str(subtoken, thp_orders);
>                         }
>
>                         if (start < 0 || end < 0 || start > end)
> -                               goto err;
> +                               return 0;
>
>                         nr = end - start + 1;
> -                       if (!strcmp(policy, "always")) {
> -                               bitmap_set(&always, start, nr);
> -                               bitmap_clear(&inherit, start, nr);
> -                               bitmap_clear(&madvise, start, nr);
> -                       } else if (!strcmp(policy, "madvise")) {
> -                               bitmap_set(&madvise, start, nr);
> -                               bitmap_clear(&inherit, start, nr);
> -                               bitmap_clear(&always, start, nr);
> -                       } else if (!strcmp(policy, "inherit")) {
> -                               bitmap_set(&inherit, start, nr);
> -                               bitmap_clear(&madvise, start, nr);
> -                               bitmap_clear(&always, start, nr);
> -                       } else if (!strcmp(policy, "never")) {
> -                               bitmap_clear(&inherit, start, nr);
> -                               bitmap_clear(&madvise, start, nr);
> -                               bitmap_clear(&always, start, nr);
> -                       } else {
> -                               pr_err("invalid policy %s in thp_anon boot parameter\n", policy);
> -                               goto err;
> +
> +                       for (i = 0; i < num_policies; i++) {
> +                               if (!strcmp(policy, policies[i].policy)) {
> +                                       bitmap_set(&policies[i].bitmap, start, nr);
> +                                       policy_set = true;
> +                               } else
> +                                       bitmap_clear(&policies[i].bitmap, start, nr);
> +                       }
> +
> +                       if (!policy_set && strcmp(policy, "never")) {
> +                               pr_err("invalid policy %s in boot parameter\n", policy);
> +                               return 0;
>                         }
>                 }
>         }
>
> -       huge_anon_orders_always = always;
> -       huge_anon_orders_madvise = madvise;
> -       huge_anon_orders_inherit = inherit;
> +       return 1;
> +}
> +
> +static char str_dup[PAGE_SIZE] __initdata;
> +static int __init setup_thp_anon(char *str)
> +{
> +       struct thp_policy_bitmap policies[] = {
> +               { "always",  huge_anon_orders_always },
> +               { "madvise",  huge_anon_orders_madvise },
> +               { "inherit",  huge_anon_orders_inherit },
> +       };
> +       char *p;
> +
> +       if (!str || strlen(str) + 1 > PAGE_SIZE)
> +               goto err;
> +
> +       strscpy(str_dup, str);
> +       p = str_dup;
> +
> +       if (!parse_huge_orders(p, policies, ARRAY_SIZE(policies),
> +                             THP_ORDERS_ALL_ANON))
> +               goto err;
> +
> +       huge_anon_orders_always = policies[0].bitmap;
> +       huge_anon_orders_madvise = policies[1].bitmap;
> +       huge_anon_orders_inherit = policies[2].bitmap;
>         anon_orders_configured = true;
>         return 1;
>
> --
> 2.46.2
>

Thanks
Barry

Re: [PATCH v2 2/4] mm: shmem: control THP support through the kernel command line

[Baolin Wang]

On 2024/10/29 08:13, Maíra Canal wrote:
> Add a new kernel command line to control the hugepage allocation policy
> for the internal shmem mount, ``transparent_hugepage_shmem``. The
> parameter is similar to ``transparent_hugepage`` and has the following
> format:
> 
> transparent_hugepage_shmem=<policy>
> 
> where ``<policy>`` is one of the seven valid policies available for
> shmem.
> 
> By configuring the default hugepage allocation policy for the internal
> shmem mount, applications that use shmem, such as the DRM GEM objects,
> can take advantage of mTHP before it's been configured through sysfs.

Just out of curiosity, do you have any performance benefit data when 
using mTHP for DRM GEM objects?

> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>   .../admin-guide/kernel-parameters.txt         |  7 +++
>   Documentation/admin-guide/mm/transhuge.rst    |  6 +++
>   mm/shmem.c                                    | 53 +++++++++++++++----
>   3 files changed, 57 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1666576acc0e..acabb04d0dd4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6926,6 +6926,13 @@
>   			See Documentation/admin-guide/mm/transhuge.rst
>   			for more details.
>   
> +	transparent_hugepage_shmem= [KNL]
> +			Format: [always|within_size|advise|never|deny|force]
> +			Can be used to control the hugepage allocation policy for
> +			the internal shmem mount.
> +			See Documentation/admin-guide/mm/transhuge.rst
> +			for more details.
> +
>   	trusted.source=	[KEYS]
>   			Format: <string>
>   			This parameter identifies the trust source as a backend
> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
> index 745055c3dc09..9b5b02c4d1ab 100644
> --- a/Documentation/admin-guide/mm/transhuge.rst
> +++ b/Documentation/admin-guide/mm/transhuge.rst
> @@ -326,6 +326,12 @@ PMD_ORDER THP policy will be overridden. If the policy for PMD_ORDER
>   is not defined within a valid ``thp_anon``, its policy will default to
>   ``never``.
>   
> +Similarly to ``transparent_hugepage``, you can control the hugepage
> +allocation policy for the internal shmem mount by using the kernel parameter
> +``transparent_hugepage_shmem=<policy>``, where ``<policy>`` is one of the
> +seven valid policies for shmem (``always``, ``within_size``, ``advise``,
> +``never``, ``deny``, and ``force``).
> +
>   Hugepages in tmpfs/shmem
>   ========================
>   
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 44282a296c33..26c1eb1b4b1d 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -582,7 +582,6 @@ static bool shmem_huge_global_enabled(struct inode *inode, pgoff_t index,
>   	}
>   }
>   
> -#if defined(CONFIG_SYSFS)
>   static int shmem_parse_huge(const char *str)
>   {
>   	if (!strcmp(str, "never"))
> @@ -599,7 +598,6 @@ static int shmem_parse_huge(const char *str)
>   		return SHMEM_HUGE_FORCE;
>   	return -EINVAL;
>   }
> -#endif
>   
>   #if defined(CONFIG_SYSFS) || defined(CONFIG_TMPFS)
>   static const char *shmem_format_huge(int huge)
> @@ -624,6 +622,20 @@ static const char *shmem_format_huge(int huge)
>   }
>   #endif
>   
> +static int shmem_valid_huge(int huge)
> +{
> +	if (!has_transparent_hugepage() &&
> +			huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
> +		return -EINVAL;
> +
> +	/* Do not override huge allocation policy with non-PMD sized mTHP */
> +	if (huge == SHMEM_HUGE_FORCE &&
> +	    huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>   static unsigned long shmem_unused_huge_shrink(struct shmem_sb_info *sbinfo,
>   		struct shrink_control *sc, unsigned long nr_to_free)
>   {
> @@ -5070,14 +5082,10 @@ static ssize_t shmem_enabled_store(struct kobject *kobj,
>   	huge = shmem_parse_huge(tmp);
>   	if (huge == -EINVAL)
>   		return -EINVAL;
> -	if (!has_transparent_hugepage() &&
> -			huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
> -		return -EINVAL;
>   
> -	/* Do not override huge allocation policy with non-PMD sized mTHP */
> -	if (huge == SHMEM_HUGE_FORCE &&
> -	    huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
> -		return -EINVAL;
> +	err = shmem_valid_huge(huge);
> +	if (err)
> +		return err;
>   
>   	shmem_huge = huge;
>   	if (shmem_huge > SHMEM_HUGE_DENY)
> @@ -5174,6 +5182,33 @@ struct kobj_attribute thpsize_shmem_enabled_attr =
>   	__ATTR(shmem_enabled, 0644, thpsize_shmem_enabled_show, thpsize_shmem_enabled_store);
>   #endif /* CONFIG_TRANSPARENT_HUGEPAGE && CONFIG_SYSFS */
>   
> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE)
> +
> +static int __init setup_transparent_hugepage_shmem(char *str)
> +{
> +	int huge, ret = 0;
> +
> +	if (!str)
> +		goto out;
> +
> +	huge = shmem_parse_huge(str);
> +	if (huge == -EINVAL)
> +		goto out;

Looks better. But shmem_parse_huge() is also a common part, and what I 
am thinking is below:

diff --git a/mm/shmem.c b/mm/shmem.c
index f8b8b1ad2631..646d8943950a 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -629,24 +629,39 @@ static unsigned int 
shmem_huge_global_enabled(struct inode *inode, pgoff_t index
         }
  }

-#if defined(CONFIG_SYSFS)
  static int shmem_parse_huge(const char *str)
  {
+       int huge;
+
+       if (!str)
+               return -EINVAL;
+
         if (!strcmp(str, "never"))
-               return SHMEM_HUGE_NEVER;
-       if (!strcmp(str, "always"))
-               return SHMEM_HUGE_ALWAYS;
-       if (!strcmp(str, "within_size"))
-               return SHMEM_HUGE_WITHIN_SIZE;
-       if (!strcmp(str, "advise"))
-               return SHMEM_HUGE_ADVISE;
-       if (!strcmp(str, "deny"))
-               return SHMEM_HUGE_DENY;
-       if (!strcmp(str, "force"))
-               return SHMEM_HUGE_FORCE;
-       return -EINVAL;
+               huge = SHMEM_HUGE_NEVER;
+       else if (!strcmp(str, "always"))
+               huge = SHMEM_HUGE_ALWAYS;
+       else if (!strcmp(str, "within_size"))
+               huge = SHMEM_HUGE_WITHIN_SIZE;
+       else if (!strcmp(str, "advise"))
+               huge = SHMEM_HUGE_ADVISE;
+       else if (!strcmp(str, "deny"))
+               huge = SHMEM_HUGE_DENY;
+       else if (!strcmp(str, "force"))
+               huge = SHMEM_HUGE_FORCE;
+       else
+               return -EINVAL;
+
+       if (!has_transparent_hugepage() &&
+           huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
+               return -EINVAL;
+
+       /* Do not override huge allocation policy with non-PMD sized mTHP */
+       if (huge == SHMEM_HUGE_FORCE &&
+           huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
+               return -EINVAL;
+
+       return huge;
  }
-#endif

  #if defined(CONFIG_SYSFS) || defined(CONFIG_TMPFS)
  static const char *shmem_format_huge(int huge)
@@ -5104,16 +5119,8 @@ static ssize_t shmem_enabled_store(struct kobject 
*kobj,
                 tmp[count - 1] = '\0';

         huge = shmem_parse_huge(tmp);
-       if (huge == -EINVAL)
-               return -EINVAL;
-       if (!has_transparent_hugepage() &&
-                       huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
-               return -EINVAL;
-
-       /* Do not override huge allocation policy with non-PMD sized mTHP */
-       if (huge == SHMEM_HUGE_FORCE &&
-           huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
-               return -EINVAL;
+       if (huge < 0)
+               return huge;

         shmem_huge = huge;
         if (shmem_huge > SHMEM_HUGE_DENY)
@@ -5210,6 +5217,25 @@ struct kobj_attribute thpsize_shmem_enabled_attr =
         __ATTR(shmem_enabled, 0644, thpsize_shmem_enabled_show, 
thpsize_shmem_enabled_store);
  #endif /* CONFIG_TRANSPARENT_HUGEPAGE && CONFIG_SYSFS */

+#if defined(CONFIG_TRANSPARENT_HUGEPAGE)
+
+static int __init setup_transparent_hugepage_shmem(char *str)
+{
+       int huge;
+
+       huge = shmem_parse_huge(str);
+       if (huge < 0) {
+               pr_warn("transparent_hugepage_shmem= cannot parse, 
ignored\n");
+               return 0;
+       }
+
+       shmem_huge = huge;
+       return 1;
+}
+__setup("transparent_hugepage_shmem=", setup_transparent_hugepage_shmem);
+
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+
  #else /* !CONFIG_SHMEM */

  /*

Re: [PATCH v2 1/4] mm: fix docs for the kernel parameter ``thp_anon=``

[Maíra Canal]

Hi Barry,

On 28/10/24 22:03, Barry Song wrote:
> On Tue, Oct 29, 2024 at 8:24 AM Maíra Canal <mcanal@igalia.com> wrote:
>>
>> If we add ``thp_anon=32,64K:always`` to the kernel command line, we
>> will see the following error:
>>
>> [    0.000000] huge_memory: thp_anon=32,64K:always: error parsing string, ignoring setting
>>
>> This happens because the correct format isn't ``thp_anon=<size>,<size>[KMG]:<state>```,
>> as [KMG] must follow each number to especify its unit. So, the correct
>> format is ``thp_anon=<size>[KMG],<size>[KMG]:<state>```.
>>
>> Therefore, adjust the documentation to reflect the correct format of the
>> parameter ``thp_anon=``.
>>
>> Fixes: dd4d30d1cdbe ("mm: override mTHP "enabled" defaults at kernel cmdline")
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> Acked-by: Barry Song <baohua@kernel.org>
>> Acked-by: David Hildenbrand <david@redhat.com>
> 
> Can we separate this and apply it to v6.12-rc? If Andrew doesn't require a new
> version for the separation, can we extract it from this series and
> apply it to mm?

That's fine on my side.

Best Regards,
- Maíra

> 
>> ---
>>   Documentation/admin-guide/kernel-parameters.txt | 2 +-
>>   Documentation/admin-guide/mm/transhuge.rst      | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 1518343bbe22..1666576acc0e 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -6688,7 +6688,7 @@
>>                          0: no polling (default)
>>
>>          thp_anon=       [KNL]
>> -                       Format: <size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>
>> +                       Format: <size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>
>>                          state is one of "always", "madvise", "never" or "inherit".
>>                          Control the default behavior of the system with respect
>>                          to anonymous transparent hugepages.
>> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
>> index 203ba7aaf5fc..745055c3dc09 100644
>> --- a/Documentation/admin-guide/mm/transhuge.rst
>> +++ b/Documentation/admin-guide/mm/transhuge.rst
>> @@ -303,7 +303,7 @@ control by passing the parameter ``transparent_hugepage=always`` or
>>   kernel command line.
>>
>>   Alternatively, each supported anonymous THP size can be controlled by
>> -passing ``thp_anon=<size>,<size>[KMG]:<state>;<size>-<size>[KMG]:<state>``,
>> +passing ``thp_anon=<size>[KMG],<size>[KMG]:<state>;<size>[KMG]-<size>[KMG]:<state>``,
>>   where ``<size>`` is the THP size (must be a power of 2 of PAGE_SIZE and
>>   supported anonymous THP)  and ``<state>`` is one of ``always``, ``madvise``,
>>   ``never`` or ``inherit``.
>> --
>> 2.46.2
>>
> 
> Thanks
> Barry

Re: [PATCH v2 2/4] mm: shmem: control THP support through the kernel command line

[Maíra Canal]

Hi Baolin,

On 28/10/24 23:03, Baolin Wang wrote:
> 
> 
> On 2024/10/29 08:13, Maíra Canal wrote:
>> Add a new kernel command line to control the hugepage allocation policy
>> for the internal shmem mount, ``transparent_hugepage_shmem``. The
>> parameter is similar to ``transparent_hugepage`` and has the following
>> format:
>>
>> transparent_hugepage_shmem=<policy>
>>
>> where ``<policy>`` is one of the seven valid policies available for
>> shmem.
>>
>> By configuring the default hugepage allocation policy for the internal
>> shmem mount, applications that use shmem, such as the DRM GEM objects,
>> can take advantage of mTHP before it's been configured through sysfs.
> 
> Just out of curiosity, do you have any performance benefit data when 
> using mTHP for DRM GEM objects?

I haven't yet benchmark mTHP with V3D (Raspberry Pi's GPU) and also I
still need to find the ideal combination of huge pages' sizes. But the
idea is to benefit from the Super Pages support that I recently
implemented in V3D [1]. Currently, I'm using a separate tmpfs mountpoint
to implement Super Pages.

I'll apply your refactor suggestion in the next version. Thanks!

[1] 
https://mairacanal.github.io/unleashing-power-enabling-super-pages-on-RPi/

Best Regards,
- Maíra

> 
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  7 +++
>>   Documentation/admin-guide/mm/transhuge.rst    |  6 +++
>>   mm/shmem.c                                    | 53 +++++++++++++++----
>>   3 files changed, 57 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/ 
>> Documentation/admin-guide/kernel-parameters.txt
>> index 1666576acc0e..acabb04d0dd4 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -6926,6 +6926,13 @@
>>               See Documentation/admin-guide/mm/transhuge.rst
>>               for more details.
>> +    transparent_hugepage_shmem= [KNL]
>> +            Format: [always|within_size|advise|never|deny|force]
>> +            Can be used to control the hugepage allocation policy for
>> +            the internal shmem mount.
>> +            See Documentation/admin-guide/mm/transhuge.rst
>> +            for more details.
>> +
>>       trusted.source=    [KEYS]
>>               Format: <string>
>>               This parameter identifies the trust source as a backend
>> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/ 
>> Documentation/admin-guide/mm/transhuge.rst
>> index 745055c3dc09..9b5b02c4d1ab 100644
>> --- a/Documentation/admin-guide/mm/transhuge.rst
>> +++ b/Documentation/admin-guide/mm/transhuge.rst
>> @@ -326,6 +326,12 @@ PMD_ORDER THP policy will be overridden. If the 
>> policy for PMD_ORDER
>>   is not defined within a valid ``thp_anon``, its policy will default to
>>   ``never``.
>> +Similarly to ``transparent_hugepage``, you can control the hugepage
>> +allocation policy for the internal shmem mount by using the kernel 
>> parameter
>> +``transparent_hugepage_shmem=<policy>``, where ``<policy>`` is one of 
>> the
>> +seven valid policies for shmem (``always``, ``within_size``, ``advise``,
>> +``never``, ``deny``, and ``force``).
>> +
>>   Hugepages in tmpfs/shmem
>>   ========================
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 44282a296c33..26c1eb1b4b1d 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -582,7 +582,6 @@ static bool shmem_huge_global_enabled(struct inode 
>> *inode, pgoff_t index,
>>       }
>>   }
>> -#if defined(CONFIG_SYSFS)
>>   static int shmem_parse_huge(const char *str)
>>   {
>>       if (!strcmp(str, "never"))
>> @@ -599,7 +598,6 @@ static int shmem_parse_huge(const char *str)
>>           return SHMEM_HUGE_FORCE;
>>       return -EINVAL;
>>   }
>> -#endif
>>   #if defined(CONFIG_SYSFS) || defined(CONFIG_TMPFS)
>>   static const char *shmem_format_huge(int huge)
>> @@ -624,6 +622,20 @@ static const char *shmem_format_huge(int huge)
>>   }
>>   #endif
>> +static int shmem_valid_huge(int huge)
>> +{
>> +    if (!has_transparent_hugepage() &&
>> +            huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
>> +        return -EINVAL;
>> +
>> +    /* Do not override huge allocation policy with non-PMD sized mTHP */
>> +    if (huge == SHMEM_HUGE_FORCE &&
>> +        huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
>> +        return -EINVAL;
>> +
>> +    return 0;
>> +}
>> +
>>   static unsigned long shmem_unused_huge_shrink(struct shmem_sb_info 
>> *sbinfo,
>>           struct shrink_control *sc, unsigned long nr_to_free)
>>   {
>> @@ -5070,14 +5082,10 @@ static ssize_t shmem_enabled_store(struct 
>> kobject *kobj,
>>       huge = shmem_parse_huge(tmp);
>>       if (huge == -EINVAL)
>>           return -EINVAL;
>> -    if (!has_transparent_hugepage() &&
>> -            huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
>> -        return -EINVAL;
>> -    /* Do not override huge allocation policy with non-PMD sized mTHP */
>> -    if (huge == SHMEM_HUGE_FORCE &&
>> -        huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
>> -        return -EINVAL;
>> +    err = shmem_valid_huge(huge);
>> +    if (err)
>> +        return err;
>>       shmem_huge = huge;
>>       if (shmem_huge > SHMEM_HUGE_DENY)
>> @@ -5174,6 +5182,33 @@ struct kobj_attribute thpsize_shmem_enabled_attr =
>>       __ATTR(shmem_enabled, 0644, thpsize_shmem_enabled_show, 
>> thpsize_shmem_enabled_store);
>>   #endif /* CONFIG_TRANSPARENT_HUGEPAGE && CONFIG_SYSFS */
>> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE)
>> +
>> +static int __init setup_transparent_hugepage_shmem(char *str)
>> +{
>> +    int huge, ret = 0;
>> +
>> +    if (!str)
>> +        goto out;
>> +
>> +    huge = shmem_parse_huge(str);
>> +    if (huge == -EINVAL)
>> +        goto out;
> 
> Looks better. But shmem_parse_huge() is also a common part, and what I 
> am thinking is below:
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index f8b8b1ad2631..646d8943950a 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -629,24 +629,39 @@ static unsigned int 
> shmem_huge_global_enabled(struct inode *inode, pgoff_t index
>          }
>   }
> 
> -#if defined(CONFIG_SYSFS)
>   static int shmem_parse_huge(const char *str)
>   {
> +       int huge;
> +
> +       if (!str)
> +               return -EINVAL;
> +
>          if (!strcmp(str, "never"))
> -               return SHMEM_HUGE_NEVER;
> -       if (!strcmp(str, "always"))
> -               return SHMEM_HUGE_ALWAYS;
> -       if (!strcmp(str, "within_size"))
> -               return SHMEM_HUGE_WITHIN_SIZE;
> -       if (!strcmp(str, "advise"))
> -               return SHMEM_HUGE_ADVISE;
> -       if (!strcmp(str, "deny"))
> -               return SHMEM_HUGE_DENY;
> -       if (!strcmp(str, "force"))
> -               return SHMEM_HUGE_FORCE;
> -       return -EINVAL;
> +               huge = SHMEM_HUGE_NEVER;
> +       else if (!strcmp(str, "always"))
> +               huge = SHMEM_HUGE_ALWAYS;
> +       else if (!strcmp(str, "within_size"))
> +               huge = SHMEM_HUGE_WITHIN_SIZE;
> +       else if (!strcmp(str, "advise"))
> +               huge = SHMEM_HUGE_ADVISE;
> +       else if (!strcmp(str, "deny"))
> +               huge = SHMEM_HUGE_DENY;
> +       else if (!strcmp(str, "force"))
> +               huge = SHMEM_HUGE_FORCE;
> +       else
> +               return -EINVAL;
> +
> +       if (!has_transparent_hugepage() &&
> +           huge != SHMEM_HUGE_NEVER && huge != SHMEM_HUGE_DENY)
> +               return -EINVAL;
> +
> +       /* Do not override huge allocation policy with non-PMD sized 
> mTHP */
> +       if (huge == SHMEM_HUGE_FORCE &&
> +           huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
> +               return -EINVAL;
> +
> +       return huge;
>   }
> -#endif
> 
>   #if defined(CONFIG_SYSFS) || defined(CONFIG_TMPFS)
>   static const char *shmem_format_huge(int huge)
> @@ -5104,16 +5119,8 @@ static ssize_t shmem_enabled_store(struct kobject 
> *kobj,
>                  tmp[count - 1] = '\0';
> 
>          huge = shmem_parse_huge(tmp);
> -       if (huge == -EINVAL)
> -               return -EINVAL;
> -       if (!has_transparent_hugepage() &&
> -                       huge != SHMEM_HUGE_NEVER && huge != 
> SHMEM_HUGE_DENY)
> -               return -EINVAL;
> -
> -       /* Do not override huge allocation policy with non-PMD sized 
> mTHP */
> -       if (huge == SHMEM_HUGE_FORCE &&
> -           huge_shmem_orders_inherit != BIT(HPAGE_PMD_ORDER))
> -               return -EINVAL;
> +       if (huge < 0)
> +               return huge;
> 
>          shmem_huge = huge;
>          if (shmem_huge > SHMEM_HUGE_DENY)
> @@ -5210,6 +5217,25 @@ struct kobj_attribute thpsize_shmem_enabled_attr =
>          __ATTR(shmem_enabled, 0644, thpsize_shmem_enabled_show, 
> thpsize_shmem_enabled_store);
>   #endif /* CONFIG_TRANSPARENT_HUGEPAGE && CONFIG_SYSFS */
> 
> +#if defined(CONFIG_TRANSPARENT_HUGEPAGE)
> +
> +static int __init setup_transparent_hugepage_shmem(char *str)
> +{
> +       int huge;
> +
> +       huge = shmem_parse_huge(str);
> +       if (huge < 0) {
> +               pr_warn("transparent_hugepage_shmem= cannot parse, 
> ignored\n");
> +               return 0;
> +       }
> +
> +       shmem_huge = huge;
> +       return 1;
> +}
> +__setup("transparent_hugepage_shmem=", setup_transparent_hugepage_shmem);
> +
> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> +
>   #else /* !CONFIG_SHMEM */
> 
>   /*