From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2D67EC8302F for ; Tue, 1 Jul 2025 11:19:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C52676B00A3; Tue, 1 Jul 2025 07:19:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C028B6B00A4; Tue, 1 Jul 2025 07:19:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AF1906B00A6; Tue, 1 Jul 2025 07:19:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 9A0BF6B00A3 for ; Tue, 1 Jul 2025 07:19:20 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 02D771064F0 for ; Tue, 1 Jul 2025 11:19:19 +0000 (UTC) X-FDA: 83615449680.02.F2F97AB Received: from mailrelay-egress12.pub.mailoutpod2-cph3.one.com (mailrelay-egress12.pub.mailoutpod2-cph3.one.com [46.30.211.187]) by imf20.hostedemail.com (Postfix) with ESMTP id F3B951C0012 for ; Tue, 1 Jul 2025 11:19:17 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=konsulko.se header.s=rsa1 header.b=DnPyqWNv; dkim=pass header.d=konsulko.se header.s=ed1 header.b=IMDbMUQ7 ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1751368758; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=DHXuxV40u1ZYjDXLBVQbXs11RHOmrGLB2bnqKxDAyss=; b=IRuKPLEbqwZ7PC72wv8hJTDMOJGtQU/6DzcyG+puLmVWJeQUREa9JpSOkO3sq3uhccsnqW H9KHD6ffn3W1wLL5SmVA/PBGOYq+f6rO8BNhGUfaDfKfZMnplD1CiNqNCFkB/+b/W7aiPF wSzK4faK6chSxRpx+YcLaApScv4kK7I= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=konsulko.se header.s=rsa1 header.b=DnPyqWNv; dkim=pass header.d=konsulko.se header.s=ed1 header.b=IMDbMUQ7; spf=none (imf20.hostedemail.com: domain of vitaly.wool@konsulko.se has no SPF policy when checking 46.30.211.187) smtp.mailfrom=vitaly.wool@konsulko.se; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1751368758; a=rsa-sha256; cv=none; b=uhSOLmG6Zk0H2qUdrB6PfwYARahrNeeomQnvwz9fBj6mgHLpQHTDm2s7YqQS7cTZYot9un m9fuLWK9Tz91hD/cuhDcTk3XQTy6CVBuSgIwKRl+OY/k8WyzO/XDTu3OwaXM6sSq2EnLiM i0K4YWu6P06y5+z2hJHUswFhVHThexU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1751368756; x=1751973556; d=konsulko.se; s=rsa1; h=to:references:message-id:content-transfer-encoding:cc:date:in-reply-to:from: subject:mime-version:content-type:from; bh=DHXuxV40u1ZYjDXLBVQbXs11RHOmrGLB2bnqKxDAyss=; b=DnPyqWNviCmpJZ2Hfth/ae8KzTc8bC9btMPr1I8VVD/4t0UE+fWAzDmTpnpribFRkeG6dMqpB7JWX dwyZf3PS8aWEGKU2C5sIVsSLGX8+xpPAfnrf1Sf//Ca8AGbqgJXJ0PLMRD7etZhKTr5tGWgdom9219 0AbgSZHChHYuFqn04J1SX4IpA6r4hfpr3Mq9KFCmlroV5wJhhEaBattoEIQQ/swKgW2vbKrCH8YNgh T/SIW7bykAwD16e+KP6UADGr28leGRSjPPx4iWEcC9G1FXuGn1uQ0CVR5P502IT3jlJUUqRZ+z78al Pvpb0LtmPdW1WzkiOQpTFsMzeNcs81Q== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1751368756; x=1751973556; d=konsulko.se; s=ed1; h=to:references:message-id:content-transfer-encoding:cc:date:in-reply-to:from: subject:mime-version:content-type:from; bh=DHXuxV40u1ZYjDXLBVQbXs11RHOmrGLB2bnqKxDAyss=; b=IMDbMUQ7RkW5LhtaMJhpYbNC1TMzZWvyW51Z3IMABcj6SDyJ7yUS5UO2ltw2SrY9SaFjYYBm8VAwY Frwzcq7DA== X-HalOne-ID: 393bfb6b-566d-11f0-ab17-85eb291bc831 Received: from smtpclient.apple (c188-150-224-8.bredband.tele2.se [188.150.224.8]) by mailrelay5.pub.mailoutpod2-cph3.one.com (Halon) with ESMTPSA id 393bfb6b-566d-11f0-ab17-85eb291bc831; Tue, 01 Jul 2025 11:19:16 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.200.121\)) Subject: Re: [PATCH v9 3/4] rust: add support for NUMA ids in allocations From: Vitaly Wool In-Reply-To: Date: Tue, 1 Jul 2025 13:19:06 +0200 Cc: linux-mm@kvack.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, Uladzislau Rezki , Danilo Krummrich , rust-for-linux@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <3D58CB40-CB1C-47A2-BB71-5C32B3609AE0@konsulko.se> References: <20250630221511.3325123-1-vitaly.wool@konsulko.se> <20250630221640.3325306-1-vitaly.wool@konsulko.se> To: Alice Ryhl X-Mailer: Apple Mail (2.3826.200.121) X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: F3B951C0012 X-Stat-Signature: fp5gbqtc3p9jyfdeww1pnx7kxrp87a1s X-Rspam-User: X-HE-Tag: 1751368757-222371 X-HE-Meta: U2FsdGVkX18aSl+5H48xagmDf/d2FqbvIF+xYHaRzxVOcRbSsXQSMGDotl/yaRXmRj1Eh+GDzytb2OWmW/X9hrYWiFYcttlcVThPCVnrah5RsqPVFmtIik/YJk7b4svoMYX3WNuDiBlXec0BRfczz1SW5ZARNCSaes0nxP27WXXYYLXWyzIvOQ5RApSP/uE/TTwQ5zU/PtAba00h0qIX82mAKbtfDIfpngB3Ev9dA8UpEfi113ilzbB7ktTLHM6QSairgHS3RmA0GRXJ93vd5RyHnlOEWw95MCNrd1lnqj53aJX7j2CdXB1Lw32L79YTGHGYDvAVw/JLIrHrh5rq3PS0Wl2wCfxaBIUO9O/JwztzPDWHADXBk5vZkHQvdxRVsGI/rtvdtEaO2012kLqeH8kvkOj2WjoEcZASvVh1c2rbo/rTd+tqnXLZ6dHqzvmVaUvyR6qXFcfbgOUXPWF1tYUNH5zppRvFr2dzQxUa45ZOeewe1qfogIyIDuFkt4mLQ5LjDDOc/LlV1hAbPX22T0ePLR1OkvgpOIglspdIbO8EQFs1+Mu4gNCkO8/c6wV/0kCjg6+KkoGiEjkz+XmCeCf9xAztT77m2bWKRLCBpm6xXd6tErh8nb2FGqqPZINwQwLqy1+IPHxSLWB4uLZqffBwS4qW6FxeJtHDzlCdxm+rM4ZqEJTgkSOh3JLwp/bJmIS5evhKiF8IHsS5GhBVB7cXRCdyWbl99AUbL+clWWuS6snu9f8hP6Hpjjngm/1TM0scZyI5EbLk4DCCxn0zfvQ2VYXDHIp+ReoDFEfIvTKPWm98MFNdMyW/nUme9si3nioLkGlZItuhTefePL8hu2wSGFJejPUOWjnqVvHPfsS3SGaIxQplhQlwe+PF3bz+L3WqfYs1JtlFPbfsI6jTKBkSWraHbgeJJSqw4bDj8rYZvORmmdssEe2bsFQW3td3+hDpDyDIMu3zk/LbOjR eUPurHPY FOfY2qR9hGwMMpK7A17ETJxIl2A71RqN7GGiy1D6ZI/GsGlDgBqgkIrybi9EgfoMH4y22w1ra80wpoyKDSWnbkh9xeVpcwtaYIbxBEgHjew9HSkTdnJIxV7o2XMLaOsDDjqgWe8B+uhcrOoG4UgPGsbnB9SXJo7tej59Sr2iQvCV2Ils07uirgUhBj8NpHzwcWlnvXHTqDFrmV94KEoNoLC4ooHXzgG6aLlqnt4ZixRQibqxw93ZeEuWhSU6Q0L4Q6B2NIskqCsQw8NE= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: > On Jul 1, 2025, at 12:34=E2=80=AFPM, Alice Ryhl = wrote: >=20 > On Tue, Jul 01, 2025 at 12:16:40AM +0200, Vitaly Wool wrote: >> Add a new type to support specifying NUMA identifiers in Rust >> allocators and extend the allocators to have NUMA id as a >> parameter. Thus, modify ReallocFunc to use the new extended realloc >> primitives from the C side of the kernel (i. e. >> k[v]realloc_node_align/vrealloc_node_align) and add the new function >> alloc_node to the Allocator trait while keeping the existing one >> (alloc) for backward compatibility. >>=20 >> This will allow to specify node to use for allocation of e. g. >> {KV}Box, as well as for future NUMA aware users of the API. >>=20 >> Signed-off-by: Vitaly Wool >=20 > My main feedback is that we should consider introducing a new trait > instead of modifying Allocator. What we could do is have a = NodeAllocator > trait that is a super-trait of Allocator and has additional methods = with > a node parameter. >=20 > A sketch: >=20 > pub unsafe trait NodeAllocator: Allocator { > fn alloc_node(layout: Layout, flags: Flags, nid: NumaNode) > -> Result, AllocError>; >=20 > unsafe fn realloc_node( > ptr: Option>, > layout: Layout, > old_layout: Layout, > flags: Flags, > nid: NumaNode, > ) -> Result, AllocError>; > } >=20 > By doing this, it's possible to have allocators that do not support > specifying the numa node which only implement Allocator, and to have > other allocators that implement both Allocator and NumaAllocator where > you are able to specify the node. >=20 > If all allocators in the kernel support numa nodes, then you can = ignore > this. This is an elegant solution indeed but I think that keeping the existing = approach goes better with the overall kernel trend of having better NUMA = support. My point is, if we add NodeAllocator as a super-trait and in a = foreseeable future all the Rust allocators will want/be required to = support NUMA (which is likely to happen), we=E2=80=99ll have to = =E2=80=9Cflatten=E2=80=9D the traits and effectively go back to the = approach expressed in this patch. >> +/// Non Uniform Memory Access (NUMA) node identifier >> +#[derive(Clone, Copy, PartialEq)] >> +pub struct NumaNode(i32); >> + >> +impl NumaNode { >> + /// create a new NUMA node identifer (non-negative integer) >> + /// returns EINVAL if a negative id or an id exceeding = MAX_NUMNODES is specified >> + pub fn new(node: i32) -> Result { >> + // SAFETY: MAX_NUMNODES never exceeds 2**10 because = NODES_SHIFT is 0..10 >> + if node < 0 || node >=3D bindings::MAX_NUMNODES as i32 { >> + return Err(EINVAL); >> + } >> + Ok(Self(node)) >> + } >> +} >> + >> +/// Specify necessary constant to pass the information to Allocator = that the caller doesn't care >> +/// about the NUMA node to allocate memory from. >> +pub mod numa { >> + use super::NumaNode; >> + >> + /// No preference for NUMA node >> + pub const NUMA_NO_NODE: NumaNode =3D = NumaNode(bindings::NUMA_NO_NODE); >> +} >=20 > Instead of using a module, you can make it an associated constant of = the > struct. >=20 > impl NumaNode { > pub const NO_NODE: NumaNode =3D NumaNode(bindings::NUMA_NO_NODE); > } >=20 > This way you can access the constant as NumaNode::NO_NODE. Thanks, noted. >=20 >> /// The kernel's [`Allocator`] trait. >> /// >> /// An implementation of [`Allocator`] can allocate, re-allocate and = free memory buffers described >> @@ -148,7 +175,7 @@ pub unsafe trait Allocator { >> /// >> /// 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`], >> + /// [`Allocator::free`], [`Allocator::realloc`] or = [`Allocator::realloc_node`], >> /// - aligned to `layout.align()`, >> /// >> /// Additionally, `Flags` are honored as documented in >> @@ -159,7 +186,38 @@ fn alloc(layout: Layout, flags: Flags) -> = Result, AllocError> { >> unsafe { Self::realloc(None, layout, Layout::new::<()>(), = flags) } >> } >>=20 >> - /// Re-allocate an existing memory allocation to satisfy the = requested `layout`. >> + /// Allocate memory based on `layout`, `flags` and `nid`. >> + /// >> + /// 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_node` 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`], [`Allocator::realloc`] or = [`Allocator::realloc_node`], >> + /// - aligned to `layout.align()`, >> + /// >> + /// Additionally, `Flags` are honored as documented in >> + /// = . >> + fn alloc_node(layout: Layout, flags: Flags, nid: NumaNode) >> + -> Result, AllocError> { >=20 > I don't think this is how rustfmt would format this. Can you run = rustfmt > on your patch? >=20 >=20 Will do, thanks. ~Vitaly