Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Herbert Xu]

On Wed, Oct 28, 2026 at 11:42:18AM -0400, Liam R. Howlett (Google) wrote:
>
> Below, one CPU adds an FDB entry for an RTM_NEWNEIGH netlink request,
> while another CPU is inside vmalloc() or vfree():
> 
>     RTM_NEWNEIGH                      vmalloc() or vfree()
>     ============                      ====================
>     spin_lock_bh(&br->hash_lock)
>                                       spin_lock(&vn->busy.lock)
>     rhashtable_insert_rehash()
>       kvmalloc(GFP_ATOMIC)
>         vmalloc()
>           spin_lock(&vn->busy.lock)
>             waits for the other CPU
>                                       softirq: a frame for the bridge
>                                         spin_lock(&br->hash_lock)
>                                           waits for RTM_NEWNEIGH

I think this is a fundamental bug in the kvmalloc non-blocking
support patch.  How can it be used at all if it is susceptible
to such dead-locks?

And if you can't use it safely why don't we just get rid of the
broken kvmalloc non-block path and have it always go to kmalloc
like it did before?
 
> Fixes: c6307674ed82 ("mm: kvmalloc: add non-blocking support for vmalloc")

BTW, rhashtable patches should be sent to linux-crypto.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Uladzislau Rezki]

On Thu, Oct 29, 2026 at 10:16:36AM +0800, Herbert Xu wrote:
> On Wed, Oct 28, 2026 at 11:42:18AM -0400, Liam R. Howlett (Google) wrote:
> >
> > Below, one CPU adds an FDB entry for an RTM_NEWNEIGH netlink request,
> > while another CPU is inside vmalloc() or vfree():
> > 
> >     RTM_NEWNEIGH                      vmalloc() or vfree()
> >     ============                      ====================
> >     spin_lock_bh(&br->hash_lock)
> >                                       spin_lock(&vn->busy.lock)
> >     rhashtable_insert_rehash()
> >       kvmalloc(GFP_ATOMIC)
> >         vmalloc()
> >           spin_lock(&vn->busy.lock)
> >             waits for the other CPU
> >                                       softirq: a frame for the bridge
> >                                         spin_lock(&br->hash_lock)
> >                                           waits for RTM_NEWNEIGH
> 
> I think this is a fundamental bug in the kvmalloc non-blocking
> support patch.  How can it be used at all if it is susceptible
> to such dead-locks?
> 
> And if you can't use it safely why don't we just get rid of the
> broken kvmalloc non-block path and have it always go to kmalloc
> like it did before?
>  
If we have a look at this as we did not have vmalloc() here:


     RTM_NEWNEIGH                      foo() or ...
     ============                      ====================
     lock_bh(&br->hash_lock)
                                       foo()
                                        lock(B)
     rhashtable_insert_rehash()
         foo()
           lock(B)
             waits for the other CPU
                                       softirq: a frame for the bridge
                                         lock(&br->hash_lock)
                                           waits for RTM_NEWNEIGH

Same issue?

I have a question:

GFP_ATOMIC/GFP_NOWAIT guarantee they do not sleep. Do they guarantee
not hitting ABBA dead-locks or they are not interrupted?

--
Uladzislau Rezki

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Liam R. Howlett (Google)]

Sorry for missing the ML in my initial email, I missed it from
get_maintainers.  I should have really clued in with such a small cc
list ;)

On 26/10/29 11:08AM, Uladzislau Rezki wrote:
> On Thu, Oct 29, 2026 at 10:16:36AM +0800, Herbert Xu wrote:
> > On Wed, Oct 28, 2026 at 11:42:18AM -0400, Liam R. Howlett (Google) wrote:
> > >
> > > Below, one CPU adds an FDB entry for an RTM_NEWNEIGH netlink request,
> > > while another CPU is inside vmalloc() or vfree():
> > > 
> > >     RTM_NEWNEIGH                      vmalloc() or vfree()
> > >     ============                      ====================
> > >     spin_lock_bh(&br->hash_lock)
> > >                                       spin_lock(&vn->busy.lock)
> > >     rhashtable_insert_rehash()
> > >       kvmalloc(GFP_ATOMIC)
> > >         vmalloc()
> > >           spin_lock(&vn->busy.lock)
> > >             waits for the other CPU
> > >                                       softirq: a frame for the bridge
> > >                                         spin_lock(&br->hash_lock)
> > >                                           waits for RTM_NEWNEIGH
> > 
> > I think this is a fundamental bug in the kvmalloc non-blocking
> > support patch.  How can it be used at all if it is susceptible
> > to such dead-locks?
> > 
> > And if you can't use it safely why don't we just get rid of the
> > broken kvmalloc non-block path and have it always go to kmalloc
> > like it did before?
> >  
> If we have a look at this as we did not have vmalloc() here:
> 
> 
>      RTM_NEWNEIGH                      foo() or ...
>      ============                      ====================
>      lock_bh(&br->hash_lock)
>                                        foo()
>                                         lock(B)
>      rhashtable_insert_rehash()
>          foo()
>            lock(B)
>              waits for the other CPU
>                                        softirq: a frame for the bridge
>                                          lock(&br->hash_lock)
>                                            waits for RTM_NEWNEIGH
> 
> Same issue?

