Re: [PATCH net-next v1] neighbour: Replace kvzalloc() with kzalloc() when GFP_ATOMIC is specified

Kohei Enju
2025-02-17T16:52:49Z

+ SLAB ALLOCATOR maintainers and reviewers

> > From: Kohei Enju <enjuk@amazon.com>
> > Date: Mon, 17 Feb 2025 01:30:16 +0900
> > > Replace kvzalloc()/kvfree() with kzalloc()/kfree() when GFP_ATOMIC is
> > > specified, since kvzalloc() doesn't support non-sleeping allocations such
> > > as GFP_ATOMIC.
> > >
> > > With incompatible gfp flags, kvzalloc() never falls back to the vmalloc
> > > path and returns immediately after the kmalloc path fails.
> > > Therefore, using kzalloc() is sufficient in this case.
> > >
> > > Fixes: 41b3caa7c076 ("neighbour: Add hlist_node to struct neighbour")
> >
> > This commit followed the old hash_buckets allocation, so I'd add
> >
> > Fixes: ab101c553bc1 ("neighbour: use kvzalloc()/kvfree()")
> >
> > too.
> >
> > Both commits were introduced in v6.13, so there's no difference in terms
> > of backporting though.
> >
> > Also, it would be nice to CC mm maintainers in case they have some
> > comments.
>
> Oh well, we need to trigger neigh_hash_grow() from a process context,
> or convert net/core/neighbour.c to modern rhashtable.

Hi all, thanks for your comments.

kzalloc() uses page allocator when size is larger than
KMALLOC_MAX_CACHE_SIZE, so I think what commit ab101c553bc1 ("neighbour:
use kvzalloc()/kvfree()") intended could be achieved by using kzalloc().

As mentioned, when using GFP_ATOMIC, kvzalloc() only tries the kmalloc
path, since the vmalloc path doesn't support the flag.
In this case, kvzalloc() is equivalent to kzalloc() in that neither try
the vmalloc path, so there is no functional change between this patch and
either commit ab101c553bc1 ("neighbour: use kvzalloc()/kvfree()") or
commit 41b3caa7c076 ("neighbour: Add hlist_node to struct neighbour").

Actually there's no real bug in the current code so the Fixes tag was not
appropriate. I shall remove the tag.

Regards,
Kohei