linux-kernel - Re: [PATCH 3/3] mm/mempolicy: introduce MPOL_WEIGHTED

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: <87jzo7zay6.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Thu, 18 Jan 2024 09:28:49 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Gregory Price <gregory.price@...verge.com>
Cc: Gregory Price <gourry.memverge@...il.com>,  <linux-mm@...ck.org>,
  <linux-kernel@...r.kernel.org>,  <linux-doc@...r.kernel.org>,
  <linux-fsdevel@...r.kernel.org>,  <linux-api@...r.kernel.org>,
  <corbet@....net>,  <akpm@...ux-foundation.org>,  <honggyu.kim@...com>,
  <rakie.kim@...com>,  <hyeongtak.ji@...com>,  <mhocko@...nel.org>,
  <vtavarespetr@...ron.com>,  <jgroves@...ron.com>,
  <ravis.opensrc@...ron.com>,  <sthanneeru@...ron.com>,
  <emirakhur@...ron.com>,  <Hasan.Maruf@....com>,
  <seungjun.ha@...sung.com>,  <hannes@...xchg.org>,
  <dan.j.williams@...el.com>,  Srinivasulu Thanneeru
 <sthanneeru.opensrc@...ron.com>
Subject: Re: [PATCH 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE
 for weighted interleaving

Gregory Price <gregory.price@...verge.com> writes:

> On Mon, Jan 15, 2024 at 01:47:31PM +0800, Huang, Ying wrote:
>> Gregory Price <gourry.memverge@...il.com> writes:
>> 
>> > +	/* Continue allocating from most recent node and adjust the nr_pages */
>> > +	if (pol->wil.cur_weight) {
>> > +		node = next_node_in(me->il_prev, nodes);
>> > +		node_pages = pol->wil.cur_weight;
>> > +		if (node_pages > rem_pages)
>> > +			node_pages = rem_pages;
>> > +		nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
>> > +						  NULL, page_array);
> ... snip ...
>> > +			if (delta > weight) {
>> > +				node_pages += weight;
>> > +				delta -= weight;
>> > +			} else {
>> > +				node_pages += delta;
>> > +				delta = 0;
>> > +			}
>> > +		}
>> > +		nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
>> > +						  NULL, page_array);
>> 
>> Should we check nr_allocated here?  Allocation may fail anyway.
>> 
>
> I thought about this briefly in both situations.
>
> If you look at alloc_pages_bulk_array_interleave(), it does not fail if
> __alloc_pages_bulk() fails, instead it continues and attempts to
> allocate from the remaining nodes.
>
> Presumably, this is because the caller of the bulk allocator can accept
> a partial-failure and will go ahead and allocate the remaining pages on
> an extra slow path.
>
> Since alloc_pages_bulk_array_interleave() appears to be capable of
> failing in the exact same way, I considered this safe.

You are right.  We should proceed with next node here.

--
Best Regards,
Huang, Ying

>> > +	if (pol->mode == MPOL_WEIGHTED_INTERLEAVE)
>> > +		return alloc_pages_bulk_array_weighted_interleave(gfp, pol,
>> > +								  nr_pages,
>> > +								  page_array);
>> > +
>> 
>> Just nit-pick, may be better to be 
>> 
>> 		return alloc_pages_bulk_array_weighted_interleave(
>>                                 gfp, pol, nr_pages, page_array);
>>
>
> Wasn't sure on style when names get this long lol, will make the change
> :]
>
>
>
> Probably v2 thursday or friday
>
> Regards
> ~Gregory