Looks like it to me.

> 
> I have a question:
> 
> GFP_ATOMIC/GFP_NOWAIT guarantee they do not sleep. Do they guarantee
> not hitting ABBA dead-locks or they are not interrupted?

GFP_ATOMIC and GFP_NOWAIT can be interrupted.  It's more about not
waiting for reclaim (kswapd might still run) and GFP_ATOMIC might dip
into reserves.

The issue is that kvmalloc() now falls back to vmalloc() for
non-blocking flags (I am restating this to make sure I'm not missing
something here, please correct me).

So this changes what is called internal to rhastable so it doesn't reach
vmalloc(), like before, if the caller cannot block - they go to
kmalloc() with the old fall back.

I *think* the question is if the change should be here in the caller,
internal to mm, or a new call to opt-in.

Considering this is the only kvmalloc() caller that forwards
non-blocking gfp from BH lock + softirq context, I think that it makes
sense to do it in the caller.

Given where we are in the release, this fixes a real issue that can
happen so maybe we take this now and settle the general handling later?

Thanks,
Liam

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Michal Hocko]

On Thu 29-10-26 10:53:56, Liam R. Howlett (Google) wrote:
> 
> Sorry for missing the ML in my initial email, I missed it from
> get_maintainers.  I should have really clued in with such a small cc
> list ;)
> 
> On 26/10/29 11:08AM, Uladzislau Rezki wrote:
> > On Thu, Oct 29, 2026 at 10:16:36AM +0800, Herbert Xu wrote:
> > > On Wed, Oct 28, 2026 at 11:42:18AM -0400, Liam R. Howlett (Google) wrote:
> > > >
> > > > Below, one CPU adds an FDB entry for an RTM_NEWNEIGH netlink request,
> > > > while another CPU is inside vmalloc() or vfree():
> > > > 
> > > >     RTM_NEWNEIGH                      vmalloc() or vfree()
> > > >     ============                      ====================
> > > >     spin_lock_bh(&br->hash_lock)
> > > >                                       spin_lock(&vn->busy.lock)
> > > >     rhashtable_insert_rehash()
> > > >       kvmalloc(GFP_ATOMIC)
> > > >         vmalloc()
> > > >           spin_lock(&vn->busy.lock)
> > > >             waits for the other CPU
> > > >                                       softirq: a frame for the bridge
> > > >                                         spin_lock(&br->hash_lock)
> > > >                                           waits for RTM_NEWNEIGH
> > > 
> > > I think this is a fundamental bug in the kvmalloc non-blocking
> > > support patch.  How can it be used at all if it is susceptible
> > > to such dead-locks?
> > > 
> > > And if you can't use it safely why don't we just get rid of the
> > > broken kvmalloc non-block path and have it always go to kmalloc
> > > like it did before?
> > >  
> > If we have a look at this as we did not have vmalloc() here:
> > 
> > 
> >      RTM_NEWNEIGH                      foo() or ...
> >      ============                      ====================
> >      lock_bh(&br->hash_lock)
> >                                        foo()
> >                                         lock(B)
> >      rhashtable_insert_rehash()
> >          foo()
> >            lock(B)
> >              waits for the other CPU
> >                                        softirq: a frame for the bridge
> >                                          lock(&br->hash_lock)
> >                                            waits for RTM_NEWNEIGH
> > 
> > Same issue?
> 
> Looks like it to me.
> 
> > 
> > I have a question:
> > 
> > GFP_ATOMIC/GFP_NOWAIT guarantee they do not sleep. Do they guarantee
> > not hitting ABBA dead-locks or they are not interrupted?
> 
> GFP_ATOMIC and GFP_NOWAIT can be interrupted.  It's more about not
> waiting for reclaim (kswapd might still run) and GFP_ATOMIC might dip
> into reserves.
> 
> The issue is that kvmalloc() now falls back to vmalloc() for
> non-blocking flags (I am restating this to make sure I'm not missing
> something here, please correct me).

Isn't the real problem that vmalloc should use irqsave version of the
spinlock to preserve the locking constrains of the caller?
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Herbert Xu]

