On Tue, Oct 29, 2024 at 8:24 AM Maíra Canal <mcanal@xxxxxxxxxx> 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@xxxxxxxxxx>
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
