linux-kernel - Re: [PATCH v5 3/4] rust: add support for NUMA ids 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: <aF5-a-bUp1pD5tiS@pollux>
Date: Fri, 27 Jun 2025 13:20:11 +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 v5 3/4] rust: add support for NUMA ids in allocations

On Fri, Jun 27, 2025 at 11:38:58AM +0200, Vitaly Wool wrote:
> +    /// Allocate memory based on `layout`, `flags` and `nid`.

The semantical meanting of layout and flags is rather obvious due to its type.

We should probably introduce a new type NumaNode with a constant for
NUMA_NODE_NONE.

> +    ///
> +    /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
> +    /// constraints (i.e. minimum size and alignment as specified by `layout`).
> +    ///
> +    /// This function is equivalent to `realloc` when called with `None`.
> +    ///
> +    /// # Guarantees
> +    ///
> +    /// When the return value is `Ok(ptr)`, then `ptr` is
> +    /// - valid for reads and writes for `layout.size()` bytes, until it is passed to
> +    ///   [`Allocator::free`] or [`Allocator::realloc`],
> +    /// - aligned to `layout.align()`,
> +    ///
> +    /// Additionally, `Flags` are honored as documented in
> +    /// <https://docs.kernel.org/core-api/mm-api.html#mm-api-gfp-flags>.
> +    fn alloc_node(layout: Layout, flags: Flags, nid: Option<i32>)
> +                -> Result<NonNull<[u8]>, AllocError> {
> +        // SAFETY: Passing `None` to `realloc` is valid by its safety requirements and asks for a
> +        // new memory allocation.
> +        unsafe { Self::realloc(None, layout, Layout::new::<()>(), flags, nid) }
>      }
>  
>      /// Re-allocate an existing memory allocation to satisfy the requested `layout`.
> @@ -196,6 +219,7 @@ unsafe fn realloc(
>          layout: Layout,
>          old_layout: Layout,
>          flags: Flags,
> +        nid: Option<i32>,
>      ) -> Result<NonNull<[u8]>, AllocError>;

Please rename to realloc_node() and expand the documentation explaining what
happens for invalid nid numbers and what happens if nid changes throughout
invocations.

Also, please introduce realloc() which calls realloc_node() with NUMA_NODE_NONE.

>  
>      /// Free an existing memory allocation.
> @@ -211,7 +235,7 @@ unsafe fn free(ptr: NonNull<u8>, layout: Layout) {
>          // SAFETY: The caller guarantees that `ptr` points at a valid allocation created by this
>          // allocator. We are passing a `Layout` with the smallest possible alignment, so it is
>          // smaller than or equal to the alignment previously used with this allocation.
> -        let _ = unsafe { Self::realloc(Some(ptr), Layout::new::<()>(), layout, Flags(0)) };
> +        let _ = unsafe { Self::realloc(Some(ptr), Layout::new::<()>(), layout, Flags(0), None) };
>      }
>  }
>  
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index aa2dfa9dca4c..4f0fe2b67593 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -58,18 +58,20 @@ fn aligned_size(new_layout: Layout) -> usize {
>  ///
>  /// One of the following: `krealloc`, `vrealloc`, `kvrealloc`.
>  struct ReallocFunc(
> -    unsafe extern "C" fn(*const crate::ffi::c_void, usize, u32) -> *mut crate::ffi::c_void,
> +    unsafe extern "C" fn(*const crate::ffi::c_void, usize, crate::ffi::c_ulong, u32,
> +                         crate::ffi::c_int
> +    ) -> *mut crate::ffi::c_void,
>  );
>  
>  impl ReallocFunc {
> -    // INVARIANT: `krealloc` satisfies the type invariants.
> -    const KREALLOC: Self = Self(bindings::krealloc);
> +    // INVARIANT: `krealloc_node` satisfies the type invariants.
> +    const KREALLOC_NODE: Self = Self(bindings::krealloc_node);
>  
> -    // INVARIANT: `vrealloc` satisfies the type invariants.
> -    const VREALLOC: Self = Self(bindings::vrealloc);
> +    // INVARIANT: `vrealloc_node` satisfies the type invariants.
> +    const VREALLOC_NODE: Self = Self(bindings::vrealloc_node);
>  
> -    // INVARIANT: `kvrealloc` satisfies the type invariants.
> -    const KVREALLOC: Self = Self(bindings::kvrealloc);
> +    // INVARIANT: `kvrealloc_node` satisfies the type invariants.
> +    const KVREALLOC_NODE: Self = Self(bindings::kvrealloc_node);

I think those constants can remain to be named just KREALLOC, etc.