On Thu, Oct 29, 2026 at 10:53:56AM -0400, Liam R. Howlett (Google) wrote:
>
> Given where we are in the release, this fixes a real issue that can
> happen so maybe we take this now and settle the general handling later?

There is no rush at all since the bug that you found has existed
since 2025:

commit c6307674ed82c0c57d6e1e3408e84ac449ab8e94
Author: Uladzislau Rezki (Sony) <urezki@gmail.com>
Date:   Tue Oct 7 14:20:35 2025 +0200

    mm: kvmalloc: add non-blocking support for vmalloc

Let's take the time to fix this properly.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Uladzislau Rezki]

> On Thu, Oct 29, 2026 at 10:53:56AM -0400, Liam R. Howlett (Google) wrote:
> >
> > Given where we are in the release, this fixes a real issue that can
> > happen so maybe we take this now and settle the general handling later?
> 
> There is no rush at all since the bug that you found has existed
> since 2025:
> 
> commit c6307674ed82c0c57d6e1e3408e84ac449ab8e94
> Author: Uladzislau Rezki (Sony) <urezki@gmail.com>
> Date:   Tue Oct 7 14:20:35 2025 +0200
> 
>     mm: kvmalloc: add non-blocking support for vmalloc
> 
> Let's take the time to fix this properly.
> 
The problem with spin_lock_irqsave() is that vmalloc() path can take
time to alloc and map pages. I am not sure this is worth to switch to
irq-save locking primitive. So this is the "heaviest" allocator in the
kernel. Callers have to be careful how the call vmalloc().

IMO, this caller has to be fixed.

--
Uladzislau Rezki

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Matthew Wilcox]

On Fri, Oct 30, 2026 at 10:52:37AM +0100, Uladzislau Rezki wrote:
> The problem with spin_lock_irqsave() is that vmalloc() path can take
> time to alloc and map pages. I am not sure this is worth to switch to
> irq-save locking primitive. So this is the "heaviest" allocator in the
> kernel. Callers have to be careful how the call vmalloc().
> 
> IMO, this caller has to be fixed.

Which caller, though?  Are you saying rhashtable_insert_rehash()
needs to call kmalloc() instead of kvmalloc()?

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Liam R. Howlett (Google)]

On 26/10/29 10:54AM, Liam R. Howlett (Google) wrote:
> 
> Sorry for missing the ML in my initial email, I missed it from
> get_maintainers.  I should have really clued in with such a small cc
> list ;)
> 
> On 26/10/29 11:08AM, Uladzislau Rezki wrote:
> > On Thu, Oct 29, 2026 at 10:16:36AM +0800, Herbert Xu wrote:
> > > On Wed, Oct 28, 2026 at 11:42:18AM -0400, Liam R. Howlett (Google) wrote:
> > > >
> > > > Below, one CPU adds an FDB entry for an RTM_NEWNEIGH netlink request,
> > > > while another CPU is inside vmalloc() or vfree():
> > > > 
> > > >     RTM_NEWNEIGH                      vmalloc() or vfree()
> > > >     ============                      ====================
> > > >     spin_lock_bh(&br->hash_lock)
> > > >                                       spin_lock(&vn->busy.lock)
> > > >     rhashtable_insert_rehash()
> > > >       kvmalloc(GFP_ATOMIC)
> > > >         vmalloc()
> > > >           spin_lock(&vn->busy.lock)
> > > >             waits for the other CPU
> > > >                                       softirq: a frame for the bridge
> > > >                                         spin_lock(&br->hash_lock)
> > > >                                           waits for RTM_NEWNEIGH
> > > 
> > > I think this is a fundamental bug in the kvmalloc non-blocking
> > > support patch.  How can it be used at all if it is susceptible
> > > to such dead-locks?
> > > 
> > > And if you can't use it safely why don't we just get rid of the
> > > broken kvmalloc non-block path and have it always go to kmalloc
> > > like it did before?
> > >  
> > If we have a look at this as we did not have vmalloc() here:
> > 
> > 
> >      RTM_NEWNEIGH                      foo() or ...
> >      ============                      ====================
> >      lock_bh(&br->hash_lock)
> >                                        foo()
> >                                         lock(B)
> >      rhashtable_insert_rehash()
> >          foo()
> >            lock(B)
> >              waits for the other CPU
> >                                        softirq: a frame for the bridge
> >                                          lock(&br->hash_lock)
> >                                            waits for RTM_NEWNEIGH
> > 
> > Same issue?
> 
> Looks like it to me.
> 
> > 
> > I have a question:
> > 
> > GFP_ATOMIC/GFP_NOWAIT guarantee they do not sleep. Do they guarantee
> > not hitting ABBA dead-locks or they are not interrupted?
> 
> GFP_ATOMIC and GFP_NOWAIT can be interrupted.  It's more about not
> waiting for reclaim (kswapd might still run) and GFP_ATOMIC might dip
> into reserves.
> 
> The issue is that kvmalloc() now falls back to vmalloc() for
> non-blocking flags (I am restating this to make sure I'm not missing
> something here, please correct me).
> 
> So this changes what is called internal to rhastable so it doesn't reach
> vmalloc(), like before, if the caller cannot block - they go to
> kmalloc() with the old fall back.
> 
> I *think* the question is if the change should be here in the caller,
> internal to mm, or a new call to opt-in.
> 
> Considering this is the only kvmalloc() caller that forwards
> non-blocking gfp from BH lock + softirq context, I think that it makes
> sense to do it in the caller.
> 
> Given where we are in the release, this fixes a real issue that can
> happen so maybe we take this now and settle the general handling later?

