linux-kernel - Re: [External Mail] [RFC PATCH] mm/mempolicy: Weighted interleave auto-tuning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241213195754.2676135-1-joshua.hahnjy@gmail.com>
Date: Fri, 13 Dec 2024 11:57:31 -0800
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: hyeonggon.yoo@...com
Cc: "gourry@...rry.net" <gourry@...rry.net>,
	kernel_team@...ynix.com,
	"rafael@...nel.org" <rafael@...nel.org>,
	"lenb@...nel.org" <lenb@...nel.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	김홍규(KIM HONGGYU) System SW <honggyu.kim@...com>,
	"ying.huang@...ux.alibaba.com" <ying.huang@...ux.alibaba.com>,
	김락기(KIM RAKIE) System SW <rakie.kim@...com>,
	"dan.j.williams@...el.com" <dan.j.williams@...el.com>,
	"Jonathan.Cameron@...wei.com" <Jonathan.Cameron@...wei.com>,
	"dave.jiang@...el.com" <dave.jiang@...el.com>,
	"horen.chuang@...ux.dev" <horen.chuang@...ux.dev>,
	"hannes@...xchg.org" <hannes@...xchg.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"kernel-team@...a.com" <kernel-team@...a.com>
Subject: Re: [External Mail] [RFC PATCH] mm/mempolicy: Weighted interleave auto-tuning

On Fri, 13 Dec 2024 15:19:20 +0900 Hyeonggon Yoo <hyeonggon.yoo@...com> wrote:

> On 2024-12-11 06:54 AM, Joshua Hahn wrote:
> > This patch introduces an auto-configuration for the interleave weights
> > that aims to balance the two goals of setting node weights to be
> > proportional to their bandwidths and keeping the weight values low.
> > This balance is controlled by a value max_node_weight, which defines the
> > maximum weight a single node can take.
> 
> Hi Joshua,
> 
> I am wondering how this is going to work for host memory + CXL memory 
> interleaving. I guess by "the ACPI table" you mean the ACPI HMAT or CXL 
> CDAT, both of which does not provide the bandwidth of host memory.
> If this feature does not consider the bandwidth of host memory, manual 
> configuration will be inevitable anyway.

Hi Hyeonggon,

Thank you for reviewing my patch! As Gregory showed in his reply,
I think it would be possible to get host bandwidth information
using the ACPI HMAT.

[-----8<-----]

> > +What:		/sys/kernel/mm/mempolicy/weighted_interleave/max_node_weight
> > +Date:		December 2024
> > +Contact:	Linux memory management mailing list <linux-mm@...ck.org>
> > +Description:	Weight limiting / scaling interface
> > +
> > +		The maximum interleave weight for a memory node. When it is
> > +		updated, any previous changes to interleave weights (i.e. via
> > +		the nodeN sysfs interfaces) are ignored, and new weights are
> > +		calculated using ACPI-reported bandwidths and scaled.
> > +
> 
> At first this paragraph sounded like "previously stored weights are 
> discarded after setting max_node_weight", but I think you mean
> "User can override the default values, but defaults values are 
> calculated regardless of the values set by the user". Right?

In the implementation, the first way you interpreted is the correct
description. That is, if a user manually changes a ndoe weight,
then updates the max_node_weight, the previous manual change will
be overwritten by the newly scaled values.

Does this behavior make sense? Perhaps it makes sense to ignore
user-changed values when doing the re-scaling if a user decides to
change the max_node_weight value themselves. 

Regardless of what implementation makes sense, I can re-write the
description so that there is no ambiguity when it comes to the
expected behavior of the code. Thank you for pointing this out!

> [...snip...]
> 
> > +int mempolicy_set_node_perf(unsigned int node, struct access_coordinate *coords)
> > +{
> > +	unsigned long *old_bw, *new_bw;
> > +	unsigned long bw_val;
> > +	u8 *old_iw, *new_iw;
> > +
> > +	/*
> > +	 * Bandwidths above this limit causes rounding errors when reducing
> > +	 * weights. This value is ~16 exabytes, which is unreasonable anyways.
> > +	 */
> > +	bw_val = min(coords->read_bandwidth, coords->write_bandwidth);
> > +	if (bw_val > (U64_MAX / 10))
> > +		return -EINVAL;
> > +
> > +	new_bw = kcalloc(nr_node_ids, sizeof(unsigned long), GFP_KERNEL);
> > +	if (!new_bw)
> > +		return -ENOMEM;
> > +
> > +	new_iw = kzalloc(nr_node_ids, GFP_KERNEL);
> 
> I think kcalloc(nr_node_ids, sizeof(u8), GFP_KERNEL); will be more readable.

I see, thank you for your input. I will make this change in a v2.

> > @@ -2012,11 +2105,12 @@ static unsigned int weighted_interleave_nid(struct mempolicy *pol, pgoff_t ilx)
> >   
> >   	rcu_read_lock();
> >   	table = rcu_dereference(iw_table);
> > +	defaults = rcu_dereference(iw_table);
> 
> Probably you intended rcu_dereference(default_iw_table)?

Yes -- thank you for the catch. I will also make this change.

> >   static struct iw_node_attr **node_attrs;
> > +static struct kobj_attribute *max_nw_attr;
> 
> Where is max_nw_attr initialized?

Oh thank you for this catch! You are correct, max_nw_attr is never
initalized. Actually, there is a typo in which I never use
max_nw_attr, I accidentally rename a different sysfs interface
to act as the intended max_nw_attr. I will make this change as well
and post a v2. 
 
> Best,
> Hyeonggon

Thank you for your input, I will make the changes that you mentioned
regardnig readability & typos. I hope to hear from you regarding the
thoughts on the behavior of re-scaling all node weights when users
update max_node_weight, and whether that should overwrite manually
set node weights.

Have a great day!
Joshua