[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zadm5r/23tonKeXB@memverge.com>
Date: Wed, 17 Jan 2024 00:34:30 -0500
From: Gregory Price <gregory.price@...verge.com>
To: "Huang, Ying" <ying.huang@...el.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
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.
> > + 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
Powered by blists - more mailing lists