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: <20250305162829.86650-1-joshua.hahnjy@gmail.com>
Date: Wed,  5 Mar 2025 11:28:29 -0500
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Yunjeong Mun <yunjeong.mun@...com>
Cc: honggyu.kim@...com,
	gregkh@...uxfoundation.org,
	rakie.kim@...com,
	akpm@...ux-foundation.org,
	rafael@...nel.org,
	lenb@...nel.org,
	dan.j.williams@...el.com,
	Jonathan.Cameron@...wei.com,
	dave.jiang@...el.com,
	horen.chuang@...ux.dev,
	hannes@...xchg.org,
	linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org,
	linux-mm@...ck.org,
	kernel-team@...a.com,
	kernel_team@...ynix.com
Subject: Re: [PATCH 1/2 v6] mm/mempolicy: Weighted Interleave Auto-tuning

Hi Yunjeong,

On Wed,  5 Mar 2025 18:49:11 +0900 Yunjeong Mun <yunjeong.mun@...com> wrote:

> Hi Joshua, thanks for reviewing my patch and for your kind explanation.

[...snip...]

> > > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > > > index 50cbb7c047fa..65a7e2baf161 100644
> > > > --- a/mm/mempolicy.c
> > > > +++ b/mm/mempolicy.c
> > > > @@ -176,47 +176,22 @@ static u8 get_il_weight(int node)
> > > >  static void reduce_interleave_weights(unsigned int *bw, u8 *new_iw)
> > > >  {
> > > > 	u64 sum_bw = 0;
> > > > -	unsigned int cast_sum_bw, sum_iw = 0;
> > > > -	unsigned int scaling_factor = 1, iw_gcd = 1;
> > > > +	unsigned int scaling_factor = 1, iw_gcd = 0;
> > > > 	int nid;
> > > > 
> > > > 	/* Recalculate the bandwidth distribution given the new info */
> > > > 	for_each_node_state(nid, N_MEMORY)
> > > > 		sum_bw += bw[nid];
> > > > 
> > > > -       for (nid = 0; nid < nr_node_ids; nid++) {
> > > >  			[...snip...]
> > 			^^^^^^^^^^^^
> > When I was originally writing the response, I missed reviewing the contents
> > inside this snipped section, which looks like this:
> > 		if (!node_state(nid, N_MEMORY)) {
> > 			new_iw[nid] = 1;
> > 			continue;
> > 		}
> > I introduced this check in v6 because without this, we end up with the
> > possibility of memoryless nodes having a 0 in the table, which can lead to some
> > problems down the line (e.g. div by 0 in alloc_pages_bulk_weighted_interleave).
> 
> To prevent division by 0 errors, how about setting new_iw to 1 when it is first 
> created, instead of setting it in the reduce function?

I think this makes sense. The original motivation for including it in
reduce_interleave_weights is because this function is usually called on newly
allocated tables, so I thought I would just combine the functionality of
initializing the table and reducing weights into one function. Howver, I now
see that there are actually a few spots when either a table is initialized
but this function isn't called, or when an already-initialized table is
given to this function.

The other rationale was that it seems a bit silly to go through and set all
weights to 1, and then immediately overwrite them with the reduced interleave
weights. With that said, none of this code is in any critical section, I'm
sure that going through one more iteration and setting the weights to 1 is
not unreasonable. 

[...snip...]

> > 
> > Respectfully, I would prefer to write my own version that takes your
> > suggestion, as opposed to applying this patch directly on top of mine so that
> > we do not introduce the build error or the potential div0. However, v7 will
> > include your suggestion, so it will go through only one loop as opposed to two.
> 
> Thanks for considering my suggestion. I look forward to the v7.
> 
> Best regards,
> Yunjeong

Thank you again for your suggestions, Yunjeong! I'll re-write the code
to incorporate them in v7. I hope you have a great day!
Joshua

Sent using hkml (https://github.com/sjp38/hackermail)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