| Message ID | 20241029002324.1062723-4-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: add more kernel parameters to control mTHP | expand |
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
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;
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(-)