Hubert, I have another version of this patch set to send out, but this
particular patch remains the same.

Are you okay with keeping it the way it is for this release?

I don't want to force your hand but I also don't want to keep a valid
deadlock scenario in-tree.

Thanks,
Liam

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Herbert Xu]

On Fri, Oct 30, 2026 at 08:56:03PM -0400, Liam R. Howlett (Google) wrote:
>
> Hubert, I have another version of this patch set to send out, but this
> particular patch remains the same.
> 
> Are you okay with keeping it the way it is for this release?
> 
> I don't want to force your hand but I also don't want to keep a valid
> deadlock scenario in-tree.

Instead of your patch, please revert the original change that
added kvmalloc to rhashtable:

commit 93f976b5190df32793908d49165f78e67fcb66cf
Author: Davidlohr Bueso <dave@stgolabs.net>
Date:   Tue Aug 21 22:01:45 2018 -0700

    lib/rhashtable: simplify bucket_table_alloc()

The newly added alloc_hooks_tag call should go after the if statement.

This patch should also be split out of your series and posted to
linux-crypto.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Uladzislau Rezki]

Hello, Matthew!

> On Fri, Oct 30, 2026 at 10:52:37AM +0100, Uladzislau Rezki wrote:
> > The problem with spin_lock_irqsave() is that vmalloc() path can take
> > time to alloc and map pages. I am not sure this is worth to switch to
> > irq-save locking primitive. So this is the "heaviest" allocator in the
> > kernel. Callers have to be careful how the call vmalloc().
> > 
> > IMO, this caller has to be fixed.
> 
> Which caller, though?  Are you saying rhashtable_insert_rehash()
> needs to call kmalloc() instead of kvmalloc()?
>
The problem is that we used to BUG_ON() in vmalloc if soft-irq or BH
is disabled, i.e. for vmalloc it is incorrect state.

See https://lore.kernel.org/all/69ff8c7c.050a0220.1036b8.000b.GAE@google.com/
Two patches to address this:

1. https://lore.kernel.org/all/20260515153009.2296191-1-urezki@gmail.com/
2. https://lore.kernel.org/all/20261026102503.30860-1-urezki@gmail.com/

second can be updated and resent as it checks if the context does soft
irq processing whereas it allows:

bh-disable;
vmalloc();

forbidding this will address the rhashtable deadlock(discussed here) but
this will just fail allocation because it is incorrect state for vmalloc
and calling kvmalloc() for rhashtable will not succeed.

i.e. rhashtable context should be fixed also. This is what i meant by
saying: this caller has to be fixed.

--
Uladzislau Rezki

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Liam R. Howlett (Google)]

On 26/11/01 08:19AM, Herbert Xu wrote:
> On Fri, Oct 30, 2026 at 08:56:03PM -0400, Liam R. Howlett (Google) wrote:
> >
> > Hubert, I have another version of this patch set to send out, but this
> > particular patch remains the same.
> > 
> > Are you okay with keeping it the way it is for this release?
> > 
> > I don't want to force your hand but I also don't want to keep a valid
> > deadlock scenario in-tree.
> 
> Instead of your patch, please revert the original change that
> added kvmalloc to rhashtable:
> 
> commit 93f976b5190df32793908d49165f78e67fcb66cf
> Author: Davidlohr Bueso <dave@stgolabs.net>
> Date:   Tue Aug 21 22:01:45 2018 -0700
> 
>     lib/rhashtable: simplify bucket_table_alloc()
> 

I don't follow.  Reverting that patch gives the same functional flow as
the patch I already sent; kzalloc() for one branch and kvzalloc() for
the other. 

