[PATCH 0/3] rust: xarray: abstract `xa_alloc` and `xa_alloc_cyclic`

[PATCH 0/3] rust: xarray: abstract `xa_alloc` and `xa_alloc_cyclic`

[Onur Özkan]

Initial version of xa_alloc and xa_alloc_cyclic abstraction
patch series.

Onur Özkan (3):
  rust: xarray: abstract `xa_alloc`
  rust: xarray: abstract `xa_alloc_cyclic`
  remove completed task from nova-core task list

 Documentation/gpu/nova/core/todo.rst |  8 ---
 rust/kernel/xarray.rs                | 82 ++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 8 deletions(-)

--
2.51.0

[PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Onur Özkan]

Implements `alloc` function to `XArray<T>` that wraps
`xa_alloc` 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@onurozkan.dev>
---
 rust/kernel/xarray.rs | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
index a49d6db28845..1b882cd2f58b 100644
--- a/rust/kernel/xarray.rs
+++ b/rust/kernel/xarray.rs
@@ -266,6 +266,45 @@ pub fn store(
             Ok(unsafe { T::try_from_foreign(old) })
         }
     }
+
+    /// Allocates an empty slot within the given limit range and stores `value` there.
+    ///
+    /// 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(
+        &mut self,
+        limit: bindings::xa_limit,
+        value: T,
+        gfp: alloc::Flags,
+    ) -> Result<u32, StoreError<T>> {
+        build_assert!(
+            T::FOREIGN_ALIGN >= 4,
+            "pointers stored in XArray must be 4-byte aligned"
+        );
+
+        let new = value.into_foreign();
+        let mut id: u32 = 0;
+
+        // SAFETY:
+        // - `self.xa.xa` is valid by the type invariant.
+        // - `new` came from `T::into_foreign`.
+        let ret =
+            unsafe { bindings::__xa_alloc(self.xa.xa.get(), &mut id, new, limit, gfp.as_raw()) };
+
+        if ret < 0 {
+            // SAFETY: `__xa_alloc` doesn't take ownership on error.
+            let value = unsafe { T::from_foreign(new) };
+            return Err(StoreError {
+                value,
+                error: Error::from_errno(ret),
+            });
+        }
+
+        Ok(id)
+    }
 }

 // SAFETY: `XArray<T>` has no shared mutable state so it is `Send` iff `T` is `Send`.
--
2.51.0

[PATCH 2/3] rust: xarray: abstract `xa_alloc_cyclic`

[Onur Özkan]

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@onurozkan.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,
+        next: &mut u32,
+        value: T,
+        gfp: alloc::Flags,
+    ) -> Result<u32, StoreError<T>> {
+        build_assert!(
+            T::FOREIGN_ALIGN >= 4,
+            "pointers stored in XArray must be 4-byte aligned"
+        );
+
+        let new = value.into_foreign();
+
+        // `__xa_alloc_cyclic` overwrites this.
+        let mut id: u32 = 0;
+
+        // SAFETY:
+        // - `self.xa.xa` is valid by the type invariant.
+        // - `new` came from `T::into_foreign`.
+        let ret = unsafe {
+            bindings::__xa_alloc_cyclic(self.xa.xa.get(), &mut id, new, limit, next, gfp.as_raw())
+        };
+
+        if ret < 0 {
+            // SAFETY: `__xa_alloc_cyclic` doesn't take ownership on error.
+            let value = unsafe { T::from_foreign(new) };
+            return Err(StoreError {
+                value,
+                error: Error::from_errno(ret),
+            });
+        }
+
+        Ok(id)
+    }
 }

 // SAFETY: `XArray<T>` has no shared mutable state so it is `Send` iff `T` is `Send`.
--
2.51.0

[PATCH 3/3] remove completed task from nova-core task list

[Onur Özkan]

The task is completed in this series.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 Documentation/gpu/nova/core/todo.rst | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst
index 0972cb905f7a..de70a857fcfd 100644
--- a/Documentation/gpu/nova/core/todo.rst
+++ b/Documentation/gpu/nova/core/todo.rst
@@ -210,14 +210,6 @@ capability, MSI API abstractions.

 | Complexity: Beginner

-XArray bindings [XARR]
-----------------------
-
-We need bindings for `xa_alloc`/`xa_alloc_cyclic` in order to generate the
-auxiliary device IDs.
-
-| Complexity: Intermediate
-
 Debugfs abstractions
 --------------------

--
2.51.0

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Benno Lossin]

