Re: [PATCH 3/9] mempool: add mempool_{alloc,free}_bulk

Eric Biggers
2025-11-07T03:53:47Z

On Fri, Oct 31, 2025 at 10:34:33AM +0100, Christoph Hellwig wrote:
> /**
> - * mempool_alloc - allocate an element from a memory pool
> + * mempool_alloc_bulk - allocate multiple elements from a memory pool
> * @pool: pointer to the memory pool
> + * @elem: partially or fully populated elements array
> + * @count: size (in entries) of @elem
> * @gfp_mask: GFP_* flags.
> *
> + * Allocate elements for each slot in @elem that is non-%NULL.

elem => elems (and likewise for mempool_free_bulk())

> * Note: This function only sleeps if the alloc_fn callback sleeps or returns
> * %NULL. Using __GFP_ZERO is not supported.
> *
> - * Return: pointer to the allocated element or %NULL on error. This function
> - * never returns %NULL when @gfp_mask allows sleeping.
> + * Return: 0 if successful, else -ENOMEM. This function never returns -ENOMEM
> + * when @gfp_mask allows sleeping.
> */
> -void *mempool_alloc_noprof(mempool_t *pool, gfp_t gfp_mask)
> +int mempool_alloc_bulk_noprof(struct mempool *pool, void **elem,
> + unsigned int count, gfp_t gfp_mask, unsigned long caller_ip)

What exactly is the behavior on partial failures? Is the return value 0
or is it -ENOMEM, and is the array restored to its original state or
might some elements have been allocated?

> +/**
> + * mempool_alloc - allocate an element from a memory pool
> + * @pool: pointer to the memory pool
> + * @gfp_mask: GFP_* flags.
> + *
> + * Note: This function only sleeps if the alloc_fn callback sleeps or returns
> + * %NULL. Using __GFP_ZERO is not supported.
> + *
> + * Return: pointer to the allocated element or %NULL on error. This function
> + * never returns %NULL when @gfp_mask allows sleeping.
> + */
> +void *mempool_alloc_noprof(struct mempool *pool, gfp_t gfp_mask)
> +{
> + void *elem[1] = { };
> +
> + if (mempool_alloc_bulk_noprof(pool, elem, 1, gfp_mask, _RET_IP_) < 0)
> + return NULL;
> + return elem[0];
> +}
> EXPORT_SYMBOL(mempool_alloc_noprof);

How much overhead does this add to mempool_alloc(), which will continue
to be the common case? I wonder if it would be worthwhile to
force-inline the bulk allocation function into it, so that it will get
generate about the same code as before.

> if (unlikely(READ_ONCE(pool->curr_nr) < pool->min_nr)) {
> spin_lock_irqsave(&pool->lock, flags);
> - if (likely(pool->curr_nr < pool->min_nr)) {
> - add_element(pool, element);
> - spin_unlock_irqrestore(&pool->lock, flags);
> - if (wq_has_sleeper(&pool->wait))
> - wake_up(&pool->wait);
> - return;
> + while (pool->curr_nr < pool->min_nr && freed < count) {
> + add_element(pool, elem[freed++]);
> + added = true;
> }
> spin_unlock_irqrestore(&pool->lock, flags);
> - }
>
> /*
> * Handle the min_nr = 0 edge case:
> @@ -572,20 +614,41 @@ void mempool_free(void *element, mempool_t *pool)
> * allocation of element when both min_nr and curr_nr are 0, and
> * any active waiters are properly awakened.
> */

The above comment has a weird position now. Maybe move it into the
'else if' block below.

> - if (unlikely(pool->min_nr == 0 &&
> + } else if (unlikely(pool->min_nr == 0 &&
> READ_ONCE(pool->curr_nr) == 0)) {
> spin_lock_irqsave(&pool->lock, flags);
> if (likely(pool->curr_nr == 0)) {
> - add_element(pool, element);
> - spin_unlock_irqrestore(&pool->lock, flags);
> - if (wq_has_sleeper(&pool->wait))
> - wake_up(&pool->wait);
> - return;
> + add_element(pool, elem[freed++]);
> + added = true;
> }
> spin_unlock_irqrestore(&pool->lock, flags);
> }
>
> - pool->free(element, pool->pool_data);
> + if (unlikely(added) && wq_has_sleeper(&pool->wait))
> + wake_up(&pool->wait);
> +
> + return freed;
> +}
> +EXPORT_SYMBOL_GPL(mempool_free_bulk);

- Eric