We will, however, restore the useless __GFP_NORETRY flag to one branch,
and a broken check.

-       if (gfp != GFP_KERNEL)
-               tbl = kzalloc(size, gfp | __GFP_NOWARN | __GFP_NORETRY);

gfp != GFP_KERNEL is supposed to just send GFP_ATOMIC in here, but
instead you are sending GFP_KERNEL | __GFP_NOFAIL as well.
gfpflags_allow_blocking() should be used here, right?

The patch you are saying to revert was correct when it went in.

> The newly added alloc_hooks_tag call should go after the if statement.

You are saying to add it to both branches of the if statement, right?

> 
> This patch should also be split out of your series and posted to
> linux-crypto.

Sounds good.

The fixes tag should still point to c6307674ed82, since that's when the
issue became visible.

Thanks,
Liam

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Herbert Xu]

On Mon, Nov 02, 2026 at 08:49:23PM -0500, Liam R. Howlett (Google) wrote:
>
> I don't follow.  Reverting that patch gives the same functional flow as
> the patch I already sent; kzalloc() for one branch and kvzalloc() for
> the other. 

I'm not talking about a literal revert.  But that patch is what
introduced the use of an unconditional kvzalloc for the !GFP_KERNEL
case which is what we are trying to undo.

So it should at least be mentioned prominently in the commit
description.

> We will, however, restore the useless __GFP_NORETRY flag to one branch,
> and a broken check.
> 
> -       if (gfp != GFP_KERNEL)
> -               tbl = kzalloc(size, gfp | __GFP_NOWARN | __GFP_NORETRY);
> 
> gfp != GFP_KERNEL is supposed to just send GFP_ATOMIC in here, but
> instead you are sending GFP_KERNEL | __GFP_NOFAIL as well.
> gfpflags_allow_blocking() should be used here, right?

That made me dig deeper into the original change and why it was
made.  It was actually Linus who suggested merging the two calls.
It's a shame that we have to bring this pattern back.

https://lore.kernel.org/all/CA+55aFxnrwFniJq6nCFn4pWRHLH-Wac9bVFKieBC-TixTsFg=Q@mail.gmail.com/

Let's add him to the discussion and see if he can offer any insights
on how we can avoid undoing this change.

To recap, kvmalloc was modified in 2025 such that it can use vmalloc
even when called with GFP_ATOMIC (commit c6307674ed82).  Unfortunately,
it does so in a way where it has to wait for other CPUs to do certain
work.  As a result, if you call kvmalloc with GFP_ATOMIC under an
external lock, this easily leads to a dead-lock when other CPUs end
up blocking on the same external lock.

Hence rhashtable is being asked to undo the change and reintroduce
this pattern to avoid the dead-lock:

	if (!gfpflags_allow_blocking(gfp))
		tbl = kzalloc(size, gfp | __GFP_NOWARN | __GFP_NORETRY);
	else
		tbl = kvzalloc(size, gfp);

My feeling is that the new kvmalloc is simply broken but its author
disagrees.

> You are saying to add it to both branches of the if statement, right?

Nevermind, I didn't look at how alloc_hooks_tag was implemented.
This is simply disgusting.

But instead of

	if (gfpflags_allow_blocking(gfp))
		tbl = alloc_hooks_tag(ht->alloc_tag,
				      kvmalloc_node_align_noprof(
					struct_size(tbl, buckets, nbuckets),
					1, gfp | __GFP_ZERO, NUMA_NO_NODE));
	else
		tbl = alloc_hooks_tag(ht->alloc_tag,
				      kmalloc_noprof(
					struct_size(tbl, buckets, nbuckets),
					gfp | __GFP_ZERO));

, plesae do

	if (gfpflags_allow_blocking(gfp))
		tbl = alloc_hooks_tag(ht->alloc_tag,
				      kvmalloc_noprof(
				        struct_size(tbl, buckets, nbuckets),
				        gfp | __GFP_ZERO));
	else
		tbl = alloc_hooks_tag(ht->alloc_tag,
				      kzalloc_noprof(
				        struct_size(tbl, buckets, nbuckets),
				        gfp));

It would be even nicer if you could add a kvzalloc_noprof helper but
it's not a deal-breaker.

> The fixes tag should still point to c6307674ed82, since that's when the
> issue became visible.

Sure that sounds good.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Linus Torvalds]

On Tue, 3 Nov 2026 at 19:58, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> To recap, kvmalloc was modified in 2025 such that it can use vmalloc
> even when called with GFP_ATOMIC (commit c6307674ed82).  Unfortunately,
> it does so in a way where it has to wait for other CPUs to do certain
> work.  As a result, if you call kvmalloc with GFP_ATOMIC under an
> external lock, this easily leads to a dead-lock when other CPUs end
> up blocking on the same external lock.

