linux-kernel - Re: [PATCH v3 2/2] rust: support align and NUMA id in allocations

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: <C349F54D-348D-44FE-A02F-E75C78608734@konsulko.se>
Date: Wed, 25 Jun 2025 22:22:36 +0200
From: Vitaly Wool <vitaly.wool@...sulko.se>
To: Danilo Krummrich <dakr@...nel.org>
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 v3 2/2] rust: support align and NUMA id in allocations



> On Jun 25, 2025, at 9:07 PM, Danilo Krummrich <dakr@...nel.org> wrote:
> 
> On Wed, Jun 25, 2025 at 08:56:05PM +0200, Danilo Krummrich wrote:
>> On Wed, Jun 25, 2025 at 08:30:26AM +0200, Vitaly Wool wrote:
>>> Add support for large (> PAGE_SIZE) alignments in Rust allocators
>>> (Kmalloc support for large alignments is limited to the requested
>>> size, which is a reasonable limitation anyway).
>> 
>> Please split this..
>> 
>>> Besides, add support for NUMA id to Vmalloc.
>> 
>> and this into separate patches.
>> 
>> Please also add some information to the commit message what you need node
>> support for. Do you also have patches to add node support to Box and Vec?

No, but there is a zswap backend implementation written in Rust and it should be  NUMA id aware.
I’m planning on submitting that basically as soon as this piece gets accepted.

>> 
>>> 
>>> Signed-off-by: Vitaly Wool <vitaly.wool@...sulko.se>
>>> ---
>>> rust/helpers/slab.c            |  8 +++++--
>>> rust/helpers/vmalloc.c         |  4 ++--
>>> rust/kernel/alloc.rs           | 28 ++++++++++++++++++++++--
>>> rust/kernel/alloc/allocator.rs | 40 +++++++++++++++++++---------------
>>> rust/kernel/alloc/kvec.rs      |  3 ++-
>>> 5 files changed, 59 insertions(+), 24 deletions(-)
>>> 
>>> diff --git a/rust/helpers/slab.c b/rust/helpers/slab.c
>>> index a842bfbddcba..221c517f57a1 100644
>>> --- a/rust/helpers/slab.c
>>> +++ b/rust/helpers/slab.c
>>> @@ -3,13 +3,17 @@
>>> #include <linux/slab.h>
>>> 
>>> void * __must_check __realloc_size(2)
>>> -rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
>>> +rust_helper_krealloc(const void *objp, size_t new_size, unsigned long align, gfp_t flags, int nid)
>> 
>> This should have a comment making it obvious why the function has two arguments
>> that are discarded. I think we should even separate it with an additional inline
>> function.
>> 
>> I do agree with discarding the align argument, given that it's not exposed to
>> users though the Allocator API.
> 
> What I meant is that proper alignment is implied when krealloc() succeeds.

I agree, I need to add some comments explaining this.

> 
>> I do disagree with discarding the nid argument though, since you change the
>> generic Allocator::realloc() API to take a node argument, which for KREALLOC and
>> KVREALLOC is silently discarded. If we introduce it, we should do so for all
>> three allocators.
>> 
>>> {
>>> + if (WARN_ON(new_size & (align - 1)))
>>> + return NULL;
>> 
>> I don't think we should have this WARN_ON(). If we want to warn about this, we
>> should already do so on the Rust side. The helper functions in this file should
>> not contain any logic.

Agreed.

>> 
>>> return krealloc(objp, new_size, flags);
>>> }
>>> 
>>> void * __must_check __realloc_size(2)
>>> -rust_helper_kvrealloc(const void *p, size_t size, gfp_t flags)
>>> +rust_helper_kvrealloc(const void *p, size_t size, unsigned long align, gfp_t flags, int nid)
>>> {
>>> + if (WARN_ON(size & (align - 1)))
>>> + return NULL;
>>> return kvrealloc(p, size, flags);
>>> }
>> 
>> Same as above.
> 
> This is actually different though, here kvrealloc() may succeed even if the
> requested alignment is not fulfilled, so this is incorrect.

I can move this logic to the Rust part, too. My point here is, for Kvrealloc with a large alignment we’ll just make the decision to use vmalloc, period. We can indeed do that on the Rust side.

~Vitaly