Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Thu, 7 Dec 2023 13:56:07 -0800
From:   Davidlohr Bueso <dave@stgolabs.net>
To:     Gregory Price <gourry.memverge@gmail.com>
Cc:     linux-mm@kvack.org, jgroves@micron.com, ravis.opensrc@micron.com,
        sthanneeru@micron.com, emirakhur@micron.com, Hasan.Maruf@amd.com,
        linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-api@vger.kernel.org, linux-arch@vger.kernel.org,
        linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
        arnd@arndb.de, tglx@linutronix.de, luto@kernel.org,
        mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
        x86@kernel.org, hpa@zytor.com, mhocko@kernel.org, tj@kernel.org,
        ying.huang@intel.com, gregory.price@memverge.com, corbet@lwn.net,
        rakie.kim@sk.com, hyeongtak.ji@sk.com, honggyu.kim@sk.com,
        vtavarespetr@micron.com, peterz@infradead.org
Subject: Re: [RFC PATCH 01/11] mm/mempolicy: implement the sysfs-based
 weighted_interleave interface
Message-ID: <uxqkbmqbvcvx6wc3g2h6vhkutv5flrq6rslwdfs7pa6kknupwh@a245pbtfqfgj>
Mail-Followup-To: Gregory Price <gourry.memverge@gmail.com>, 
        linux-mm@kvack.org, jgroves@micron.com, ravis.opensrc@micron.com, 
        sthanneeru@micron.com, emirakhur@micron.com, Hasan.Maruf@amd.com, 
        linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, 
        linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, 
        arnd@arndb.de, tglx@linutronix.de, luto@kernel.org, mingo@redhat.com, 
        bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, 
        mhocko@kernel.org, tj@kernel.org, ying.huang@intel.com, gregory.price@memverge.com, 
        corbet@lwn.net, rakie.kim@sk.com, hyeongtak.ji@sk.com, honggyu.kim@sk.com, 
        vtavarespetr@micron.com, peterz@infradead.org
References: <20231207002759.51418-1-gregory.price@memverge.com>
 <20231207002759.51418-2-gregory.price@memverge.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20231207002759.51418-2-gregory.price@memverge.com>
User-Agent: NeoMutt/20231006
Precedence: bulk

On Wed, 06 Dec 2023, Gregory Price wrote:

>Signed-off-by: Rakie Kim <rakie.kim@sk.com>
>Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
>Co-developed-by: Gregory Price <gregory.price@memverge.com>
>Signed-off-by: Gregory Price <gregory.price@memverge.com>
>Co-developed-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
>Signed-off-by: Hyeongtak Ji <hyeongtak.ji@sk.com>

fyi Rakie's tag needs to be last, per the From.

...

>+What:		/sys/kernel/mm/mempolicy/weighted_interleave/
>+Date:		December 2023
>+Contact:	Linux memory management mailing list <linux-mm@kvack.org>
>+Description:	Configuration Interface for the Weighted Interleave policy
>+
>+What:		/sys/kernel/mm/mempolicy/weighted_interleave/nodeN/
>+Date:		December 2023
>+Contact:	Linux memory management mailing list <linux-mm@kvack.org>
>+Description:	Configuration interface for accesses initiated from nodeN
>+
>+		The directory to configure access initiator weights for nodeN.
>+
>+		Possible numa nodes which have not been marked as a CPU node
>+		at boot will not have a nodeN directory made for them at boot.

This could be better rephrased without the negation. ie:

"Only numa nodes with CPUs (compute) will have a nodeN directory."

>+		Hotplug for CPU nodes is not supported.

Can this even happen? Hot-adding a previously offlined CPU won't change/add a
new numa node. So just rm the line altogether?