Ok, so it sounds like that change was clearly complete garbage and needs fixing.

The only reason for GFP_ATOMIC is that you're in a context where you
can't block. That's the *point*. So if GFP_ATOMIC waits for something,
it's just fundamentally buggy.

Sadly, the commit you point to doesn't talk about *why* that
braindamage was done. Looking at the original thread it seems like it
was for kmsan.

I didn't actually find the kmsan use case, but I didn't search very
hard. I absolutely *DETEST* it when "debug infrastructure" causes
actual bugs and mis-designs. That's the opposite of debugging.

IOW, I guess we do have to split those allocation cases up, and we
damn well should remove GFP_ATOMIC support from vmalloc() again since
it's clearly buggered beyond belief.

              Linus

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Herbert Xu]

On Tue, Nov 03, 2026 at 08:28:36PM -0800, Linus Torvalds wrote:
>
> Sadly, the commit you point to doesn't talk about *why* that
> braindamage was done. Looking at the original thread it seems like it
> was for kmsan.

Here is the link to the whole series that was applied (v4):

https://lore.kernel.org/all/20251007122035.56347-11-urezki@gmail.com/T/#m50cf87c3f12015a56abe5a6359525019851f1b97

This is the original RFC:

https://lore.kernel.org/all/20250704152537.55724-1-urezki@gmail.com/

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Linus Torvalds]

On Tue, 3 Nov 2026 at 21:21, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Tue, Nov 03, 2026 at 08:28:36PM -0800, Linus Torvalds wrote:
> >
> > Sadly, the commit you point to doesn't talk about *why* that
> > braindamage was done. Looking at the original thread it seems like it
> > was for kmsan.
>
> Here is the link to the whole series that was applied (v4):

None of that says why.  It only repeats what I already said: it seems
to be due to kmsan doing bad things.

              Linus

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Herbert Xu]

On Tue, Nov 03, 2026 at 09:39:03PM -0800, Linus Torvalds wrote:
>
> None of that says why.  It only repeats what I already said: it seems
> to be due to kmsan doing bad things.

Fair enough.

When I was told that folks needed vmalloc under GFP_ATOMIC I didn't
question it any further:

https://lore.kernel.org/all/aeiHkG7X_dieMdw5@milan/

Looking back, it does look like it was never explained who these
folks are.

Ulazislau, so who needs vmalloc GFP_ATOMIC support in the kernel?
Or is it only used by out-of-kernel code?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Uladzislau Rezki]

Hello, Herbert!

> >
> > None of that says why.  It only repeats what I already said: it seems
> > to be due to kmsan doing bad things.
> 
> Fair enough.
> 
> When I was told that folks needed vmalloc under GFP_ATOMIC I didn't
> question it any further:
> 
> https://lore.kernel.org/all/aeiHkG7X_dieMdw5@milan/
> 
> Looking back, it does look like it was never explained who these
> folks are.
> 
> Ulazislau, so who needs vmalloc GFP_ATOMIC support in the kernel?
> Or is it only used by out-of-kernel code?
>
There are users in kernel. For example:

drivers/GPU
    /* Allocate the file in vmalloc memory, it's likely to be big */
    iter.start = __vmalloc(file_size, GFP_NOWAIT);

AMD-GPU:
    kvzalloc(size, GFP_NOWAIT);
    kvzalloc_objs(struct amdgpu_coredump_ring, GFP_NOWAIT);
    kvzalloc(ring->ring_size, GFP_NOWAIT);

Virtio ring-testing:
    (alloc_ring(0)), gfpflags_allow_blocking(0) does not allow blocking.

net/ipv4/udp.c:
    err = bpf_iter_udp_realloc_batch(iter, batch_sks, GFP_NOWAIT);

etc...

Apart of above, the obvious user of GFP_ATOMIC is the bridge forwarding
database. The br_fdb_add_local() holds br->hash_lock with bottom halves
disabled, on rehash event it invokes rhashtable_insert_rehash() to allocate
a new bucket table with GFP_ATOMIC | __GFP_NOWARN. In that scenario kvmalloc()
can fallback to vmalloc().

This path runs and exists because syzbot hit the old in_interrupt() check
there, leading to 04aa71da5f3 ("mm/vmalloc: do not trigger BUG() on BH
disabled context").

The vmalloc() fallback improves the chance of completing the insert on
the spot to minimize -ENOMEM hit rate thus minimize queuing the
irq_work_queue(&ht->run_irq_work) to retry allocation in process context.

According to XFS code and comments it wants non-blocking support for
kvmalloc().

But indeed the v4 cover letter didn't name a consumer. The bridge
report came later.

--
Uladzislau Rezki

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Herbert Xu]