On Mon Oct 6, 2025 at 6:30 PM CEST, Onur Özkan wrote:
> Implements `alloc` function to `XArray<T>` that wraps
> `xa_alloc` 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@onurozkan.dev>
> ---
>  rust/kernel/xarray.rs | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> index a49d6db28845..1b882cd2f58b 100644
> --- a/rust/kernel/xarray.rs
> +++ b/rust/kernel/xarray.rs
> @@ -266,6 +266,45 @@ pub fn store(
>              Ok(unsafe { T::try_from_foreign(old) })
>          }
>      }
> +
> +    /// Allocates an empty slot within the given limit range and stores `value` there.
> +    ///
> +    /// 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(
> +        &mut self,
> +        limit: bindings::xa_limit,
> +        value: T,
> +        gfp: alloc::Flags,
> +    ) -> Result<u32, StoreError<T>> {

I think it would be a good idea to make the id a newtype wrapper around
u32. Maybe not even allow users to manually construct it or even inspect
it if possible.

---
Cheers,
Benno

> +        build_assert!(
> +            T::FOREIGN_ALIGN >= 4,
> +            "pointers stored in XArray must be 4-byte aligned"
> +        );

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Boqun Feng]

HI Onur,

On Mon, Oct 06, 2025 at 07:30:22PM +0300, Onur Özkan wrote:
> Implements `alloc` function to `XArray<T>` that wraps
> `xa_alloc` safely.
> 
> Resolves a task from the nova/core task list under the "XArray
> bindings [XARR]" section in "Documentation/gpu/nova/core/todo.rst"
> file.
> 

Having this information is good, however I feel it's better if you
explain/expand what exact the usage will be on the XArray, otherwise,
it'll be hard for people to dig in the history and find out why we add
this. Thanks!

Regards,
Boqun

> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
>  rust/kernel/xarray.rs | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> index a49d6db28845..1b882cd2f58b 100644
> --- a/rust/kernel/xarray.rs
> +++ b/rust/kernel/xarray.rs
> @@ -266,6 +266,45 @@ pub fn store(
>              Ok(unsafe { T::try_from_foreign(old) })
>          }
>      }
> +
> +    /// Allocates an empty slot within the given limit range and stores `value` there.
> +    ///
> +    /// 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(
> +        &mut self,
> +        limit: bindings::xa_limit,
> +        value: T,
> +        gfp: alloc::Flags,
> +    ) -> Result<u32, StoreError<T>> {
> +        build_assert!(
> +            T::FOREIGN_ALIGN >= 4,
> +            "pointers stored in XArray must be 4-byte aligned"
> +        );
> +
> +        let new = value.into_foreign();
> +        let mut id: u32 = 0;
> +
> +        // SAFETY:
> +        // - `self.xa.xa` is valid by the type invariant.
> +        // - `new` came from `T::into_foreign`.
> +        let ret =
> +            unsafe { bindings::__xa_alloc(self.xa.xa.get(), &mut id, new, limit, gfp.as_raw()) };
> +
> +        if ret < 0 {
> +            // SAFETY: `__xa_alloc` doesn't take ownership on error.
> +            let value = unsafe { T::from_foreign(new) };
> +            return Err(StoreError {
> +                value,
> +                error: Error::from_errno(ret),
> +            });
> +        }
> +
> +        Ok(id)
> +    }
>  }
> 
>  // SAFETY: `XArray<T>` has no shared mutable state so it is `Send` iff `T` is `Send`.
> --
> 2.51.0
> 

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Onur Özkan]

On Mon, 6 Oct 2025 16:09:42 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> HI Onur,
> 
> On Mon, Oct 06, 2025 at 07:30:22PM +0300, Onur Özkan wrote:
> > Implements `alloc` function to `XArray<T>` that wraps
> > `xa_alloc` safely.
> > 
> > Resolves a task from the nova/core task list under the "XArray
> > bindings [XARR]" section in "Documentation/gpu/nova/core/todo.rst"
> > file.
> > 
> 
> Having this information is good, however I feel it's better if you
> explain/expand what exact the usage will be on the XArray, otherwise,
> it'll be hard for people to dig in the history and find out why we add
> this. Thanks!
> 

Very true, thanks.

-Onur

