Re: [PATCH 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving

[Gregory Price <gregory.price@memverge.com>]

On Mon, Jan 15, 2024 at 01:47:31PM +0800, Huang, Ying wrote:
> Gregory Price <gourry.memverge@gmail.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