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@xxxxxxxxxxxx>
> 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