>+
>+What:		/sys/kernel/mm/mempolicy/weighted_interleave/nodeN/nodeM
>+		/sys/kernel/mm/mempolicy/weighted_interleave/nodeN/nodeM/weight
>+Date:		December 2023
>+Contact:	Linux memory management mailing list <linux-mm@kvack.org>
>+Description:	Configuration interface for target nodes accessed from nodeNN
>+
>+		The interleave weight for a memory node (M) from initiating
>+		node (N). These weights are utilized by processes which have set
>+		the mempolicy to MPOL_WEIGHTED_INTERLEAVE and have opted into
>+		global weights by omitting a task-local weight array.
>+
>+		These weights only affect new allocations, and changes at runtime
>+		will not cause migrations on already allocated pages.
>+
>+		If the weight of 0 is desired, the appropriate way to do this is
>+		by removing the node from the weighted interleave nodemask.
>+
>+		Minimum weight: 1
>+		Maximum weight: 255
>diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>index 10a590ee1c89..ce332b5e7a03 100644
>--- a/mm/mempolicy.c
>+++ b/mm/mempolicy.c
>@@ -131,6 +131,11 @@ static struct mempolicy default_policy = {
>
> static struct mempolicy preferred_node_policy[MAX_NUMNODES];
>
>+struct interleave_weight_table {
>+	unsigned char weights[MAX_NUMNODES];
>+};
>+static struct interleave_weight_table *iw_table;
>+
> /**
>  * numa_nearest_node - Find nearest node by state
>  * @node: Node id to start the search
>@@ -3067,3 +3072,224 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
>		p += scnprintf(p, buffer + maxlen - p, ":%*pbl",
>			       nodemask_pr_args(&nodes));
> }
>+
>+struct iw_node_info {
>+	struct kobject kobj;
>+	int src;
>+	int dst;
>+};
>+
>+static ssize_t node_weight_show(struct kobject *kobj,
>+				struct kobj_attribute *attr, char *buf)
>+{
>+	struct iw_node_info *node_info = container_of(kobj, struct iw_node_info,
>+						      kobj);
>+	return sysfs_emit(buf, "%d\n",
>+			  iw_table[node_info->src].weights[node_info->dst]);
>+}
>+
>+static ssize_t node_weight_store(struct kobject *kobj,
>+				 struct kobj_attribute *attr,
>+				 const char *buf, size_t count)
>+{
>+	unsigned char weight = 0;
>+	struct iw_node_info *node_info = NULL;
>+
>+	node_info = container_of(kobj, struct iw_node_info, kobj);
>+
>+	if (kstrtou8(buf, 0, &weight) || !weight)
>+		return -EINVAL;
>+
>+	iw_table[node_info->src].weights[node_info->dst] = weight;
>+
>+	return count;
>+}

iw_table will need some (basic) form of serialization.

...

>+static int __init mempolicy_sysfs_init(void)
>+{
>+	int err, nid;
>+	int cpunodes = 0;
>+	struct kobject *root_kobj;
>+
>+	for_each_node_state(nid, N_CPU)
>+		cpunodes += 1;
>+	iw_table = kmalloc_array(cpunodes, sizeof(*iw_table), GFP_KERNEL);
>+	if (!iw_table) {
>+		pr_err("failed to create interleave weight table\n");
>+		err = -ENOMEM;
>+		goto fail_obj;

No ref here yet, just return -ENOMEM.

>+	}
>+	memset(iw_table, 1, cpunodes * sizeof(*iw_table));
>+
>+	root_kobj = kzalloc(sizeof(struct kobject), GFP_KERNEL);
>+	if (!root_kobj)
>+		return -ENOMEM;
>+
>+	kobject_init(root_kobj, &mempolicy_kobj_ktype);
>+	err = kobject_add(root_kobj, mm_kobj, "mempolicy");
>+	if (err) {
>+		pr_err("failed to add kobject to the system\n");
>+		goto fail_obj;
>+	}
>+
>+	err = sysfs_create_group(root_kobj, &mempolicy_attr_group);
>+	if (err) {
>+		pr_err("failed to register mempolicy group\n");
>+		goto fail_obj;
>+	}
>+
>+	err = add_weighted_interleave_group(root_kobj);
>+fail_obj:
>+	if (err)
>+		kobject_put(root_kobj);
>+	return err;
>+
>+}
>+late_initcall(mempolicy_sysfs_init);
>--
>2.39.1
>
>