> Regards,
> Boqun
> 
> > Signed-off-by: Onur Özkan <work@onurozkan.dev>
> > ---
> >  rust/kernel/xarray.rs | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> > 
> > diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> > index a49d6db28845..1b882cd2f58b 100644
> > --- a/rust/kernel/xarray.rs
> > +++ b/rust/kernel/xarray.rs
> > @@ -266,6 +266,45 @@ pub fn store(
> >              Ok(unsafe { T::try_from_foreign(old) })
> >          }
> >      }
> > +
> > +    /// Allocates an empty slot within the given limit range and
> > stores `value` there.
> > +    ///
> > +    /// 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(
> > +        &mut self,
> > +        limit: bindings::xa_limit,
> > +        value: T,
> > +        gfp: alloc::Flags,
> > +    ) -> Result<u32, StoreError<T>> {
> > +        build_assert!(
> > +            T::FOREIGN_ALIGN >= 4,
> > +            "pointers stored in XArray must be 4-byte aligned"
> > +        );
> > +
> > +        let new = value.into_foreign();
> > +        let mut id: u32 = 0;
> > +
> > +        // SAFETY:
> > +        // - `self.xa.xa` is valid by the type invariant.
> > +        // - `new` came from `T::into_foreign`.
> > +        let ret =
> > +            unsafe { bindings::__xa_alloc(self.xa.xa.get(), &mut
> > id, new, limit, gfp.as_raw()) }; +
> > +        if ret < 0 {
> > +            // SAFETY: `__xa_alloc` doesn't take ownership on
> > error.
> > +            let value = unsafe { T::from_foreign(new) };
> > +            return Err(StoreError {
> > +                value,
> > +                error: Error::from_errno(ret),
> > +            });
> > +        }
> > +
> > +        Ok(id)
> > +    }
> >  }
> > 
> >  // SAFETY: `XArray<T>` has no shared mutable state so it is `Send`
> > iff `T` is `Send`. --
> > 2.51.0
> > 

Re: [PATCH 2/3] rust: xarray: abstract `xa_alloc_cyclic`

[Alice Ryhl]

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@onurozkan.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.

> +        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.

> +        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

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Alice Ryhl]

On Mon, Oct 06, 2025 at 09:31:43PM +0200, Benno Lossin wrote:
> On Mon Oct 6, 2025 at 6:30 PM CEST, Onur Özkan wrote:
> > Implements `alloc` function to `XArray<T>` that wraps
> > `xa_alloc` 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@onurozkan.dev>
> > ---
> >  rust/kernel/xarray.rs | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> > index a49d6db28845..1b882cd2f58b 100644
> > --- a/rust/kernel/xarray.rs
> > +++ b/rust/kernel/xarray.rs
> > @@ -266,6 +266,45 @@ pub fn store(
> >              Ok(unsafe { T::try_from_foreign(old) })
> >          }
> >      }
> > +
> > +    /// Allocates an empty slot within the given limit range and stores `value` there.
> > +    ///
> > +    /// 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(
> > +        &mut self,
> > +        limit: bindings::xa_limit,
> > +        value: T,
> > +        gfp: alloc::Flags,
> > +    ) -> Result<u32, StoreError<T>> {
> 
> I think it would be a good idea to make the id a newtype wrapper around
> u32. Maybe not even allow users to manually construct it or even inspect
> it if possible.

What? People need to know what the assigned index is.

Alice

Re: [PATCH 0/3] rust: xarray: abstract `xa_alloc` and `xa_alloc_cyclic`

[Alice Ryhl]

On Mon, Oct 06, 2025 at 07:30:21PM +0300, Onur Özkan wrote:
> Initial version of xa_alloc and xa_alloc_cyclic abstraction
> patch series.
> 
> Onur Özkan (3):
>   rust: xarray: abstract `xa_alloc`
>   rust: xarray: abstract `xa_alloc_cyclic`
>   remove completed task from nova-core task list
> 
>  Documentation/gpu/nova/core/todo.rst |  8 ---
>  rust/kernel/xarray.rs                | 82 ++++++++++++++++++++++++++++
>  2 files changed, 82 insertions(+), 8 deletions(-)

We should send xarray patches to the linux-mm@kvack.org too.

Alice

Re: [PATCH 2/3] rust: xarray: abstract `xa_alloc_cyclic`

[Onur Özkan]

On Tue, 7 Oct 2025 10:56:56 +0000
Alice Ryhl <aliceryhl@google.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@onurozkan.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

Re: [PATCH 0/3] rust: xarray: abstract `xa_alloc` and `xa_alloc_cyclic`

[Onur Özkan]

On Tue, 7 Oct 2025 11:01:59 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Mon, Oct 06, 2025 at 07:30:21PM +0300, Onur Özkan wrote:
> > Initial version of xa_alloc and xa_alloc_cyclic abstraction
> > patch series.
> > 
> > Onur Özkan (3):
> >   rust: xarray: abstract `xa_alloc`
> >   rust: xarray: abstract `xa_alloc_cyclic`
> >   remove completed task from nova-core task list
> > 
> >  Documentation/gpu/nova/core/todo.rst |  8 ---
> >  rust/kernel/xarray.rs                | 82
> > ++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 8
> > deletions(-)
> 
> We should send xarray patches to the linux-mm@kvack.org too.
> 
> Alice

I suppose it should be added to the MAINTAINERS file? `get_maintainer.pl
rust/kernel/xarray.rs` doesn't give that address right now.

Thanks,
Onur

Re: [PATCH 2/3] rust: xarray: abstract `xa_alloc_cyclic`

[Miguel Ojeda]

On Tue, Oct 7, 2025 at 7:19 PM Onur Özkan <work@onurozkan.dev> wrote:
>
> Why do we not like to use the bindings type directly?

For public APIs, we generally try to avoid exposing C types:

    https://docs.kernel.org/rust/general-information.html#abstractions-vs-bindings

Sometimes it still makes sense, of course, e.g. a method may return
an inner type so that it gets used by other abstractions to call into
C. But generally we want to avoid exposing those for drivers and other
abstractions wherever possible.

Cheers,
Miguel

Re: [PATCH 2/3] rust: xarray: abstract `xa_alloc_cyclic`

[Onur Özkan]

On Tue, 7 Oct 2025 19:28:31 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Tue, Oct 7, 2025 at 7:19 PM Onur Özkan <work@onurozkan.dev> wrote:
> >
> > Why do we not like to use the bindings type directly?
> 
> For public APIs, we generally try to avoid exposing C types:
> 
>     https://docs.kernel.org/rust/general-information.html#abstractions-vs-bindings
> 
> Sometimes it still makes sense, of course, e.g. a method may return
> an inner type so that it gets used by other abstractions to call into
> C. But generally we want to avoid exposing those for drivers and other
> abstractions wherever possible.
> 
> Cheers,
> Miguel

Thank you for explaining it, I will drop the bindings type in the next
version.

Regards,
Onur

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Benno Lossin]

