Re: [PATCH 2/4] mm/slub: allow to set node and align in k[v]realloc

[Danilo Krummrich]

On Fri, Jun 27, 2025 at 11:37:14AM +0200, Vitaly Wool wrote:
> Reimplement k[v]realloc_node() to be able to set node and
> alignment should a user need to do so. In order to do that while
> retaining the maximal backward compatibility, the following rules
> are honored:
> * kmalloc/kzalloc/krealloc remain unchanged
> * kvmalloc/kvrealloc/kvcalloc remain unchanged
> * kvrealloc remains unchanged
> * kvrealloc_node is implemented as a new function taking align and
>   NUMA id as extra parameters compared to kvrealloc.
> * krealloc_node is implemented as a new function taking NUMA id
>   as an extra parameter compared to krealloc
> * kvmalloc_node/kvzalloc_node/kvcalloc_node get an extra parameter
>   (alignment)

I see what you're doing here:

You created vrealloc_node_noprof() in the previous patch, taking the following
arguments:

	vrealloc_node_noprof(const void *p, size_t size,
			     unsigned long align,
			     gfp_t flags, int nid)

And now you're aligning the newly introduced krealloc_node() and
kvrealloc_node() with that.

The idea for having an align argument on krealloc_node() simply is that it
fails if the alignment requirement can't be fulfilled by the corresponding
kmalloc bucket, such that we can fall back to vrealloc_node() in
kvrealloc_node().

Generally, this makes sense to me.

However, now you consequently have to add the align argument to kvmalloc_node(),
kvzalloc_node(), kvcalloc_node() as well.

This is what creates this huge diffstat changing all the users.

IMHO, the problem here was introduced already with vrealloc_node_noprof() taking
an align argument in your previous patch, since now you have to adjust
everything else to logically follow the same naming scheme.

Instead, I think you should introduce vrealloc_node_align(),
kvrealloc_node_align(), etc. This way no existing function signatures, such as
kvmalloc_node() have to be changed and no users have to be adjusted.

- Danilo