Re: [PATCH v8 3/4] rust: add support for NUMA ids in allocations

[Danilo Krummrich <dakr@kernel.org>]

On Sat, Jun 28, 2025 at 05:25:52PM +0200, Vitaly Wool wrote:
> 
> 
> > On Jun 28, 2025, at 2:21 PM, Danilo Krummrich <dakr@kernel.org> wrote:
> > 
> > On Sat, Jun 28, 2025 at 12:26:11PM +0200, Vitaly Wool wrote:
> >> +/// 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 is specified
> >> +    pub fn new(node: i32) -> Result<Self> {
> >> +        if node < 0 {
> >> +            return Err(EINVAL);
> >> +        }
> > 
> > Should we also check for MAX_NUMNODES?
> 
> Good point, thanks.
> 
> > 
> >> +        Ok(Self(node))
> >> +    }
> >> +}
> > 
> > <snip>
> > 
> >> +    /// Re-allocate an existing memory allocation to satisfy the requested `layout` and
> >> +    /// optionally a specific NUMA node request to allocate the memory for.
> > 
> > It's not an Option anymore, so we may want to drop 'optionally'. Also please
> > leave an empty line here.
> > 
> >> +    /// Systems employing a Non Uniform Memory Access (NUMA) architecture contain
> >> +    /// collections of hardware resources including processors, memory, and I/O buses,
> >> +    /// that comprise what is commonly known as a NUMA node.
> >> +    /// `nid` stands for NUMA id, i. e. NUMA node identifier, which is a non-negative
> >> +    /// integer if a node needs to be specified, or NUMA_NO_NODE if the caller doesn't care.
> > 
> > Please also explain what happens when the NumaNode changes between calls to
> > realloc_node().
> > 
> > Does it have to remain the same NumaNode? Do we need a safety requirement for
> > that?
> 
> Since we don’t implement that logic, we trust the C part. The current implementation will refuse to realloc for a different node, and I believe that is the right thing to do because transferring an allocation to a different node doesn’t go well with the concept of simple adjustment of the allocation size.
> 
> Do you believe it is necessary to explicitly state it here in the comments?

Yes, we should document what can be expected to happen in this case, i.e. that
it will cause an AllocError.