Re: [Kernel Bug] WARNING in mempool_alloc_noprof

Re: [Kernel Bug] WARNING in mempool_alloc_noprof

[Vlastimil Babka]

On 2/3/26 17:59, Christoph Hellwig wrote:
> On Tue, Feb 03, 2026 at 05:55:27PM +0100, Vlastimil Babka wrote:
>> On 2/3/26 17:27, Christoph Hellwig wrote:
>> > On Tue, Feb 03, 2026 at 06:52:39PM +0900, Harry Yoo wrote:
>> >> Maybe the changelog could be rephrased a bit,
>> >> but overall LGTM, thanks!
>> > 
>> > 
>> > No, that does not make sense.  If mempool is used with __GFP_RECLAIM in
>> > the flags it won't fail, and if it isn't, GFP_NOFAIL can't work.
>> 
>> So that means as long as there's __GFP_RECLAIM, __GFP_NOFAIL isn't wrong,
>> just redundant.
> 
> Given how picky the rest of the mm is about __GFP_NOFAIL, silently
> accepting it where it has no (or a weird and unexpected) effect
> seems like a disservice to the users.

OK then. But I don't think we need to add checks to the mempool hot paths.
If somebody uses __GFP_NOFAIL, eventually it will trickle to the existing
warning that triggered here. If it's using slab then eventually that will
reach the page allocator too. Maybe not with some custom alloc functions,
but meh.

This f2fs_encrypt_one_page() case is weird though (and the relevant parts
seem to be identical in current mainline).
It uses GFP_NOFS, so __GFP_RECLAIM is there. It only adds __GFP_NOFAIL in
case fscrypt_encrypt_pagecache_blocks() already failed with -ENOMEM.

That means fscrypt_alloc_bounce_page() returns NULL, which is either the
WARN_ON_ONCE(!fscrypt_bounce_page_pool) case (but the report doesn't include
such a warning), or mempool_alloc() failed - but that shouldn't happen with
GFP_NOFS?

(Also the !fscrypt_bounce_page_pool is therefore an infinite retry loop,
isn't it? Which would be truly a bug, unless I'm missing something.)

Ah but fscrypt_encrypt_pagecache_blocks() can also return -ENOMEM due to
fscrypt_crypt_data_unit() returning it.

And there theoretically in v6.12.11 skcipher_request_alloc() could return
-ENOMEM. In practice I assume this report was achieved by fault injection.
But that possibility is gone with mainline commit 52e7e0d88933 ("fscrypt:
Switch to sync_skcipher and on-stack requests") anyway.

I think the whole "goto retry_encrypt;" loop in f2fs_encrypt_one_page()
should just be removed.

Re: [Kernel Bug] WARNING in mempool_alloc_noprof

[Eric Biggers]

On Tue, Feb 03, 2026 at 07:30:07PM +0100, Vlastimil Babka wrote:
> On 2/3/26 17:59, Christoph Hellwig wrote:
> > On Tue, Feb 03, 2026 at 05:55:27PM +0100, Vlastimil Babka wrote:
> >> On 2/3/26 17:27, Christoph Hellwig wrote:
> >> > On Tue, Feb 03, 2026 at 06:52:39PM +0900, Harry Yoo wrote:
> >> >> Maybe the changelog could be rephrased a bit,
> >> >> but overall LGTM, thanks!
> >> > 
> >> > 
> >> > No, that does not make sense.  If mempool is used with __GFP_RECLAIM in
> >> > the flags it won't fail, and if it isn't, GFP_NOFAIL can't work.
> >> 
> >> So that means as long as there's __GFP_RECLAIM, __GFP_NOFAIL isn't wrong,
> >> just redundant.
> > 
> > Given how picky the rest of the mm is about __GFP_NOFAIL, silently
> > accepting it where it has no (or a weird and unexpected) effect
> > seems like a disservice to the users.
> 
> OK then. But I don't think we need to add checks to the mempool hot paths.
> If somebody uses __GFP_NOFAIL, eventually it will trickle to the existing
> warning that triggered here. If it's using slab then eventually that will
> reach the page allocator too. Maybe not with some custom alloc functions,
> but meh.
> 
> This f2fs_encrypt_one_page() case is weird though (and the relevant parts
> seem to be identical in current mainline).
> It uses GFP_NOFS, so __GFP_RECLAIM is there. It only adds __GFP_NOFAIL in
> case fscrypt_encrypt_pagecache_blocks() already failed with -ENOMEM.
> 
> That means fscrypt_alloc_bounce_page() returns NULL, which is either the
> WARN_ON_ONCE(!fscrypt_bounce_page_pool) case (but the report doesn't include
> such a warning), or mempool_alloc() failed - but that shouldn't happen with
> GFP_NOFS?
> 
> (Also the !fscrypt_bounce_page_pool is therefore an infinite retry loop,
> isn't it? Which would be truly a bug, unless I'm missing something.)
> 
> Ah but fscrypt_encrypt_pagecache_blocks() can also return -ENOMEM due to
> fscrypt_crypt_data_unit() returning it.
> 
> And there theoretically in v6.12.11 skcipher_request_alloc() could return
> -ENOMEM. In practice I assume this report was achieved by fault injection.
> But that possibility is gone with mainline commit 52e7e0d88933 ("fscrypt:
> Switch to sync_skcipher and on-stack requests") anyway.
> 
> I think the whole "goto retry_encrypt;" loop in f2fs_encrypt_one_page()
> should just be removed.

Indeed, fscrypt_encrypt_pagecache_blocks() (or the older code it was
derived from) used to do multiple memory allocations.  Now it only
allocates the bounce page itself.

Also, the intended usage is what ext4 does: use GFP_NOFS for the first
page in the bio for a guaranteed allocation from the mempool, then
GFP_NOWAIT for any subsequent pages.  If any of the subsequent
allocations fails, ext4 submits the bio early and starts a new one.

f2fs does it differently and just always uses GFP_NOFS.  Yes, that
doesn't make sense.  I guess ideally it would be changed to properly do
opportunistic allocations in the same way as ext4.  But until then, just
removing the retry loop sounds good.

- Eric