Re: [PATCH 06/11] mempool: factor out a mempool_alloc_from_pool helper

[Hugh Dickins <hughd@google.com>]

On Thu, 13 Nov 2025, Christoph Hellwig wrote:

> Add a helper for the mempool_alloc slowpath to better separate it from the
> fast path, and also use it to implement mempool_alloc_preallocated which
> shares the same logic.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
...
> @@ -413,8 +457,6 @@ void *mempool_alloc_noprof(mempool_t *pool, gfp_t gfp_mask)
>  {
>  	gfp_t gfp_temp = mempool_adjust_gfp(&gfp_mask);
>  	void *element;
> -	unsigned long flags;
> -	wait_queue_entry_t wait;
>  
>  	VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO);
>  	might_alloc(gfp_mask);
> @@ -428,53 +470,22 @@ void *mempool_alloc_noprof(mempool_t *pool, gfp_t gfp_mask)
>  		element = pool->alloc(gfp_temp, pool->pool_data);
>  	}
>  
> -	if (likely(element))
> -		return element;
> -
> -	spin_lock_irqsave(&pool->lock, flags);
> -	if (likely(pool->curr_nr)) {
> -		element = remove_element(pool);
> -		spin_unlock_irqrestore(&pool->lock, flags);
> -		/* paired with rmb in mempool_free(), read comment there */
> -		smp_wmb();
> +	if (unlikely(!element)) {
>  		/*
> -		 * Update the allocation stack trace as this is more useful
> -		 * for debugging.
> +		 * Try to allocate an element from the pool.
> +		 *
> +		 * The first pass won't have __GFP_DIRECT_RECLAIM and won't
> +		 * sleep in mempool_alloc_from_pool.  Retry the allocation
> +		 * with all flags set in that case.
>  		 */
> -		kmemleak_update_trace(element);
> -		return element;
> -	}
> -
> -	/*
> -	 * We use gfp mask w/o direct reclaim or IO for the first round.  If
> -	 * alloc failed with that and @pool was empty, retry immediately.
> -	 */
> -	if (gfp_temp != gfp_mask) {
> -		spin_unlock_irqrestore(&pool->lock, flags);
> -		gfp_temp = gfp_mask;
> -		goto repeat_alloc;
> -	}
> -
> -	/* We must not sleep if !__GFP_DIRECT_RECLAIM */
> -	if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
> -		spin_unlock_irqrestore(&pool->lock, flags);
> -		return NULL;
> +		element = mempool_alloc_from_pool(pool, gfp_mask);
> +		if (!element && gfp_temp != gfp_mask) {

No, that is wrong, it breaks the mempool promise: linux-next oopses
in swap_writepage_bdev_async(), which relies on bio_alloc(,,,GFP_NOIO)
to return a good bio.

The refactoring makes it hard to see, but the old version always used
to go back to repeat_alloc at the end, if __GFP_DIRECT_RECLAIM,
whereas here it only does so the first time, when gfp_temp != gfp_mask.

After bisecting to here, I changed that "gfp_temp != gfp_mask" to
"(gfp & __GFP_DIRECT_RECLAIM)", and it worked again.  But other patches
have come in on top, so below is a patch to the final mm/mempool.c...

> +			gfp_temp = gfp_mask;
> +			goto repeat_alloc;
> +		}
>  	}
>  
> -	/* Let's wait for someone else to return an element to @pool */
> -	init_wait(&wait);
> -	prepare_to_wait(&pool->wait, &wait, TASK_UNINTERRUPTIBLE);
> -
> -	spin_unlock_irqrestore(&pool->lock, flags);
> -
> -	/*
> -	 * FIXME: this should be io_schedule().  The timeout is there as a
> -	 * workaround for some DM problems in 2.6.18.
> -	 */
> -	io_schedule_timeout(5*HZ);
> -
> -	finish_wait(&pool->wait, &wait);
> -	goto repeat_alloc;
> +	return element;
>  }
>  EXPORT_SYMBOL(mempool_alloc_noprof);
...

[PATCH] mempool: fix NULL from mempool_alloc_noprof()

mempool_alloc_noprof() with __GFP_DIRECT_RECLAIM used to loop until it
had allocated an element, but recently regressed to returning NULL when
pool->alloc and mempool_alloc_from_pool() have both failed twice, causing
oops in __swap_writepage() (and presumably others relying on mempool).

Fixes: 1d091d2c5bf3 ("mempool: factor out a mempool_alloc_from_pool helper")
Signed-off-by: Hugh Dickins <hughd@google.com>
---
 mm/mempool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mempool.c b/mm/mempool.c
index 1558601600ba..dc9f2a1dc35f 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -576,7 +576,7 @@ void *mempool_alloc_noprof(struct mempool *pool, gfp_t gfp_mask)
 		 * with all flags set in that case.
 		 */
 		if (!mempool_alloc_from_pool(pool, &element, 1, 0, gfp_mask) &&
-		    gfp_temp != gfp_mask) {
+		    (gfp_mask & __GFP_DIRECT_RECLAIM)) {
 			gfp_temp = gfp_mask;
 			goto repeat_alloc;
 		}
-- 
2.51.0