Re: [PATCH] mm: simplify the mempool_alloc_bulk API

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 6/2/26 18:00, Christoph Hellwig wrote:
> The mempool_alloc_bulk was modelled after the alloc_pages_bulk API,
> including some misunderstanding of it.
> 
> Remove checking for NULL slots in the array, as alloc_pages_bulk and
> kmem_cache_alloc_bulk always fill the array from the beginning and thus
> we know the offset of the first failing allocation.  This removes support
> for working well with alloc_pages_bulk used to refill page arrays that
> might have an entry removed from in the middle, but that is only used by
> sunrpc and hopefully on it's way out.

Importantly sunrpc doesn't use mempool_alloc_bulk() so we're not affecting
it here.

> Also remove the allocated parameter as it is redundant because the caller
> can simply specific and offset into the entries array.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Somewhat late, but small and affects only one caller. So applied to
slab/for-7.2/alloc_bulk on top of the kmem_cache_alloc_bulk() change.

> ---
>  block/blk-crypto-fallback.c |  9 +++++----
>  include/linux/mempool.h     |  2 +-
>  mm/mempool.c                | 27 ++++++++++-----------------
>  3 files changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
> index 61f595410832..ab6924fba280 100644
> --- a/block/blk-crypto-fallback.c
> +++ b/block/blk-crypto-fallback.c
> @@ -199,8 +199,8 @@ static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src,
>  	pages += nr_segs * (PAGE_PTRS_PER_BVEC - 1);
>  
>  	/*
> -	 * Try a bulk allocation first.  This could leave random pages in the
> -	 * array unallocated, but we'll fix that up later in mempool_alloc_bulk.
> +	 * Try a bulk allocation first.  This might not fill all allocated
> +	 * pages, but we'll fix that up later in mempool_alloc_bulk.
>  	 *
>  	 * Note: alloc_pages_bulk needs the array to be zeroed, as it assumes
>  	 * any non-zero slot already contains a valid allocation.
> @@ -208,8 +208,9 @@ static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src,
>  	memset(pages, 0, sizeof(struct page *) * nr_segs);
>  	nr_allocated = alloc_pages_bulk(GFP_KERNEL, nr_segs, pages);
>  	if (nr_allocated < nr_segs)
> -		mempool_alloc_bulk(blk_crypto_bounce_page_pool, (void **)pages,
> -				nr_segs, nr_allocated);
> +		mempool_alloc_bulk(blk_crypto_bounce_page_pool,
> +				(void **)pages + nr_allocated,
> +				nr_segs - nr_allocated);
>  	memalloc_noio_restore(memflags);
>  	*pages_ret = pages;
>  	return bio;
> diff --git a/include/linux/mempool.h b/include/linux/mempool.h
> index e8e440e04a06..a0fa6d43e0dc 100644
> --- a/include/linux/mempool.h
> +++ b/include/linux/mempool.h
> @@ -66,7 +66,7 @@ void *mempool_alloc_noprof(struct mempool *pool, gfp_t gfp_mask) __malloc;
>  #define mempool_alloc(...)						\
>  	alloc_hooks(mempool_alloc_noprof(__VA_ARGS__))
>  int mempool_alloc_bulk_noprof(struct mempool *pool, void **elem,
> -		unsigned int count, unsigned int allocated);
> +		unsigned int count);
>  #define mempool_alloc_bulk(...)						\
>  	alloc_hooks(mempool_alloc_bulk_noprof(__VA_ARGS__))
>  
> diff --git a/mm/mempool.c b/mm/mempool.c
> index db23e0eef652..473a029fa31f 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -419,12 +419,8 @@ static unsigned int mempool_alloc_from_pool(struct mempool *pool, void **elems,
>  	spin_lock_irqsave(&pool->lock, flags);
>  	if (unlikely(pool->curr_nr < count - allocated))
>  		goto fail;
> -	for (i = 0; i < count; i++) {
> -		if (!elems[i]) {
> -			elems[i] = remove_element(pool);
> -			allocated++;
> -		}
> -	}
> +	while (allocated < count)
> +		elems[allocated++] = remove_element(pool);
>  	spin_unlock_irqrestore(&pool->lock, flags);
>  
>  	/* Paired with rmb in mempool_free(), read comment there. */
> @@ -479,22 +475,21 @@ static inline gfp_t mempool_adjust_gfp(gfp_t *gfp_mask)
>   * @pool:	pointer to the memory pool
>   * @elems:	partially or fully populated elements array
>   * @count:	number of entries in @elem that need to be allocated
> - * @allocated:	number of entries in @elem already allocated
>   *
> - * Allocate elements for each slot in @elem that is non-%NULL. This is done by
> - * first calling into the alloc_fn supplied at pool initialization time, and
> - * dipping into the reserved pool when alloc_fn fails to allocate an element.
> + * Allocate @count elements into @elems.  This is done by first calling into the
> + * alloc_fn supplied at pool initialization time, and dipping into the reserved
> + * pool when alloc_fn fails to allocate an element.
>   *
>   * On return all @count elements in @elems will be populated.
>   *
>   * Return: Always 0.  If it wasn't for %$#^$ alloc tags, it would return void.
>   */
>  int mempool_alloc_bulk_noprof(struct mempool *pool, void **elems,
> -		unsigned int count, unsigned int allocated)
> +		unsigned int count)
>  {
>  	gfp_t gfp_mask = GFP_KERNEL;
>  	gfp_t gfp_temp = mempool_adjust_gfp(&gfp_mask);
> -	unsigned int i = 0;
> +	unsigned int allocated = 0;
>  
>  	VM_WARN_ON_ONCE(count > pool->min_nr);
>  	might_alloc(gfp_mask);
> @@ -514,11 +509,9 @@ int mempool_alloc_bulk_noprof(struct mempool *pool, void **elems,
>  	 * Try to allocate the elements using the allocation callback first as
>  	 * that might succeed even when the caller's bulk allocation did not.
>  	 */
> -	for (i = 0; i < count; i++) {
> -		if (elems[i])
> -			continue;
> -		elems[i] = pool->alloc(gfp_temp, pool->pool_data);
> -		if (unlikely(!elems[i]))
> +	while (allocated < count) {
> +		elems[allocated] = pool->alloc(gfp_temp, pool->pool_data);
> +		if (unlikely(!elems[allocated]))
>  			goto use_pool;
>  		allocated++;
>  	}