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
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
© 2016 - 2024 Red Hat, Inc.