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
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
>
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
> >
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"
> + );
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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.