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 4DD83C3ABC9 for ; Sun, 11 May 2025 02:58:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 12E196B0083; Sat, 10 May 2025 22:58:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0DFBF6B0085; Sat, 10 May 2025 22:58:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC14C6B0088; Sat, 10 May 2025 22:58:44 -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 CF4046B0083 for ; Sat, 10 May 2025 22:58:44 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id B92461A1553 for ; Sun, 11 May 2025 02:58:45 +0000 (UTC) X-FDA: 83429119410.22.9B2A726 Received: from mail-yw1-f176.google.com (mail-yw1-f176.google.com [209.85.128.176]) by imf30.hostedemail.com (Postfix) with ESMTP id B60E580005 for ; Sun, 11 May 2025 02:58:43 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gY+3APP9; spf=pass (imf30.hostedemail.com: domain of joshua.hahnjy@gmail.com designates 209.85.128.176 as permitted sender) smtp.mailfrom=joshua.hahnjy@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1746932323; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=4rIqno4q3SZ3Qt2EV0P6akhqys5m6oT2jaLjH6WpTDE=; b=U8YiFosUoy6JgCVCrG8BEuPlNRMwZQXkW/wiVyI+b1ABxukkKorUplQPp/6KATyPqJo43y cz/yLqUPW85XFfV6Y/bZp9eRtr1jXm5fRbWBCLDN6unAjJFBud/YxXb/Je+ci2o2onI+t/ EIdQ6mviSbBrA/4nxdNs4xhlgfQXbIc= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gY+3APP9; spf=pass (imf30.hostedemail.com: domain of joshua.hahnjy@gmail.com designates 209.85.128.176 as permitted sender) smtp.mailfrom=joshua.hahnjy@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746932323; a=rsa-sha256; cv=none; b=ugF8Cma2klOqaOpIGWK19EcHxMgnJG7un1uH3TaWHgquvG8bvHX5gU0g8ET0QxiFEVofQW XJ259gdCcsh74r6wy2ZQZLnYtMmmqXe0yv0OwlVBgpo1cI2VAd9A+RubSxg5jej8g6FG1A OiYvTEzhdjXcbihA1k6aPVi3usD9FSs= Received: by mail-yw1-f176.google.com with SMTP id 00721157ae682-70a35432c21so26794167b3.0 for ; Sat, 10 May 2025 19:58:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1746932323; x=1747537123; darn=kvack.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=4rIqno4q3SZ3Qt2EV0P6akhqys5m6oT2jaLjH6WpTDE=; b=gY+3APP9jazl8ct3DZXE9PeE2A6QVQRht2xU1hn8Oc57FdmAUHfGC0SuaiwcO7NJPx 2sv1cdlU9EM9AL/rgrRNLLOoH5+fnoeYFCUYIg0OnloPfV/N0/kmNTKYx6+di1y754YY twURQciTrHk9+gB3LfxeE+bTqkjJzrtvfJzNhktPYsJ87FNVRxAkrUynMNiMe0jI/oY9 sRS7BM8A2m3AM2YnmTqqaz+XY8xk+rYoM3dFiFyxqXtmUy/uPHoMRczhGDkUQMWLYfwf 3zHwwamyGOFDqD/PdLwVA19gHbwC9IMG7FSQ9ADS+AQOcPP93d6DQhITqHJGvA+mTpUO ufwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1746932323; x=1747537123; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4rIqno4q3SZ3Qt2EV0P6akhqys5m6oT2jaLjH6WpTDE=; b=f8FJ3JuzGZ8aO7DR/L/JozXzXcMmMgsqHYXUByTbeEms+TehvRdTgC1agfachfHFGe 95toOuiGjWO3oyolvniXNeEXL/61x2ETH2OrzFBO+Wp+IamRVFV3sntHken6ezF+vHKO jWoRPbFaaJVDy5+aGyxS4tP+Eb7DP7jl4maspOpJcxfvX0ku39IKIRDILgxQb3xbKF+s /lacodtTrrR3mlWJrbEGSudgs/AWfrpXF/yiHxuFNWhyuecNTdZ3quwpkR7GdjH0BSJf eFBlk/IrHOYc3SS0dWbr0aIcWI7kO+dmCDh4yVbjP2+BgnnUU2JMfJGjbyROlihrouwc KC9Q== X-Forwarded-Encrypted: i=1; AJvYcCVMgcRC6rcdSa5doEaaaa5TzeZYs9eWX/4xtnRwA2iHwUXjd+RxkM+GGFwmTEoGTKjmbBOBjQeevg==@kvack.org X-Gm-Message-State: AOJu0Yw/bfmdxwZpSQRnZH88qwAgBs9JoAXVahsaXbWxKL0AdpZ4KpT5 GlIK4FtEPqNHbe4rCTwIhiTqutZn/2dS3U9gx0bn141k4SDThzEC X-Gm-Gg: ASbGncv9HN9BGsT5Q9qJqcRPvE3ZY2hDb4k8Cwl6sed223tZEurMkM2Nvl31FICatZ4 AEU6kFpZqn2ByjRxMIs6/XslXhLCPrD8aotV3S2bRoiw6MoxngxjJxrkL7y1KHTp/+ZUJ6gEZyN 7aIUQaVL/BkgpAvzKdGZfgkfrkaIdtiTPYjHbcBNNpBkm5BlY09JEACzgn1NWrZ+ssU2sAZSWIY RKPXcXnR58i3ygoIVimAMHn3CdnH269xKyhOlXBkiy5j4nzki1rLPdtbPF4ksdCOBBzQHOcrOBF U8FhvnvXjLXfUIJS4oFlGkgGmFTw4gnmHMThD+UhmahgNcuDZQ== X-Google-Smtp-Source: AGHT+IHzJNRE1nE1qbW2MZGxQNyfCfDoJdMT84okbgjoRCuPxSjJvjYiOEtbzvisFV9KGgOn7dPL7Q== X-Received: by 2002:a05:690c:6c07:b0:6fd:33a5:59a with SMTP id 00721157ae682-70a3fa3df25mr129045327b3.18.1746932322631; Sat, 10 May 2025 19:58:42 -0700 (PDT) Received: from localhost ([2a03:2880:25ff:1::]) by smtp.gmail.com with ESMTPSA id 00721157ae682-70a3d9cb39esm13386787b3.72.2025.05.10.19.58.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 10 May 2025 19:58:41 -0700 (PDT) From: Joshua Hahn To: Joshua Hahn , Andrew Morton Cc: "Huang, Ying" , gourry@gourry.net, honggyu.kim@sk.com, yunjeong.mun@sk.com, gregkh@linuxfoundation.org, rafael@kernel.org, lenb@kernel.org, dan.j.williams@intel.com, Jonathan.Cameron@huawei.com, dave.jiang@intel.com, horen.chuang@linux.dev, hannes@cmpxchg.org, osalvador@suse.de, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, linux-mm@kvack.org, kernel-team@meta.com Subject: Re: [PATCH v8] mm/mempolicy: Weighted Interleave Auto-tuning Date: Sat, 10 May 2025 19:58:39 -0700 Message-ID: <20250511025840.2410154-1-joshua.hahnjy@gmail.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20250510185150.4078843-1-joshua.hahnjy@gmail.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Stat-Signature: c4jcbnogkuehkfn35fut4akqk39zm4fn X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: B60E580005 X-HE-Tag: 1746932323-788970 X-HE-Meta: U2FsdGVkX18oqobizfy2h8qTNVbn8vQLG0jUZX7LMyftv+kNB+7UsunDgeyCnRqTBNX1jkpSIBcQA8D3v2LqdMIDnnOqNjN34kVDBbcybroLwmNSceuSvNVNYer4CNRGjHvMmhlwslMXa3b8IQ23Nkv0I9FiqMKNkEqpvBobNkSCh0PtxPBWxMXIYc3+V2uZXDjJog0JC4t5vTgIlWXJ3kUtOlM+jQnYVvZqjr1wCuFDBZQ2lys+aXaCi84IqZ/RkXjy2Lfdtf0bQKoujYWvmabb26VljPH9bZtvX6x2dVG/692k6cwct2u7Q+JgWITucW9Ze2wwU6o/rSiayOGpDjgtaefPhXwG/g1AgF2Vo5Ep8PJwzPHsZNc5ClnYREnMLqSGTIpCOklac7UXskgUX3TSSj/VINBmB3JVp9nySDNmHcn/zWFK2rIlU7eLpTKTxVpIamGPKtcBTh4sHVYst3ZZbWwuQMeOuCycW4SlB1IPHRrwjuYFUNE/tWll7o0wxzQsROMP94Dh2ftRQNuX/itlzHWZGrEs/ZESNfm+wa7qthlgwukrysbzblgDtSrRVfL/AwmteUgmhtkqsxpqBs22KlXH+N20IRdCcFbO/UgE40lwbSf1EgLZGuxCsWj3BNg9nR9Ssv3udBwc9PMYCG0obWWGnAXZpNzpuPcjjXe4XCKrfRbWcqg0JHlQNhaIlWpTYEh1wRZKzhrVDOpsI112/QSgSBc0s3SJRigiQSLgOR8qyFBl4nJRHhlRSzLdEErAwoPZJ2NRWCzz21ctIICYqsmTlY5X5TEabbUCZzqGw7+Kx8gc73MPQ1dx1PkPZRaKSkbfLUQ4+NRfxYyLjfTdsvkFjB+qnzVazgcZSijKvf4ozujGE1DZTzTqSlWbKnrWzH0apAGGuucGhBYe0rPLUmEpv2OcAwnwI1vWU8V4n0FAQZimfGj2Kvwd/U/32oOHR+7QlXfY7NxOSVq TVJL+oQc 7YPLLrk52qxUhFxljg6NyoBl+I5arK/aZKKWo22GMMRdH+FtvVpJKvogSv6vbzEJphhGU5oIaMZ7ZqtkgVYlW5pfO3wW5Lgh8RhBw5JQggsm7yZ0pGZBx5nyhxm0exkGeaLCJc3nDxoJGq7ZAKwN2dqRxUXu+TL3RZjOqIcIakEzOXiHzmEUfhip6ch/pQRHAmy2lxo8npu0QqVKlKmEuIev5Gt1gmQrFw53N9+lUPX22LejU849swYm6wHLjoVIL5S+62kgKBlv5FKLetceAihzWg4iOlF1lBZcx9BeetTb+G/26QlXwkQUlFS5y0BNs6RMeI5066DZn609JNA5hHpYuX24quUVm+2ciMRqsytYqRtKieUPsRscj5Q+SGVHGUPjH8frJHqzvy1FFPrz4MHLnIQStVuH+PrcnLJBHm97kuNWpoy/514kBoI7s1w8DHer12MdSqusRcrFtMWerz/MnaeIw/TFRZHORn1BllZ7kR9HvongAs/dXQh1htqENd7ExEw9PTl5tzI5g/6WNyeaqdJQLQdwmXrpxYhhK+tc6puQBYQ8VgjSc2g== 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: Hello Andrew, I was hoping to fold this fixlet in with the patch this belongs to. It includes some wordsmithing changes, some code simplification/cleanups, and makes sure that the code behavior matches that of the ABI I described. I've kept the original message below as well, where Ying suggested the changes present in this fixlet. Please let me know if this fixlet is too big, and you would rather prefer a new version instead. Thank you as always for your patience and support! Joshua diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave index ec13382c606f..649c0e9b895c 100644 --- a/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave +++ b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave @@ -24,7 +24,7 @@ Description: Weight configuration interface for nodeN empty string, ...) will return -EINVAL. Changing the weight to a valid value will automatically - update the system to manual mode as well. + switch the system to manual mode as well. What: /sys/kernel/mm/mempolicy/weighted_interleave/auto Date: May 2025 diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h index 3e8da8ba1146..0fe96f3ab3ef 100644 --- a/include/linux/mempolicy.h +++ b/include/linux/mempolicy.h @@ -57,15 +57,6 @@ struct mempolicy { } w; }; -/* - * A null weighted_interleave_state is interpted as having .mode = "auto", - * and .iw_table is interpreted as an array of 1s with length nr_node_ids. - */ -struct weighted_interleave_state { - bool mode_auto; - u8 iw_table[]; -}; - /* * Support for managing mempolicy data objects (clone, copy, destroy) * The default fast path of a NULL MPOL_DEFAULT policy is always inlined. diff --git a/mm/mempolicy.c b/mm/mempolicy.c index f542691b7123..0624d735a2e7 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -148,6 +148,14 @@ static struct mempolicy preferred_node_policy[MAX_NUMNODES]; */ static const int weightiness = 32; +/* + * A null weighted_interleave_state is interpreted as having .mode="auto", + * and .iw_table is interpreted as an array of 1s with length nr_node_ids. + */ +struct weighted_interleave_state { + bool mode_auto; + u8 iw_table[]; +}; static struct weighted_interleave_state __rcu *wi_state; static unsigned int *node_bw_table; @@ -3561,9 +3569,8 @@ static ssize_t node_store(struct kobject *kobj, struct kobj_attribute *attr, int i; node_attr = container_of(attr, struct iw_node_attr, kobj_attr); - if (count == 0 || sysfs_streq(buf, "")) - weight = 0; - else if (kstrtou8(buf, 0, &weight) || weight == 0) + if (count == 0 || sysfs_streq(buf, "") || + kstrtou8(buf, 0, &weight) || weight == 0) return -EINVAL; new_wi_state = kzalloc(struct_size(new_wi_state, iw_table, nr_node_ids), @@ -3630,9 +3637,15 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj, if (!input) { old_wi_state = rcu_dereference_protected(wi_state, lockdep_is_held(&wi_state_lock)); - if (old_wi_state) - memcpy(new_wi_state->iw_table, old_wi_state->iw_table, - nr_node_ids * sizeof(u8)); + if (!old_wi_state) + goto update_wi_state; + if (input == old_wi_state->mode_auto) { + mutex_unlock(&wi_state_lock); + return count; + } + + memcpy(new_wi_state->iw_table, old_wi_state->iw_table, + nr_node_ids * sizeof(u8)); goto update_wi_state; } @@ -3707,8 +3720,12 @@ static void wi_state_free(void) kfree(&wi_group->wi_kobj); } +static struct kobj_attribute wi_auto_attr = + __ATTR(auto, 0664, weighted_interleave_auto_show, + weighted_interleave_auto_store); + static void wi_cleanup(void) { - sysfs_remove_file(&wi_group->wi_kobj, &wi_group->auto_kobj_attr.attr); + sysfs_remove_file(&wi_group->wi_kobj, &wi_auto_attr.attr); sysfs_wi_node_delete_all(); wi_state_free(); } @@ -3798,10 +3815,6 @@ static int wi_node_notifier(struct notifier_block *nb, return NOTIFY_OK; } -static struct kobj_attribute wi_auto_attr = - __ATTR(auto, 0664, weighted_interleave_auto_show, - weighted_interleave_auto_store); - static int __init add_weighted_interleave_group(struct kobject *mempolicy_kobj) { int nid, err; On Sat, 10 May 2025 11:51:50 -0700 Joshua Hahn wrote: > On Sat, 10 May 2025 13:25:32 +0800 "Huang, Ying" wrote: > > Hi Ying, > Thank you for reviewing my patch, as always! > > > Hi, Joshua, > > > > Thank you for updated version! And sorry for late reply. > > > > Joshua Hahn writes: > > [...snip...] > > > > diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave > > > index 0b7972de04e9..ec13382c606f 100644 > > > --- a/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave > > > +++ b/Documentation/ABI/testing/sysfs-kernel-mm-mempolicy-weighted-interleave > > > @@ -20,6 +20,35 @@ Description: Weight configuration interface for nodeN > > > Minimum weight: 1 > > > Maximum weight: 255 > > > > > > - Writing an empty string or `0` will reset the weight to the > > > - system default. The system default may be set by the kernel > > > - or drivers at boot or during hotplug events. > > > + Writing invalid values (i.e. any values not in [1,255], > > > + empty string, ...) will return -EINVAL. > > > + > > > + Changing the weight to a valid value will automatically > > > + update the system to manual mode as well. > > > > s/update/switch/ ? > > > > But my English is poor, please keep your version if you think that it's > > better. > > I have no particular preference here, whatever will make it easiest for the > users to understand what is happening. I'll take your suggestion! > > [...snip...] > > > > +/* > > > + * A null weighted_interleave_state is interpted as having .mode = "auto", > > > + * and .iw_table is interpreted as an array of 1s with length nr_node_ids. > > > + */ > > > > Better to move the comments to above "wi_state" definition? > > > > > +struct weighted_interleave_state { > > > + bool mode_auto; > > > + u8 iw_table[]; > > > +}; > > > + > > > > Why do you put the type definition in mempolicy.h instead of > > mempolicy.c? I don't find other users except mempolicy.c. > > Good point, I'll move the definition to mempolicy.c and move the comment > to the wi_state definition as well. > > [...snip...] > > > > @@ -3450,31 +3555,104 @@ static ssize_t node_show(struct kobject *kobj, struct kobj_attribute *attr, > > > static ssize_t node_store(struct kobject *kobj, struct kobj_attribute *attr, > > > const char *buf, size_t count) > > > { > > > + struct weighted_interleave_state *new_wi_state, *old_wi_state = NULL; > > > struct iw_node_attr *node_attr; > > > - u8 *new; > > > - u8 *old; > > > u8 weight = 0; > > > + int i; > > > > > > node_attr = container_of(attr, struct iw_node_attr, kobj_attr); > > > if (count == 0 || sysfs_streq(buf, "")) > > > weight = 0; > > > > According to revised ABI, we should return -EINVAL here? > > Great catch, I completely ignored the ABI description that I wrote... > I'll go ahead and just return -EINVAL here! > > [...snip...] > > > > +static ssize_t weighted_interleave_auto_store(struct kobject *kobj, > > > + struct kobj_attribute *attr, const char *buf, size_t count) > > > +{ > > > + struct weighted_interleave_state *new_wi_state, *old_wi_state = NULL; > > > + unsigned int *bw; > > > + bool input; > > > + int i; > > > + > > > + if (kstrtobool(buf, &input)) > > > + return -EINVAL; > > > + > > > + new_wi_state = kzalloc(struct_size(new_wi_state, iw_table, nr_node_ids), > > > + GFP_KERNEL); > > > + if (!new_wi_state) > > > + return -ENOMEM; > > > + for (i = 0; i < nr_node_ids; i++) > > > + new_wi_state->iw_table[i] = 1; > > > + > > > + mutex_lock(&wi_state_lock); > > > + if (!input) { > > > > If input == old_wi_state->mode_auto, we can return directly? > > Yes, that makes sense to me. > > > > static void wi_cleanup(void) { > > > + sysfs_remove_file(&wi_group->wi_kobj, &wi_group->auto_kobj_attr.attr); > > > > Why not just > > > > sysfs_remove_file(&wi_group->wi_kobj, &wi_auto_attr.attr); > > > > ? > > Also makes sense!! > > > --- > > Best Regards, > > Huang, Ying > > Thank you for your great feedback Ying, I'll make changes based on > your suggestions and shortly send up a v9. I hope you have a great day! > Joshua >