On Thu, 06 Mar 2025, Yunsheng Lin wrote: > On 2025/3/6 7:41, NeilBrown wrote: > > On Wed, 05 Mar 2025, Yunsheng Lin wrote: > >> > >> For the existing btrfs and sunrpc case, I am agreed that there > >> might be valid use cases too, we just need to discuss how to > >> meet the requirements of different use cases using simpler, more > >> unified and effective APIs. > > > > We don't need "more unified". > > What I meant about 'more unified' is how to avoid duplicated code as > much as possible for two different interfaces with similar functionality. > > The best way I tried to avoid duplicated code as much as possible is > to defragment the page_array before calling the alloc_pages_bulk() > for the use case of btrfs and sunrpc so that alloc_pages_bulk() can > be removed of the assumption populating only NULL elements, so that > the API is simpler and more efficient. > > > > > If there are genuinely two different use cases with clearly different > > needs - even if only slightly different - then it is acceptable to have > > two different interfaces. Be sure to choose names which emphasise the > > differences. > > The best name I can come up with for the use case of btrfs and sunrpc > is something like alloc_pages_bulk_refill(), any better suggestion about > the naming? I think alloc_pages_bulk_refill() is a good name. So: - alloc_pages_bulk() would be given an uninitialised array of page pointers and a required count and would return the number of pages that were allocated - alloc_pages_bulk_refill() would be given an initialised array of page pointers some of which might be NULL. It would attempt to allocate pages for the non-NULL pointers and return the total number of allocated pages in the array - just like the current alloc_pages_bulk(). sunrpc could usefully use both of these interfaces. alloc_pages_bulk() could be implemented by initialising the array and then calling alloc_pages_bulk_refill(). Or alloc_pages_bulk_refill() could be implemented by compacting the pages and then calling alloc_pages_bulk(). If we could duplicate the code and have two similar but different functions. The documentation for _refill() should make it clear that the pages might get re-ordered. Having looked at some of the callers I agree that the current interface is not ideal for many of them, and that providing a simpler interface would help. Thanks, NeilBrown