On Wed, Nov 04, 2026 at 12:11:24PM +0100, Uladzislau Rezki wrote:
>
> There are users in kernel. For example:

Thanks for the list.

> drivers/GPU
>     /* Allocate the file in vmalloc memory, it's likely to be big */
>     iter.start = __vmalloc(file_size, GFP_NOWAIT);
> 
> AMD-GPU:
>     kvzalloc(size, GFP_NOWAIT);
>     kvzalloc_objs(struct amdgpu_coredump_ring, GFP_NOWAIT);
>     kvzalloc(ring->ring_size, GFP_NOWAIT);

These appear to be executing out of process context without spin
locks so they're not really GFP_ATOMIC.

> Virtio ring-testing:
>     (alloc_ring(0)), gfpflags_allow_blocking(0) does not allow blocking.

I didn't really look into this as it's just test code.

> net/ipv4/udp.c:
>     err = bpf_iter_udp_realloc_batch(iter, batch_sks, GFP_NOWAIT);

This looks buggy in exactly the same way as rhashtable.  It's
called under spin_lock_bh, and the same lock can be taken on
the softirq path on another CPU.

So are there any safe users of kvmalloc with GFP_ATOMIC? If not
I suggest that we change kvmalloc to disallow vmalloc when called
with GFP_ATOMIC.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Linus Torvalds]

On Thu, 5 Nov 2026 at 00:51, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Wed, Nov 04, 2026 at 12:11:24PM +0100, Uladzislau Rezki wrote:
> >
> > There are users in kernel. For example:
>
> Thanks for the list.
>
> > drivers/GPU
> >     /* Allocate the file in vmalloc memory, it's likely to be big */
> >     iter.start = __vmalloc(file_size, GFP_NOWAIT);
> >
> > AMD-GPU:
> >     kvzalloc(size, GFP_NOWAIT);
> >     kvzalloc_objs(struct amdgpu_coredump_ring, GFP_NOWAIT);
> >     kvzalloc(ring->ring_size, GFP_NOWAIT);
>
> These appear to be executing out of process context without spin
> locks so they're not really GFP_ATOMIC.

Historically, GFP_ATOMIC and GFP_NOWAIT have been largely the same
except for GFP_ATOMIC eating into reserves.

But I wonder if we might be able to soften that a bit, or add a new
form, because it looks like the AMD-GPU case at least doesn't really
care about any kind of "don't sleep", what it really just wants is an
"optimistic allocation" - try to get the memory, but don't spend
effort on freeing things if there is not enough memort.

The AMD code in question literally has a comment that says

        /* Dump ring content if memory allocation succeeds. */

so while it's possible that it has some atomicity requirements, it
doesn't look that way to me from a very quick look.

The etnaviv case looks to be the exact same thing: it is definitely
called in normal process context (that code takes a mutex lock around
it all).

So those two cases really don't looke like they are about "atomicity" at all.

> > net/ipv4/udp.c:
> >     err = bpf_iter_udp_realloc_batch(iter, batch_sks, GFP_NOWAIT);
>
> This looks buggy in exactly the same way as rhashtable.  It's
> called under spin_lock_bh, and the same lock can be taken on
> the softirq path on another CPU.

Yeah, that case is definitely actually atomic and holds a lock. Not great.

Is there any way to limit the vmalloc() case to the actuall atomic one
and just fail when it needs to busy-wait?

               Linus

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Uladzislau Rezki]

