linux-kernel - Re: [PATCH 2/3] rust: xarray: abstract `xa_alloc

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: <20251007152724.2b33a899@nimda.home>
Date: Tue, 7 Oct 2025 15:31:45 +0300
From: Onur Özkan <work@...rozkan.dev>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: rust-for-linux@...r.kernel.org, ojeda@...nel.org, alex.gaynor@...il.com,
 boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
 lossin@...nel.org, tmgross@...ch.edu, dakr@...nel.org,
 linux-kernel@...r.kernel.org, acourbot@...dia.com, airlied@...il.com,
 simona@...ll.ch, maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
 tzimmermann@...e.de, corbet@....net, lyude@...hat.com,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH 2/3] rust: xarray: abstract `xa_alloc_cyclic`

On Tue, 7 Oct 2025 10:56:56 +0000
Alice Ryhl <aliceryhl@...gle.com> wrote:

> On Mon, Oct 06, 2025 at 07:30:23PM +0300, Onur Özkan wrote:
> > Implements `alloc_cyclic` function to `XArray<T>` that
> > wraps `xa_alloc_cyclic` safely.
> > 
> > Resolves a task from the nova/core task list under the "XArray
> > bindings [XARR]" section in "Documentation/gpu/nova/core/todo.rst"
> > file.
> > 
> > Signed-off-by: Onur Özkan <work@...rozkan.dev>
> > ---
> >  rust/kernel/xarray.rs | 43
> > +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43
> > insertions(+)
> > 
> > diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> > index 1b882cd2f58b..4c2fdf53c7af 100644
> > --- a/rust/kernel/xarray.rs
> > +++ b/rust/kernel/xarray.rs
> > @@ -305,6 +305,49 @@ pub fn alloc(
> > 
> >          Ok(id)
> >      }
> > +
> > +    /// Allocates an empty slot within the given `limit`, storing
> > `value` and cycling from `*next`.
> > +    ///
> > +    /// May drop the lock if needed to allocate memory, and then
> > reacquire it afterwards.
> > +    ///
> > +    /// On success, returns the allocated id.
> > +    ///
> > +    /// On failure, returns the element which was attempted to be
> > stored.
> > +    pub fn alloc_cyclic(
> > +        &mut self,
> > +        limit: bindings::xa_limit,
> 
> Could we use a Range<u32> type or similar here? I don't think we want
> a bindings type.
> 

Why do we not like to use the bindings type directly?


> > +        next: &mut u32,
> 
> So this is a mutable reference because it writes `*id + 1` to next,
> taking wrap-around into account? The docs should probably explain
> that.
> 

Sure. To be honest, I didn't really like doing this in the first place.
I can drop the mutable reference and return the next value as part of
the result to make it more idiomatic. This way, it will be easier for
the caller to use, especially for those who don't care about the next
value.

> > +        value: T,
> > +        gfp: alloc::Flags,
> > +    ) -> Result<u32, StoreError<T>> {
> > +        build_assert!(
> > +            T::FOREIGN_ALIGN >= 4,
> > +            "pointers stored in XArray must be 4-byte aligned"
> > +        );
> 
> It should be enough to have this in the constructor. I don't think
> it's needed here.
> 
> Alice

Regards,
Onur