Re: [PATCH v2] mm: alloc_pages_bulk: remove assumption of populating only NULL elements

[Yunsheng Lin]

On 3/8/2025 5:02 AM, NeilBrown wrote:

...

> > >    allocated pages in the array - just like the current
> > >    alloc_pages_bulk().
> >
> > I guess 'the total number of allocated pages in the array ' include
> > the pages which are already in the array before calling the above
> > API?
>
> Yes - just what the current function does.
> Though I don't know that we really need that detail.
> I think there are three interesting return values:
>
> - hard failure - don't bother trying again soon:   maybe -ENOMEM
> - success - all pages are allocated:  maybe 0 (or 1?)
> - partial success - at least one page allocated, ok to try again
>    immediately - maybe -EAGAIN (or 0).

Yes, the above makes sense. And I guess returning '-ENOMEM' & '0' &
'-EAGAIN' seems like a more explicit value.

> >

...

> >
>
> If I were do work on this (and I'm not, so you don't have to follow my
> ideas) I would separate the bulk_alloc into several inline functions and
> combine them into the different interfaces that you want.  This will
> result in duplicated object code without duplicated source code.  The
> object code should be optimal.

Thanks for the detailed suggestion, it seems feasible.
If the 'add to a linked list' dispose was not removed in the [1],
I guess it is worth trying.
But I am not sure if it is still worth it at the cost of the above
mentioned 'duplicated object code' considering the array defragmenting
seem to be able to unify the dispose of 'add to end of array' and
'add to next hole in array'.

I guess I can try with the easier one using array defragmenting first,
and try below if there is more complicated use case.

1. 
https://lore.kernel.org/all/f1c75db91d08cafd211eca6a3b199b629d4ffe16.1734991165.git.luizcap@xxxxxxxxxx/

> The parts of the function are:
>   - validity checks - fallback to single page allocation
>   - select zone - fallback to single page allocation
>   - allocate multiple pages in the zone and dispose of them
>   - allocate a single page
>
> The "dispose of them" is one of
>    - add to a linked list
>    - add to end of array
>    - add to next hole in array
>
> These three could be inline functions that the "allocate multiple pages"
> and "allocate single page" functions call.  We can pass these as
> function arguments and the compile will inline them.
> I imagine these little function would take one page and return
> a bool indicating if any more are wanted.
>
> The three functions: alloc_bulk_array alloc_bulk_list
> alloc_bulk_refill_array would each look like:
>
>    validity checks: do we need to allocate anything?
>
>    if want more than one page &&
>       am allowed to do mulipage (e.g. not __GFP_ACCOUNT) &&
>       zone = choose_zone() {
>          alloc_multi_from_zone(zone, dispose_function)
>    }
>    if nothing allocated
>       alloc_single_page(dispose_function)
>
> Each would have a different dispose_function and the initial checks
> would be quite different, as would the return value.
>
> Thanks for working on this.
>
> NeilBrown