From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C004D6B6DF for ; Thu, 31 Oct 2024 12:51:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1B7F76B0085; Thu, 31 Oct 2024 08:51:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 167DC6B0088; Thu, 31 Oct 2024 08:51:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 02FB36B008C; Thu, 31 Oct 2024 08:51:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id D97AF6B0085 for ; Thu, 31 Oct 2024 08:51:32 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 84EA8415B7 for ; Thu, 31 Oct 2024 12:51:32 +0000 (UTC) X-FDA: 82733882616.25.03E0570 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by imf11.hostedemail.com (Postfix) with ESMTP id 18C514001A for ; Thu, 31 Oct 2024 12:50:55 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=OJJ+ekAp; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf11.hostedemail.com: domain of mcanal@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=mcanal@igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730378928; a=rsa-sha256; cv=none; b=l3fomNIG4RedL23RVD3cPvNfuFNnq/4YN33Hufw+Dj9/z/Pr3oguD7sd2z7cmrkQEX9U24 GLeipyzkBnjO4VtGiAQ961CzYfZgPGPP6jKsNjswE10xHlUcr/zmkY6BTy/qWN2VPo3CAG /9anfLpqQaBx5RQQV5ZHT3oVYvFuwmo= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=OJJ+ekAp; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf11.hostedemail.com: domain of mcanal@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=mcanal@igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730378928; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=pHltMX9LLDodjoFNq4oyjQF0WKzjMpxw/dGQ9QZLO3U=; b=GRX0+3cK+LNjl9dZBxpNPuhxebNVMfDoj6q6N3rn8itpjg4E/RJC4zqDYZtQXkYGfQvlgp 3lTAc1yUNAyzGaF/Nf9rVFe4aLcRRamzqUwFE6tTMSQ8l7abVPYe24j9lkDsZ2AnSl82R8 tsj7268slRPF2g6Zg3rzwH5yNgZe214= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=pHltMX9LLDodjoFNq4oyjQF0WKzjMpxw/dGQ9QZLO3U=; b=OJJ+ekApQQKAPl3CnRjns8iW2S S/n/Va8F8V1WXrcXiImAuA+vEQshA/m+Cv7dqmFeIm9awtFnlw+hU5AHLfyV/LU2f6YUh/+nShGJo OCgMWXkLbSlmKcjRpB64Flah7LH8M+iIORw9lYdUY+2xNN2VOhUKT+tYFroXnzsWg5tmrTQI6jBtr 1zPpm1by9rx+HJcpbmCVAE+XI2BescWnMbVMZYEIZuvQphyawBchAy2Kcsd4fPCdvqVbR5kLBojW6 Bm3bArrJ080zvH81YsQlxSXd0jHsyVVnzQLasmKnWqEeBXeXPCxcDRjyDVEMYNsqVvzlQfR/CHtYF ZDK2pD3A==; Received: from [187.36.213.55] (helo=[192.168.1.103]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1t6Udt-00014M-Qd; Thu, 31 Oct 2024 13:51:14 +0100 Message-ID: <899284fa-953f-48a1-af29-222d0d55881c@igalia.com> Date: Thu, 31 Oct 2024 09:51:05 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 3/4] mm: shmem: override mTHP shmem default with a kernel parameter To: David Hildenbrand , Jonathan Corbet , Andrew Morton , Hugh Dickins , Barry Song , Ryan Roberts , Baolin Wang , Lance Yang Cc: linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-dev@igalia.com References: <20241030130308.1066299-1-mcanal@igalia.com> <20241030130308.1066299-4-mcanal@igalia.com> <2c507326-3267-431e-936a-23e2ab6a3baf@redhat.com> Content-Language: en-US From: =?UTF-8?Q?Ma=C3=ADra_Canal?= In-Reply-To: <2c507326-3267-431e-936a-23e2ab6a3baf@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: 5mxrfnpq6s88wadtmge5pj8zd3mst3px X-Rspamd-Queue-Id: 18C514001A X-Rspamd-Server: rspam08 X-Rspam-User: X-HE-Tag: 1730379055-492621 X-HE-Meta: U2FsdGVkX1/ga304wAwRuoiXluCDkPNTY+JHmQEltkTOl5ZtPBaa/5T3kPeTYnK2WKzb891PEgAVG1YkJqaOh6w2FioKN45xtAR+YVR82kI9CMHB337zCL07XOomUWj/LudlLY4kHJ74bDob4bhDMP0JNzW3tnCV150jQsqYo1XcbYOoWnLXmGh3f9jlk1qsQ6J4VluB6jGskLyyCDMR/EHX3VZVevrpkOD98bw+a6qO7/XYzxAOrg31l8eSw80HM+vBNXWSgUxfPdw+GApEJWX0cWhhkwur2VO0RAwM8puc4YwRxOGaaO/sxzTWeAbi7vT4BCTFvX5Qab7Z+azuIDZ99hCbR6mMJRG7rTYA+sEeByLZQnx8lF+ObBzV6wjSfQyOBgqkrmDeyIiXHZpzrurY5sNa+JFtKl4FsL8FyppE3xvKQ2MIvXdWo6/PD3Y6Lm5543oImXwNMSvgt4gkwTKk2UIiUKVmHXrz1ZmjL0Hhy8EVnWEd5W3L9BwqVdCCubImbwhjW5yROdHywRFKd7Si4eSBQZgkOprWvt8l/aK//g/iRYlheujg75/7Qqbtqzfjsf4ogLk8DuVnFb8aMry0OnXNup8fOt11SPCijMia9KAXRquMKMBPUpBoqzXgIZgL1hwlyPjuf9gypqUuCBW7XuHnmqNla14n7Sk1gj3OnVXDIt3gWf1UPN+LIxHDu395Hm/8Vw/UjQo+g0S+ZSgjJLhpwUDrnJCHdv8EUjcrOMtwOkxMZq+d/RdiLjps7J4qE1Q+HM4ziIqLpwdrcdtmLmEr58ZOEo69Xd7VI0e6vSjfNkW0kHzVUMrf19L08pgVXMi/pY8SCbIcZXoJhODeJo9yd6JmNXfuRefUue0dA1tV50Qxxn9E7F3Uy3YwVDekLz+4x9uhotlIDx34LDmI1EB6dZM5FF6+9kOKKnlgBIslVnit7HX898NqF/IxnvpLF6fUGq5nWwVpOZY 5CUMcZea D6AcMPLUUZuVdL2LutoX4nzbYpvRLvSNvpW15ITe9i7aAOhbI65Jd7CFQ0DE4r5PO/I/jd4/N4lURJC1x+w21ymRPtKPMQAQCZMmBCnLdcPxHANZ0FH5qp3+VsgabwDANiW+xNjJK0Qr4SrDwMUzE2xi4NxTEtw4iXuS77oBbgj/t3i5QpSPsdOiaUwCPT5344NFZTEV7SofAjK+agXff8MKS4R6u2izPcwjJz91qHj/ulW1HeFX6YDM0ewWDlVSisuE0+tu6jZC4+n25T/085bfY9/kLllR3tvIBRO/Vcv5UlmClZI5OKYGeLQhVCY0T/y7MEJI6yBiKhGbxXNseFVhHVdKUyDKy+c8NfxtqfiFQHk8d00kpIPBlq/3JG52UE+2Mbj03Jr6rpko= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi David, On 31/10/24 09:37, David Hildenbrand wrote: > On 30.10.24 13:58, Maíra Canal wrote: >> 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=[KMG],[KMG]:;[KMG]- >> [KMG]: >> >> 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 >> --- >>   .../admin-guide/kernel-parameters.txt         |  10 ++ >>   Documentation/admin-guide/mm/transhuge.rst    |  17 +++ >>   mm/shmem.c                                    | 109 +++++++++++++++++- >>   3 files changed, 135 insertions(+), 1 deletion(-) >> [...] >> diff --git a/mm/shmem.c b/mm/shmem.c >> index dfcc88ec6e34..c2299fa0b345 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; >> @@ -5180,6 +5182,26 @@ struct kobj_attribute thpsize_shmem_enabled_attr = >>   #if defined(CONFIG_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_FILE_DEFAULT) >> +        goto err; >> + >> +    return order; >> +err: >> +    pr_err("invalid size %s in thp_shmem boot parameter\n", size_str); >> +    return -EINVAL; >> +} > > Hm, mostly copy and paste. You could reuse existing get_order_from_str() > simply by passing in the supported orders and moving error reporting to > the caller. > Can I use functions from mm/huge_memory.c here? > static inline int get_order_from_str(const char *size_str, >         int valid_orders) > { >     ... >     if (!is_power_of_2(size)) >         return -EINVAL; >     order = get_order(size); >     if (BIT(order) & ~valid_orders) >         return -EINVAL; >     return order; > } > >> + >>   static int __init setup_transparent_hugepage_shmem(char *str) >>   { >>       int huge; >> @@ -5195,6 +5217,91 @@ 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) >> +{ >> +    char *token, *range, *policy, *subtoken; >> +    unsigned long always, inherit, madvise, within_size; >> +    char *start_size, *end_size; >> +    int start, end, nr; >> +    char *p; >> + >> +    if (!str || strlen(str) + 1 > PAGE_SIZE) >> +        goto err; >> +    strscpy(str_dup, str); >> + >> +    always = huge_shmem_orders_always; >> +    inherit = huge_shmem_orders_inherit; >> +    madvise = huge_shmem_orders_madvise; >> +    within_size = huge_shmem_orders_within_size; >> +    p = str_dup; >> +    while ((token = strsep(&p, ";")) != NULL) { >> +        range = strsep(&token, ":"); >> +        policy = token; >> + >> +        if (!policy) >> +            goto err; >> + >> +        while ((subtoken = strsep(&range, ",")) != NULL) { >> +            if (strchr(subtoken, '-')) { >> +                start_size = strsep(&subtoken, "-"); >> +                end_size = subtoken; >> + >> +                start = get_order_from_str(start_size); >> +                end = get_order_from_str(end_size); >> +            } else { >> +                start = end = get_order_from_str(subtoken); >> +            } >> + >> +            if (start < 0 || end < 0 || start > end) >> +                goto err; >> + >> +            nr = end - start + 1; >> +            if (!strcmp(policy, "always")) { >> +                bitmap_set(&always, start, nr); >> +                bitmap_clear(&inherit, start, nr); >> +                bitmap_clear(&madvise, start, nr); >> +                bitmap_clear(&within_size, start, nr); >> +            } else if (!strcmp(policy, "advise")) { >> +                bitmap_set(&madvise, start, nr); >> +                bitmap_clear(&inherit, start, nr); >> +                bitmap_clear(&always, start, nr); >> +                bitmap_clear(&within_size, start, nr); >> +            } else if (!strcmp(policy, "inherit")) { >> +                bitmap_set(&inherit, start, nr); >> +                bitmap_clear(&madvise, start, nr); >> +                bitmap_clear(&always, start, nr); >> +                bitmap_clear(&within_size, start, nr); >> +            } else if (!strcmp(policy, "within_size")) { >> +                bitmap_set(&within_size, start, nr); >> +                bitmap_clear(&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); >> +                bitmap_clear(&within_size, start, nr); >> +            } else { >> +                pr_err("invalid policy %s in thp_shmem boot >> parameter\n", policy); >> +                goto err; >> +            } >> +        } >> +    } > > > Similarly, copy-paste. But not that easy to abstract :) So maybe we'll > have to keep that as is for now. On v2 [1], I abstracted to reduce copy and paste, but me and Barry agreed that adding this sort of header to linux/huge_mm.h was weird. [1] https://lore.kernel.org/linux-mm/20241029002324.1062723-4-mcanal@igalia.com/ Best Regards, - Maíra > >