The mempool wake-up mechanism has a edge case bug that affects pools
created with min_nr=0. When a thread blocks waiting for memory from an
empty pool (curr_nr == 0), subsequent mempool_free() calls fail to wake
the waiting thread because the condition "curr_nr < min_nr" evaluates
to "0 < 0" which is false, this causes threads to sleep indefinitely.

There is at least 2 places where the mempool created with min_nr=0:

1. lib/btree.c:191: mempool_create(0, btree_alloc, btree_free, NULL)
2. drivers/md/dm-verity-fec.c:791:
    mempool_init_slab_pool(&f->extra_pool, 0, f->cache)

Add an explicit check in mempool_free() to handle the min_nr=0 case:
when the pool has zero minimum reserves, is currently empty, and has
active waiters, wake them up. The wq_has_sleeper() avoids unnecessary
wake-ups when no threads are waiting.

Signed-off-by: Yadan Fan <ydfan@suse.com>
---
  mm/mempool.c | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

diff --git a/mm/mempool.c b/mm/mempool.c
index 3223337135d0..803f8853e0f1 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -545,6 +545,22 @@ void mempool_free(void *element, mempool_t *pool)
                 }
                 spin_unlock_irqrestore(&pool->lock, flags);
         }
+       /*
+        * Handle the min_nr = 0 edge case:
+        * For zero-minimum pools, curr_nr < min_nr (0 < 0) never succeeds,
+        * so waiters sleeping on pool->wait would never be woken by the
+        * normal wake-up path. This explicit check ensures that when
+        * pool->min_nr == 0 and pool->curr_nr == 0, any active waiters
+        * are properly awakened.
+        * The wq_has_sleeper() avoids unnecessary wake-ups when no
+        * threads are waiting.
+        */
+       if (unlikely(pool->min_nr == 0 &&
+                    READ_ONCE(pool->curr_nr) == 0 &&
+                    wq_has_sleeper(&pool->wait))) {
+               wake_up(&pool->wait);
+       }
+
         pool->free(element, pool->pool_data);
  }
  EXPORT_SYMBOL(mempool_free);
-- 
2.50.1

On Wed, 16 Jul 2025 23:37:30 +0800 Yadan Fan <ydfan@suse.com> wrote:

> The mempool wake-up mechanism has a edge case bug that affects pools
> created with min_nr=0. When a thread blocks waiting for memory from an
> empty pool (curr_nr == 0), subsequent mempool_free() calls fail to wake
> the waiting thread because the condition "curr_nr < min_nr" evaluates
> to "0 < 0" which is false, this causes threads to sleep indefinitely.
> 
> There is at least 2 places where the mempool created with min_nr=0:
> 
> 1. lib/btree.c:191: mempool_create(0, btree_alloc, btree_free, NULL)
> 2. drivers/md/dm-verity-fec.c:791:
>     mempool_init_slab_pool(&f->extra_pool, 0, f->cache)

This is very old code.  Can you suggest why this has taken so long to
surface?

Which is a roundabout way of asking "should this be backported into
-stable kernels".  For that we'd need to know how this issue is
affecting our users.

> Add an explicit check in mempool_free() to handle the min_nr=0 case:
> when the pool has zero minimum reserves, is currently empty, and has
> active waiters, wake them up. The wq_has_sleeper() avoids unnecessary
> wake-ups when no threads are waiting.

Do we need the separate test?  What's wrong with the obvious approach
of replacing the "<" with "<=" in the preceding test?

And would the previous (ie, existing) test benefit from the
wq_has_sleeper() check?

> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -545,6 +545,22 @@ void mempool_free(void *element, mempool_t *pool)
>                  }
>                  spin_unlock_irqrestore(&pool->lock, flags);
>          }
> +       /*
> +        * Handle the min_nr = 0 edge case:
> +        * For zero-minimum pools, curr_nr < min_nr (0 < 0) never succeeds,
> +        * so waiters sleeping on pool->wait would never be woken by the
> +        * normal wake-up path. This explicit check ensures that when
> +        * pool->min_nr == 0 and pool->curr_nr == 0, any active waiters
> +        * are properly awakened.
> +        * The wq_has_sleeper() avoids unnecessary wake-ups when no
> +        * threads are waiting.
> +        */
> +       if (unlikely(pool->min_nr == 0 &&
> +                    READ_ONCE(pool->curr_nr) == 0 &&
> +                    wq_has_sleeper(&pool->wait))) {
> +               wake_up(&pool->wait);
> +       }
> +

Something strange is happening with the whitespace here.  I pretty much
retyped the patch.  Please have a chat with your email client ;)

Hi Andrew,

On 7/17/25 05:19, Andrew Morton wrote:
> On Wed, 16 Jul 2025 23:37:30 +0800 Yadan Fan <ydfan@suse.com> wrote:
> 
>> The mempool wake-up mechanism has a edge case bug that affects pools
>> created with min_nr=0. When a thread blocks waiting for memory from an
>> empty pool (curr_nr == 0), subsequent mempool_free() calls fail to wake
>> the waiting thread because the condition "curr_nr < min_nr" evaluates
>> to "0 < 0" which is false, this causes threads to sleep indefinitely.
>>
>> There is at least 2 places where the mempool created with min_nr=0:
>>
>> 1. lib/btree.c:191: mempool_create(0, btree_alloc, btree_free, NULL)
>> 2. drivers/md/dm-verity-fec.c:791:
>>     mempool_init_slab_pool(&f->extra_pool, 0, f->cache)
> 
> This is very old code.  Can you suggest why this has taken so long to
> surface?
> 
> Which is a roundabout way of asking "should this be backported into
> -stable kernels".  For that we'd need to know how this issue is
> affecting our users.

There is no real issue yet, I just reviewed the codes here and found this,
I thought it may needs to fix so that I sent this patch.

> 
>> Add an explicit check in mempool_free() to handle the min_nr=0 case:
>> when the pool has zero minimum reserves, is currently empty, and has
>> active waiters, wake them up. The wq_has_sleeper() avoids unnecessary
>> wake-ups when no threads are waiting.
> 
> Do we need the separate test?  What's wrong with the obvious approach
> of replacing the "<" with "<=" in the preceding test?

Simply changing to "<=" has problem since add_element() has 
"BUG_ON(pool->curr_nr >= pool->min_nr);".

> 
> And would the previous (ie, existing) test benefit from the
> wq_has_sleeper() check?

I think it could have benefit for the existing test, wq_has_sleeper() is 
cost cheaper than wake_up().

I will submit a new patch containing it.

> 
>> --- a/mm/mempool.c
>> +++ b/mm/mempool.c
>> @@ -545,6 +545,22 @@ void mempool_free(void *element, mempool_t *pool)
>>                  }
>>                  spin_unlock_irqrestore(&pool->lock, flags);
>>          }
>> +       /*
>> +        * Handle the min_nr = 0 edge case:
>> +        * For zero-minimum pools, curr_nr < min_nr (0 < 0) never succeeds,
>> +        * so waiters sleeping on pool->wait would never be woken by the
>> +        * normal wake-up path. This explicit check ensures that when
>> +        * pool->min_nr == 0 and pool->curr_nr == 0, any active waiters
>> +        * are properly awakened.
>> +        * The wq_has_sleeper() avoids unnecessary wake-ups when no
>> +        * threads are waiting.
>> +        */
>> +       if (unlikely(pool->min_nr == 0 &&
>> +                    READ_ONCE(pool->curr_nr) == 0 &&
>> +                    wq_has_sleeper(&pool->wait))) {
>> +               wake_up(&pool->wait);
>> +       }
>> +
> 
> Something strange is happening with the whitespace here.  I pretty much
> retyped the patch.  Please have a chat with your email client ;)
> 

Sorry for this, I may just messed up somehow my client configuration, will fix it.

Thanks,
Yadan