On Tue Oct 7, 2025 at 12:58 PM CEST, Alice Ryhl wrote:
> On Mon, Oct 06, 2025 at 09:31:43PM +0200, Benno Lossin wrote:
>> On Mon Oct 6, 2025 at 6:30 PM CEST, Onur Özkan wrote:
>> > Implements `alloc` function to `XArray<T>` that wraps
>> > `xa_alloc` 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@onurozkan.dev>
>> > ---
>> >  rust/kernel/xarray.rs | 39 +++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 39 insertions(+)
>> >
>> > diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
>> > index a49d6db28845..1b882cd2f58b 100644
>> > --- a/rust/kernel/xarray.rs
>> > +++ b/rust/kernel/xarray.rs
>> > @@ -266,6 +266,45 @@ pub fn store(
>> >              Ok(unsafe { T::try_from_foreign(old) })
>> >          }
>> >      }
>> > +
>> > +    /// Allocates an empty slot within the given limit range and stores `value` there.
>> > +    ///
>> > +    /// 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(
>> > +        &mut self,
>> > +        limit: bindings::xa_limit,
>> > +        value: T,
>> > +        gfp: alloc::Flags,
>> > +    ) -> Result<u32, StoreError<T>> {
>> 
>> I think it would be a good idea to make the id a newtype wrapper around
>> u32. Maybe not even allow users to manually construct it or even inspect
>> it if possible.
>
> What? People need to know what the assigned index is.

The documentation says "allocated id", so I assumed that it was some
internal, implementation-dependent thing, not an index. In that case we
should change the docs instead.

---
Cheers,
Benno

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Alice Ryhl]

