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 1B05DD13596 for ; Mon, 28 Oct 2024 11:34:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8AF5F6B0085; Mon, 28 Oct 2024 07:34:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 85F316B0088; Mon, 28 Oct 2024 07:34:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 726EE6B0089; Mon, 28 Oct 2024 07:34:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 557206B0085 for ; Mon, 28 Oct 2024 07:34:30 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 8CB10C092F for ; Mon, 28 Oct 2024 11:34:05 +0000 (UTC) X-FDA: 82722802638.04.FD90C84 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by imf27.hostedemail.com (Postfix) with ESMTP id 098D14000D for ; Mon, 28 Oct 2024 11:34:03 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=eyuSdExE; spf=pass (imf27.hostedemail.com: domain of mcanal@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=mcanal@igalia.com; dmarc=pass (policy=none) header.from=igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730115188; a=rsa-sha256; cv=none; b=jfGvl9LmjZslH9MSx/Ro3a7eSNMzjE6MI6JMV2WtnpiTGRgCV6RUK5ZKDPF60OmUTe4kbQ OVxff8++uYNxgn7b9idZR26I/QwQiHGxF1X0i5rOOSMtXAd3ge/Ypr5dawOWYD/Dge2hlb QHs3PYEZSKKRD0DjNHDh+rxNejVWLrk= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=eyuSdExE; spf=pass (imf27.hostedemail.com: domain of mcanal@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=mcanal@igalia.com; dmarc=pass (policy=none) header.from=igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730115188; 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=odLg8OLtLEPB9lZYVjnwYcKDJqm1UOOqheOqN1GgRNM=; b=jMscOjGY9yDrDa0xQRBEJ3S3aRxU/DQEiGdyd6OhS5oBDLSK1muIyQ75mFJ+7x60ekZBHo eJKXmXmLeiZLRyV8cOj4111Do+lai8KQq+eowY259edqKrPnhlhU+67vhEeE9lMAVBCbcn jiMn7JZ3ichA5zmaVRxbJ0iKLdVkCws= 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=odLg8OLtLEPB9lZYVjnwYcKDJqm1UOOqheOqN1GgRNM=; b=eyuSdExE91ixSVAYvRqlkQCeKW BfPS2+HRPptimKZcKXtx5c/NMMds3ub3l3pzmN69ca0RMuz9l/5PRalx+MtbXIXBqkxyBIFUwxIGY 9QdYl0xhbBXuPKTUX7fIl0KIredaMV5/rw5aLTbYWXPqEusgIsYepvIc0TyZFSWs6d3fuRgGP8CSM E022BzzYdi0G/a/kgMWoLsAKF1GDXVHMCynGbUw+vbr4WvEdZkWVva94PhsAGs4fR1gLjSvITbwZt GSWkUss3/jotIdYiP0CTQo/YHV9bm+/7TU3WeIoLQ/GmFeH3Dpm4SvwMBP9NeYxQBpT8A73bLNVRi oAa5fCCw==; 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 1t5O0m-00G3bt-6N; Mon, 28 Oct 2024 12:34:16 +0100 Message-ID: Date: Mon, 28 Oct 2024 08:34:08 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/3] mm: shmem: override mTHP shmem default with a kernel parameter To: Barry Song <21cnbao@gmail.com> Cc: Jonathan Corbet , Andrew Morton , Hugh Dickins , David Hildenbrand , Ryan Roberts , Baolin Wang , Lance Yang , linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-dev@igalia.com References: <20241027175743.1056710-1-mcanal@igalia.com> <20241027175743.1056710-4-mcanal@igalia.com> <2505d52c-3454-4892-8c90-e3d9b2f0c84f@igalia.com> Content-Language: en-US From: =?UTF-8?Q?Ma=C3=ADra_Canal?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: penzpcezx8dnbapgb7pwbkcbhsq6w81r X-Rspamd-Queue-Id: 098D14000D X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1730115243-389792 X-HE-Meta: U2FsdGVkX1+LXnsdO5gfT76srworV5CpeQoGFgyaCBG4XxqL7sAxOtHm1ru6hogchrpsDW2bOgMQG98tcxcK3IRodGx8j5K+cNmiB8Kc4AFWHI2JrD1aHMu0v0QMaSOX4Vb3aJACW938sMcrg5BcC8PGDsgxXvvj99092w5L8XQ2yI3MLveJurgf57nZmvr0VTSlIYxwJzcN377gJp4pzAhyJAaye2xA88xg19Msn5x45AMlHBZDi8WnvxJ3tItjzlnFB8zsiMzdD4/au4idweUW1Gq2fjUWKDqzMyHPC9FP/OjjVucLcBDv7CSG/7FxiJ2lC/0wVuVfhdn91TVqsvK66z1dCVPRiTqe3l7m8lrZXe7RdXiLsb7Fxbs2RPz62KVgJfh3jhiMlcfzdSkKUfeef8Vp071fFOxufapnZL38x+PKrFLDdD6pnKpnv5GOz4lIGRTtpKSYdALZCcjxi/9L1Yr/Bj9Sygw+EHsipLdPSVkVcRRq9HBZHIdihdLfBPZa0kGZfUxtxH27f7Ia43Xl/N9+0QT3WG612j+nSdovPivfzPpcKS1Ly/Q8UBiH4oHGQ4tqXVHpAMA4PZgauZx9UEg0AMSVuYZAjYpipmlxAJZXIaJrr+rmCyLkIWL2kCNNkb3n5VK8XywupPjTJ6CiTtExPriRwQuPHiJto1xWWkYeR4qMyf+UNvXAApeNEg1B7UJUN1qw0Liz/DxE7yjmYCRi9R3v4FgRBEmGij2OjYSr32ZOvyFGEOEt5ap7wtRMlnBcGOx/u18PSr3W4tJct5sibQl5sxqOfp+DouVXCAQIuQHUzT4fhYNnZWNqMBm/nxps4RFSXJ0ZD4JE5A0PGFcWryRtvMYQGgS3orh/v0B16fGkMBFBqDSMSYwbgDQ337jAJvSksPH+B84+DWMyFyc+Wpmi7HO63CdtBl3CoKh3yBiD5PicF2vG5Z4HjafS+SlrLG3coZ5fAgA QI5HThKq 6tOtCYh32MkvSnvak+qNKqN+ytcz8C2aKaisCdP02lguq7ckXrDtmdDn3EJyQlW1Qoihm1RezSw0o08789mf+yrM9JsWCFB/9Dn3gCUxncskRFpZukqDh4L406rr2f2ADMRL6WlbTyBwGnRkOU960gBB5svpP9c8nfUImx7J0UGVdiY4qT3fjG6HTlk8yIXAHt2X23MvRnUUTbfsFKUDYK2VipALPxHSzFDEwnDKSpF5enpHwbmFR6CbVN1y47rubeyLXlRSMEmX/Tx06W5AV3cdFhmyQsqsSfzVAbx7iNZ2TLNMXoPep94aBm7IywSV37u/hEHzoGboHSaD9BIB4fmE/RzIBPNNzGlibxClOwfVnf3Wq7cNiqFkvLQ== 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 Barry, On 28/10/24 08:09, Barry Song wrote: > On Mon, Oct 28, 2024 at 6:10 PM Maíra Canal wrote: >> >> Hi Barry, >> >> On 27/10/24 18:54, Barry Song wrote: >>> On Mon, Oct 28, 2024 at 6:58 AM 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 huge pages, 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(-) >>>> >>> >>> Hi Maíra, >>> >>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >>>> index acabb04d0dd4..595fa096e28b 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. >>>> >>>> + shmem_anon= [KNL] >>>> + Format: [KMG],[KMG]:;[KMG]-[KMG]: >>>> + Control the default policy of each hugepage size for the >>>> + internal shmem mount. 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. >>> >>> I'm not sure this is the right name. How about "thp_shmem"? >> >> Oops, sorry about that. >> >>> >>>> + >>>> 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 24cdeafd8260..0a7a7d04f725 100644 >>>> --- a/mm/shmem.c >>>> +++ b/mm/shmem.c [...] >>>> static int __init setup_transparent_hugepage_shmem(char *str) >>>> { >>>> int huge, ret = 0; >>>> @@ -5206,6 +5228,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; >>>> + strcpy(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; >>>> + } >>>> + } >>>> + } >>>> + >>>> + huge_shmem_orders_always = always; >>>> + huge_shmem_orders_madvise = madvise; >>>> + huge_shmem_orders_inherit = inherit; >>>> + huge_shmem_orders_within_size = within_size; >>>> + shmem_orders_configured = true; >>>> + return 1; >>>> + >>>> +err: >>>> + pr_warn("thp_shmem=%s: error parsing string, ignoring setting\n", str); >>>> + return 0; >>>> +} >>> >>> Can we share source code with thp_anon since there's a lot of duplication? >> >> I'm not a regular mm contributor and I'm most usually around drivers, so >> I don't know exactly here I could add shared code. Should I add the >> headers to "internal.h"? > > My comment isn't related to drivers or memory management. It's solely about > avoiding code duplication. For example, we could create a shared function to > handle both controls, reducing redundant code :-) Let me rephrase it. I completely agree that we should avoid code duplication. I'm asking where is the best place to add the headers of the shared functions. "linux/shmem_fs.h" doesn't look appropriate to me, so I believe the remaining options would be "linux/huge_mm.h" or "internal.h". I would like to know your opinion about those two options. Best Regards, - Maíra > >> >> Best Regards, >> - Maíra >> >>> >>>> +__setup("thp_shmem=", setup_thp_shmem); >>>> + >>>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ >>>> >>>> #else /* !CONFIG_SHMEM */ >>>> -- >>>> 2.46.2 >>>> >>> >>> Thanks >>> barry >>