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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aF51PTZh0gRVFuYu@pollux>
Date: Fri, 27 Jun 2025 12:41:01 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Vitaly Wool <vitaly.wool@...sulko.se>
Cc: linux-mm@...ck.org, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, Uladzislau Rezki <urezki@...il.com>,
	Alice Ryhl <aliceryhl@...gle.com>, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 2/4] mm/slub: allow to set node and align in k[v]realloc

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