On Wed, Oct 08, 2025 at 12:18:24PM +0200, Benno Lossin wrote:
> On Tue Oct 7, 2025 at 12:58 PM CEST, Alice Ryhl wrote:
> > On Mon, Oct 06, 2025 at 09:31:43PM +0200, Benno Lossin wrote:
> >> On Mon Oct 6, 2025 at 6:30 PM CEST, Onur Özkan wrote:
> >> > Implements `alloc` function to `XArray<T>` that wraps
> >> > `xa_alloc` 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@onurozkan.dev>
> >> > ---
> >> >  rust/kernel/xarray.rs | 39 +++++++++++++++++++++++++++++++++++++++
> >> >  1 file changed, 39 insertions(+)
> >> >
> >> > diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> >> > index a49d6db28845..1b882cd2f58b 100644
> >> > --- a/rust/kernel/xarray.rs
> >> > +++ b/rust/kernel/xarray.rs
> >> > @@ -266,6 +266,45 @@ pub fn store(
> >> >              Ok(unsafe { T::try_from_foreign(old) })
> >> >          }
> >> >      }
> >> > +
> >> > +    /// Allocates an empty slot within the given limit range and stores `value` there.
> >> > +    ///
> >> > +    /// 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(
> >> > +        &mut self,
> >> > +        limit: bindings::xa_limit,
> >> > +        value: T,
> >> > +        gfp: alloc::Flags,
> >> > +    ) -> Result<u32, StoreError<T>> {
> >> 
> >> I think it would be a good idea to make the id a newtype wrapper around
> >> u32. Maybe not even allow users to manually construct it or even inspect
> >> it if possible.
> >
> > What? People need to know what the assigned index is.
> 
> The documentation says "allocated id", so I assumed that it was some
> internal, implementation-dependent thing, not an index. In that case we
> should change the docs instead.

An xarray is a map from integer to pointer. The allocated id the key in
this map. The alloc method picks the smallest unused key in a given
range.

Alice

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Onur Özkan]

On Wed, 8 Oct 2025 13:01:34 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Wed, Oct 08, 2025 at 12:18:24PM +0200, Benno Lossin wrote:
> > On Tue Oct 7, 2025 at 12:58 PM CEST, Alice Ryhl wrote:
> > > On Mon, Oct 06, 2025 at 09:31:43PM +0200, Benno Lossin wrote:
> > >> On Mon Oct 6, 2025 at 6:30 PM CEST, Onur Özkan wrote:
> > >> > Implements `alloc` function to `XArray<T>` that wraps
> > >> > `xa_alloc` 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@onurozkan.dev>
> > >> > ---
> > >> >  rust/kernel/xarray.rs | 39
> > >> > +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39
> > >> > insertions(+)
> > >> >
> > >> > diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> > >> > index a49d6db28845..1b882cd2f58b 100644
> > >> > --- a/rust/kernel/xarray.rs
> > >> > +++ b/rust/kernel/xarray.rs
> > >> > @@ -266,6 +266,45 @@ pub fn store(
> > >> >              Ok(unsafe { T::try_from_foreign(old) })
> > >> >          }
> > >> >      }
> > >> > +
> > >> > +    /// Allocates an empty slot within the given limit range
> > >> > and stores `value` there.
> > >> > +    ///
> > >> > +    /// 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(
> > >> > +        &mut self,
> > >> > +        limit: bindings::xa_limit,
> > >> > +        value: T,
> > >> > +        gfp: alloc::Flags,
> > >> > +    ) -> Result<u32, StoreError<T>> {
> > >> 
> > >> I think it would be a good idea to make the id a newtype wrapper
> > >> around u32. Maybe not even allow users to manually construct it
> > >> or even inspect it if possible.
> > >
> > > What? People need to know what the assigned index is.
> > 
> > The documentation says "allocated id", so I assumed that it was some
> > internal, implementation-dependent thing, not an index. In that
> > case we should change the docs instead.
> 
> An xarray is a map from integer to pointer. The allocated id the key
> in this map. The alloc method picks the smallest unused key in a given
> range.
> 
> Alice

Perhaps we should document it as "allocated key" or "allocated id (key)"
?

-Onur

Re: [PATCH 1/3] rust: xarray: abstract `xa_alloc`

[Alice Ryhl]

On Wed, Oct 08, 2025 at 04:22:01PM +0300, Onur Özkan wrote:
> On Wed, 8 Oct 2025 13:01:34 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> 
> > On Wed, Oct 08, 2025 at 12:18:24PM +0200, Benno Lossin wrote:
> > > On Tue Oct 7, 2025 at 12:58 PM CEST, Alice Ryhl wrote:
> > > The documentation says "allocated id", so I assumed that it was some
> > > internal, implementation-dependent thing, not an index. In that
> > > case we should change the docs instead.
> > 
> > An xarray is a map from integer to pointer. The allocated id the key
> > in this map. The alloc method picks the smallest unused key in a given
> > range.
> > 
> > Alice
> 
> Perhaps we should document it as "allocated key" or "allocated id (key)"
> ?

I think 'allocated key' makes sense.

Alice