On 2025/1/3 0:38, Luiz Capitulino wrote: > On 2024-12-25 07:36, Yunsheng Lin wrote: >> On 2024/12/24 6:00, Luiz Capitulino wrote: >> >>> /* >>> - * __alloc_pages_bulk - Allocate a number of order-0 pages to a list or array >>> + * __alloc_pages_bulk - Allocate a number of order-0 pages to an array >>> * @gfp: GFP flags for the allocation >>> * @preferred_nid: The preferred NUMA node ID to allocate from >>> * @nodemask: Set of nodes to allocate from, may be NULL >>> - * @nr_pages: The number of pages desired on the list or array >>> - * @page_list: Optional list to store the allocated pages >>> - * @page_array: Optional array to store the pages >>> + * @nr_pages: The number of pages desired in the array >>> + * @page_array: Array to store the pages >>> * >>> * This is a batched version of the page allocator that attempts to >>> - * allocate nr_pages quickly. Pages are added to page_list if page_list >>> - * is not NULL, otherwise it is assumed that the page_array is valid. >>> + * allocate nr_pages quickly. Pages are added to the page_array. >>> * >>> - * For lists, nr_pages is the number of pages that should be allocated. >>> - * >>> - * For arrays, only NULL elements are populated with pages and nr_pages >>> + * Note that only NULL elements are populated with pages and nr_pages >> >> It is not really related to this patch, but while we are at this, the above >> seems like an odd behavior. By roughly looking at all the callers of that >> API, it seems like only the below callers rely on that? >> fs/erofs/zutil.c: z_erofs_gbuf_growsize() >> fs/xfs/xfs_buf.c: xfs_buf_alloc_pages() >> >> It seems it is quite straight forward to change the above callers to not >> rely on the above behavior, and we might be able to avoid more checking >> by removing the above behavior? > > Hi Yunsheng, > > Assuming the use-case is valid, I think we might want to keep common code > in the API vs. duplicating it in callers? I was thinking maybe adding a wrapper/helper around the __alloc_pages_bulk() to avoid the overhead for other usecase and make the semantics more obvious if it is an valid use-case. > > In any case, even if we decide to go for your suggestion, I'd prefer not > to grow the scope of this series since this could delay its inclusion. > Dropping the list-API (and dead code) is actually important so IMHO it > should go in first. Sure. >