On Thu, Nov 05, 2026 at 08:22:52AM -0800, Linus Torvalds wrote:
> On Thu, 5 Nov 2026 at 00:51, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >
> > On Wed, Nov 04, 2026 at 12:11:24PM +0100, Uladzislau Rezki wrote:
> > >
> > > There are users in kernel. For example:
> >
> > Thanks for the list.
> >
> > > drivers/GPU
> > >     /* Allocate the file in vmalloc memory, it's likely to be big */
> > >     iter.start = __vmalloc(file_size, GFP_NOWAIT);
> > >
> > > AMD-GPU:
> > >     kvzalloc(size, GFP_NOWAIT);
> > >     kvzalloc_objs(struct amdgpu_coredump_ring, GFP_NOWAIT);
> > >     kvzalloc(ring->ring_size, GFP_NOWAIT);
> >
> > These appear to be executing out of process context without spin
> > locks so they're not really GFP_ATOMIC.
> 
> Historically, GFP_ATOMIC and GFP_NOWAIT have been largely the same
> except for GFP_ATOMIC eating into reserves.
> 
> But I wonder if we might be able to soften that a bit, or add a new
> form, because it looks like the AMD-GPU case at least doesn't really
> care about any kind of "don't sleep", what it really just wants is an
> "optimistic allocation" - try to get the memory, but don't spend
> effort on freeing things if there is not enough memort.
> 
> The AMD code in question literally has a comment that says
> 
>         /* Dump ring content if memory allocation succeeds. */
> 
> so while it's possible that it has some atomicity requirements, it
> doesn't look that way to me from a very quick look.
> 
> The etnaviv case looks to be the exact same thing: it is definitely
> called in normal process context (that code takes a mutex lock around
> it all).
> 
> So those two cases really don't looke like they are about "atomicity" at all.
> 
> > > net/ipv4/udp.c:
> > >     err = bpf_iter_udp_realloc_batch(iter, batch_sks, GFP_NOWAIT);
> >
> > This looks buggy in exactly the same way as rhashtable.  It's
> > called under spin_lock_bh, and the same lock can be taken on
> > the softirq path on another CPU.
> 
> Yeah, that case is definitely actually atomic and holds a lock. Not great.
> 
> Is there any way to limit the vmalloc() case to the actuall atomic one
> and just fail when it needs to busy-wait?
>
Probably it is not easy. vmalloc() takes several locks including
during page-table setup.

Probably the simplest option is to return NULL for non-blocking vmalloc()
allocations when BH are disabled. That would address the reported
rhashtable deadlock.

--
Uladzislau Rezki

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Uladzislau Rezki]

On Thu, Nov 05, 2026 at 04:51:39PM +0800, Herbert Xu wrote:
> On Wed, Nov 04, 2026 at 12:11:24PM +0100, Uladzislau Rezki wrote:
> >
> > There are users in kernel. For example:
> 
> Thanks for the list.
> 
> > drivers/GPU
> >     /* Allocate the file in vmalloc memory, it's likely to be big */
> >     iter.start = __vmalloc(file_size, GFP_NOWAIT);
> > 
> > AMD-GPU:
> >     kvzalloc(size, GFP_NOWAIT);
> >     kvzalloc_objs(struct amdgpu_coredump_ring, GFP_NOWAIT);
> >     kvzalloc(ring->ring_size, GFP_NOWAIT);
> 
> These appear to be executing out of process context without spin
> locks so they're not really GFP_ATOMIC.
> 
> > Virtio ring-testing:
> >     (alloc_ring(0)), gfpflags_allow_blocking(0) does not allow blocking.
> 
> I didn't really look into this as it's just test code.
> 
> > net/ipv4/udp.c:
> >     err = bpf_iter_udp_realloc_batch(iter, batch_sks, GFP_NOWAIT);
> 
> This looks buggy in exactly the same way as rhashtable.  It's
> called under spin_lock_bh, and the same lock can be taken on
> the softirq path on another CPU.
> 
> So are there any safe users of kvmalloc with GFP_ATOMIC? If not
> I suggest that we change kvmalloc to disallow vmalloc when called
> with GFP_ATOMIC.
> 
kvmalloc() disallow and vmalloc() allow?

The deadlock looks like from below spin_lock():

<snip>
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index e4570bbed854..97aac4a4a92b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1025,7 +1025,13 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 			}
 		}
 	} else {
-		spin_lock(&br->hash_lock);
+		if (in_serving_softirq()) {
+			if (!spin_trylock(&br->hash_lock))
+				return;
+		} else {
+			spin_lock(&br->hash_lock);
+		}
+
 		fdb = fdb_create(br, source, addr, vid, flags);
 		if (fdb) {
 			trace_br_fdb_update(br, source, addr, vid, flags);
<snip>

is that applicable for bridge update code?

--
Uladzislau Rezki

Re: [PATCH 4/4] rhashtable: fix vmalloc() deadlock in atomic rehash

[Linus Torvalds]

On Thu, 5 Nov 2026 at 10:44, Uladzislau Rezki <urezki@gmail.com> wrote:
>
> @@ -1025,7 +1025,13 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>                         }
>                 }
>         } else {
> -               spin_lock(&br->hash_lock);
> +               if (in_serving_softirq()) {
> +                       if (!spin_trylock(&br->hash_lock))

I don't think that in_serving_softirq() makes any sense.

If I read this code correctly - and it's possible I didn't - this is
always called under local_bh_disable().

And local_bh_disable() will have increased the preempt-count exactly
so that it looks like it's in_serving_softirq().

Maybe I got lost somewhere, but it basically looks like that test will
always be true and is pointless (and misleading).

         Linus