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