[PATCH RFC 0/4] rust: Introduce support for C linked list interfacing and DRM Buddy bindings

[PATCH RFC 0/4] rust: Introduce support for C linked list interfacing and DRM Buddy bindings

[Joel Fernandes]

Introduction
============
This patchset introduces an interface to iterate over doubly circular linked
lists used in the kernel (allocated by C kernel code). The main usecase is
iterating over the list of blocks provided by the DRM buddy allocator.

The first 2 patches introduce a new rust module called clist. The last 2
patches use the clist in the DRM buddy allocator bindings. Samples have been
provided to show the usage.

A question may arise:  Why not use rust list.rs for this?
=========================================================
Rust's list.rs is used to provide safe intrusive lists for Rust-allocated
items. In doing so, it takes ownership of the items in the list and the links
between list items. However, the usecase for DRM buddy allocator bindings, the
C side allocates the items in the list, and also links the list together. Due
to this, there is an ownership conflict making list.rs not the best abstraction
for this usecase. What we need is a view of the list, not ownership of it.
Further, the list links in a bindings usecase may come from C allocated
objects, not from the Rust side.

Other comments
==============
The patches IMO are ahead of RFC-quality, but I am still keeping the RFC
tag for initial comments and to get a sense of what everyone thinks. I already
presented the idea in Zulip and it seemed it mostly got agreements there. I
rebased the patches on linux-next. I can also add MAINTAINER entries in v2, if
folks agree this should have its own MAINTAINER record.

Joel Fernandes (4):
  rust: clist: Add abstraction for iterating over C linked lists
  samples: rust: Add sample demonstrating C linked list iteration
  rust: drm: Add DRM buddy allocator bindings
  samples: rust: Add sample demonstrating DRM buddy allocator

 rust/bindings/bindings_helper.h |  11 +
 rust/helpers/drm.c              |  23 +-
 rust/helpers/helpers.c          |   1 +
 rust/helpers/list.c             |  28 +++
 rust/kernel/clist.rs            | 296 ++++++++++++++++++++++++++
 rust/kernel/drm/buddy.rs        | 357 ++++++++++++++++++++++++++++++++
 rust/kernel/drm/mod.rs          |   1 +
 rust/kernel/lib.rs              |   1 +
 samples/rust/Kconfig            |  25 +++
 samples/rust/Makefile           |   3 +
 samples/rust/rust_clist_c.c     |  54 +++++
 samples/rust/rust_clist_main.rs | 138 ++++++++++++
 samples/rust/rust_drm_buddy.rs  | 106 ++++++++++
 13 files changed, 1043 insertions(+), 1 deletion(-)
 create mode 100644 rust/helpers/list.c
 create mode 100644 rust/kernel/clist.rs
 create mode 100644 rust/kernel/drm/buddy.rs
 create mode 100644 samples/rust/rust_clist_c.c
 create mode 100644 samples/rust/rust_clist_main.rs
 create mode 100644 samples/rust/rust_drm_buddy.rs


base-commit: 131f3d9446a6075192cdd91f197989d98302faa6
-- 
2.34.1

[PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Joel Fernandes]

Provides a safe interface for iterating over C's intrusive
linked lists (`list_head` structures). At the moment, supports
only read-only iteration. DRM Buddy bindings will use this to
iterate over DRM buddy blocks allocated in a linked list.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 rust/helpers/helpers.c |   1 +
 rust/helpers/list.c    |  28 ++++
 rust/kernel/clist.rs   | 296 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs     |   1 +
 4 files changed, 326 insertions(+)
 create mode 100644 rust/helpers/list.c
 create mode 100644 rust/kernel/clist.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 79c72762ad9c..634fa2386bbb 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -32,6 +32,7 @@
 #include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
+#include "list.c"
 #include "maple_tree.c"
 #include "mm.c"
 #include "mutex.c"
diff --git a/rust/helpers/list.c b/rust/helpers/list.c
new file mode 100644
index 000000000000..74e8f40b7141
--- /dev/null
+++ b/rust/helpers/list.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/list.h>
+
+bool rust_helper_list_empty(const struct list_head *head)
+{
+	return list_empty(head);
+}
+
+void rust_helper_list_del(struct list_head *entry)
+{
+	list_del(entry);
+}
+
+void rust_helper_INIT_LIST_HEAD(struct list_head *list)
+{
+	INIT_LIST_HEAD(list);
+}
+
+void rust_helper_list_add(struct list_head *new, struct list_head *head)
+{
+	list_add(new, head);
+}
+
+void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
+{
+	list_add_tail(new, head);
+}
diff --git a/rust/kernel/clist.rs b/rust/kernel/clist.rs
new file mode 100644
index 000000000000..e6a46974b1ba
--- /dev/null
+++ b/rust/kernel/clist.rs
@@ -0,0 +1,296 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! List processing module for C list_head linked lists.
+//!
+//! C header: [`include/linux/list.h`](srctree/include/linux/list.h)
+//!
+//! Provides a safe interface for iterating over C's intrusive `list_head` structures.
+//! At the moment, supports only read-only iteration.
+//!
+//! # Examples
+//!
+//! ```ignore
+//! use core::ptr::NonNull;
+//! use kernel::{
+//!     bindings,
+//!     clist,
+//!     container_of,
+//!     prelude::*, //
+//! };
+//!
+//! // Example C-side struct (typically from C bindings):
+//! // struct c_item {
+//! //     u64 offset;
+//! //     struct list_head link;
+//! //     /* ... other fields ... */
+//! // };
+//!
+//! // Rust-side struct to hold pointer to C-side struct.
+//! struct Item {
+//!     ptr: NonNull<bindings::c_item>,
+//! }
+//!
+//! impl clist::FromListHead for Item {
+//!     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
+//!         let item_ptr = container_of!(link, bindings::c_item, link) as *mut _;
+//!         Item { ptr: NonNull::new_unchecked(item_ptr) }
+//!     }
+//! }
+//!
+//! impl Item {
+//!     fn offset(&self) -> u64 {
+//!         unsafe { (*self.ptr.as_ptr()).offset }
+//!     }
+//! }
+//!
+//! // Get the list head from C code.
+//! let c_list_head = unsafe { bindings::get_item_list() };
+//!
+//! // Rust wraps and iterates over the list.
+//! let list = unsafe { clist::Clist::<Item>::new(c_list_head) };
+//!
+//! // Check if empty.
+//! if list.is_empty() {
+//!     pr_info!("List is empty\n");
+//! }
+//!
+//! // Iterate over items.
+//! for item in list.iter() {
+//!     pr_info!("Item offset: {}\n", item.offset());
+//! }
+//! ```
+
+use crate::bindings;
+use core::marker::PhantomData;
+
+/// Trait for types that can be constructed from a C list_head pointer.
+///
+/// This typically encapsulates `container_of` logic, allowing iterators to
+/// work with high-level types without knowing about C struct layout details.
+///
+/// # Safety
+///
+/// Implementors must ensure that `from_list_head` correctly converts the
+/// `list_head` pointer to the containing struct pointer using proper offset
+/// calculations (typically via container_of! macro).
+///
+/// # Examples
+///
+/// ```ignore
+/// impl FromListHead for MyItem {
+///     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
+///         let item_ptr = container_of!(link, bindings::my_c_struct, link_field) as *mut _;
+///         MyItem { ptr: NonNull::new_unchecked(item_ptr) }
+///     }
+/// }
+/// ```
+pub trait FromListHead: Sized {
+    /// Create instance from list_head pointer embedded in containing struct.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure `link` points to a valid list_head embedded in the
+    /// containing struct, and that the containing struct is valid for the
+    /// lifetime of the returned instance.
+    unsafe fn from_list_head(link: *const bindings::list_head) -> Self;
+}
+
+/// Iterator over C list items.
+///
+/// Uses the [`FromListHead`] trait to convert list_head pointers to
+/// Rust types and iterate over them.
+///
+/// # Invariants
+/// - `head` points to a valid, initialized list_head.
+/// - `current` points to a valid node in the list.
+/// - The list is not modified during iteration.
+///
+/// # Examples
+///
+/// ```ignore
+/// // Iterate over blocks in a C list_head
+/// for block in clist::iter_list_head::<Block>(&list_head) {
+///     // Process block
+/// }
+/// ```
+pub struct ClistIter<'a, T: FromListHead> {
+    current: *const bindings::list_head,
+    head: *const bindings::list_head,
+    _phantom: PhantomData<&'a T>,
+}
+
+// SAFETY: Safe to send iterator if T is Send.
+unsafe impl<'a, T: FromListHead + Send> Send for ClistIter<'a, T> {}
+
+impl<'a, T: FromListHead> Iterator for ClistIter<'a, T> {
+    type Item = T;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        // SAFETY: Pointers are valid per the type's invariants. The list
+        // structure is valid and we traverse according to kernel list semantics.
+        unsafe {
+            self.current = (*self.current).next;
+
+            if self.current == self.head {
+                return None;
+            }
+
+            // Use the trait to convert list_head to T.
+            Some(T::from_list_head(self.current))
+        }
+    }
+}
+
+/// Create a read-only iterator over a C list_head.
+///
+/// This is a convenience function for creating iterators directly
+/// from a list_head reference.
+///
+/// # Safety
+///
+/// Caller must ensure:
+/// - `head` is a valid, initialized list_head.
+/// - All items in the list are valid instances that can be converted via [`FromListHead`].
+/// - The list is not modified during iteration.
+///
+/// # Examples
+///
+/// ```ignore
+/// // Iterate over items in a C list.
+/// for item in clist::iter_list_head::<Item>(&c_list_head) {
+///     // Process item
+/// }
+/// ```
+pub fn iter_list_head<'a, T: FromListHead>(head: &'a bindings::list_head) -> ClistIter<'a, T> {
+    ClistIter {
+        current: head as *const _,
+        head: head as *const _,
+        _phantom: PhantomData,
+    }
+}
+
+/// Check if a C list is empty.
+///
+/// # Safety
+///
+/// Caller must ensure `head` points to a valid, initialized list_head.
+#[inline]
+pub unsafe fn list_empty(head: *const bindings::list_head) -> bool {
+    // SAFETY: Caller ensures head is valid and initialized.
+    unsafe { bindings::list_empty(head) }
+}
+
+/// Initialize a C list_head to an empty list.
+///
+/// # Safety
+///
+/// Caller must ensure `head` points to valid memory for a list_head.
+#[inline]
+pub unsafe fn init_list_head(head: *mut bindings::list_head) {
+    // SAFETY: Caller ensures head points to valid memory for a list_head.
+    unsafe { bindings::INIT_LIST_HEAD(head) }
+}
+
+/// An interface to C list_head structures.
+///
+/// This provides an iterator-based interface for traversing C's intrusive
+/// linked lists. Items must implement the [`FromListHead`] trait.
+///
+/// Designed for iterating over lists created and managed by C code (e.g.,
+/// drm_buddy block lists). [`Clist`] does not own the `list_head` or the items.
+/// It's a non-owning view for iteration.
+///
+/// # Invariants
+/// - `head` points to a valid, initialized list_head.
+/// - All items in the list are valid instances of `T`.
+/// - The list is not modified while iterating.
+///
+/// # Thread Safety
+/// [`Clist`] can have its ownership transferred between threads ([`Send`]) if `T: Send`.
+/// But its never [`Sync`] - concurrent Rust threads with `&Clist` could call C FFI unsafely.
+/// For concurrent access, wrap in a `Mutex` or other synchronization primitive.
+///
+/// # Examples
+///
+/// ```ignore
+/// use kernel::clist::Clist;
+///
+/// // C code provides the populated list_head.
+/// let list = unsafe { Clist::<Item>::new(c_list_head) };
+///
+/// // Iterate over items.
+/// for item in list.iter() {
+///     // Process item.
+/// }
+/// ```
+pub struct Clist<T: FromListHead> {
+    head: *mut bindings::list_head,
+    _phantom: PhantomData<T>,
+}
+
+// SAFETY: Safe to send Clist if T is Send (pointer moves, C data stays in place).
+unsafe impl<T: FromListHead + Send> Send for Clist<T> {}
+
+impl<T: FromListHead> Clist<T> {
+    /// Wrap an existing C list_head pointer without taking ownership.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure:
+    /// - `head` points to a valid, initialized list_head.
+    /// - `head` remains valid for the lifetime of the returned [`Clist`].
+    /// - The list is not modified by C code while [`Clist`] exists. Caller must
+    ///   implement mutual exclusion algorithms if required, to coordinate with C.
+    /// - Caller is responsible for requesting or working with C to free `head` if needed.
+    pub unsafe fn new(head: *mut bindings::list_head) -> Self {
+        // SAFETY: Caller ensures head is valid and initialized
+        Self {
+            head,
+            _phantom: PhantomData,
+        }
+    }
+
+    /// Check if the list is empty.
+    ///
+    /// # Examples
+    ///
+    /// ```ignore
+    /// let list = Clist::<Block>::new()?;
+    /// assert!(list.is_empty());
+    /// ```
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        // SAFETY: self.head points to valid list_head per invariant.
+        unsafe { list_empty(self.head) }
+    }
+
+    /// Iterate over items in the list.
+    ///
+    /// # Examples
+    ///
+    /// ```ignore
+    /// for item in list.iter() {
+    ///     // Process item
+    /// }
+    /// ```
+    pub fn iter(&self) -> ClistIter<'_, T> {
+        // SAFETY: self.head points to valid list_head per invariant.
+        unsafe { iter_list_head::<T>(&*self.head) }
+    }
+
+    /// Get the raw list_head pointer.
+    ///
+    /// This is useful when interfacing with C APIs that need the list_head pointer.
+    pub fn as_raw(&self) -> *mut bindings::list_head {
+        self.head
+    }
+}
+
+impl<'a, T: FromListHead> IntoIterator for &'a Clist<T> {
+    type Item = T;
+    type IntoIter = ClistIter<'a, T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.iter()
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c2eea9a2a345..b69cc5ed3b59 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -72,6 +72,7 @@
 pub mod bug;
 #[doc(hidden)]
 pub mod build_assert;
+pub mod clist;
 pub mod clk;
 #[cfg(CONFIG_CONFIGFS_FS)]
 pub mod configfs;
-- 
2.34.1

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Danilo Krummrich]

On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
> Provides a safe interface for iterating over C's intrusive

I'm not sure we're there just yet, I count eight unsafe blocks in the subsequent
sample code.

Don't get me wrong, there is no way to make the API safe entirely, but "safe
interface" is clearly a wrong promise. :)

Some more high level comments below.

> +//! // Rust-side struct to hold pointer to C-side struct.
> +//! struct Item {
> +//!     ptr: NonNull<bindings::c_item>,
> +//! }
> +//!
> +//! impl clist::FromListHead for Item {
> +//!     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
> +//!         let item_ptr = container_of!(link, bindings::c_item, link) as *mut _;
> +//!         Item { ptr: NonNull::new_unchecked(item_ptr) }
> +//!     }
> +//! }

If you just embedd a pointer to the C struct in a struct Item you don't cover
the lifetime relationship.

Instead this should be something like

	#[repr(transparent)]
	struct Entry<T>(Opaque<T>);

or

	struct Entry<'a, T>(NonNull<T>, PhantomData<&'a T>);

where T is the C list entry type.

You can then have a setup where an &Entry borrows from a &CListHead, which
borrows from a Clist.

I'd also provide a macro for users to generate this structure as well as the
corresponding FromListHead impl.

> +//! // Rust wraps and iterates over the list.
> +//! let list = unsafe { clist::Clist::<Item>::new(c_list_head) };

This function has a lot of safety requirements that need to be covered.

It also should be, besides the unsafe FromListHead trait, the only unsafe
function needed.

The Clist should ideally have methods for all kinds of list iterators, e.g.
list_for_each_entry_{safe,reverse,continue}() etc.

Of course you don't need to provide all of them in an initial implementation,
but we should set the direction.

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Joel Fernandes]

Hi Danilo,

On 10/30/2025 5:15 PM, Danilo Krummrich wrote:
> On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
>> Provides a safe interface for iterating over C's intrusive
> 
> I'm not sure we're there just yet, I count eight unsafe blocks in the subsequent
> sample code.
> 
> Don't get me wrong, there is no way to make the API safe entirely, but "safe
> interface" is clearly a wrong promise. :)

Well, to be fair most of the unsafe statements are related to bindings, not
clist per-se. There are 8 unsafe references in the sample, of these 3 are
related to clist.  The remaining 5 are bindings/ffi related. However, I am Ok
with removing the mention of 'safe' from this comment.

> 
> Some more high level comments below.
> 
>> +//! // Rust-side struct to hold pointer to C-side struct.
>> +//! struct Item {
>> +//!     ptr: NonNull<bindings::c_item>,
>> +//! }
>> +//!
>> +//! impl clist::FromListHead for Item {
>> +//!     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
>> +//!         let item_ptr = container_of!(link, bindings::c_item, link) as *mut _;
>> +//!         Item { ptr: NonNull::new_unchecked(item_ptr) }
>> +//!     }
>> +//! }
> 
> If you just embedd a pointer to the C struct in a struct Item you don't cover
> the lifetime relationship.
> 
> Instead this should be something like
> 
> 	#[repr(transparent)]
> 	struct Entry<T>(Opaque<T>);
> 
> or
> 
> 	struct Entry<'a, T>(NonNull<T>, PhantomData<&'a T>);
> 
> where T is the C list entry type.
> 
> You can then have a setup where an &Entry borrows from a &CListHead, which
> borrows from a Clist.

Yes, in my implementation my iterator creates a new value on in iterator's
next() function without lifetime relationships:

            // Use the trait to convert list_head to T.
            Some(T::from_list_head(self.current))

I think you're saying that we should have a lifetime relationship between the
rust Clist and the rust entry (item) returned by from_list_head() right?

Your suggestion makes sense for cases where a rust Item/Entry has a Drop
implementation that then makes the C side free the object and its list_head. DRM
Buddy does not have this requirement, however your suggestion makes the code
future proof and better. Basically, the rust item wrapper must outlive the clist
view is what you're saying, I think. That can be achieved by making the rust
item borrow from the clist. I will work on this accordingly then.

> I'd also provide a macro for users to generate this structure as well as the
> corresponding FromListHead impl.
> 
>> +//! // Rust wraps and iterates over the list.
>> +//! let list = unsafe { clist::Clist::<Item>::new(c_list_head) };
> 
> This function has a lot of safety requirements that need to be covered.

Sure, I can add those to the example as well.

> 
> It also should be, besides the unsafe FromListHead trait, the only unsafe
> function needed.
> 
> The Clist should ideally have methods for all kinds of list iterators, e.g.
> list_for_each_entry_{safe,reverse,continue}() etc.
> 
> Of course you don't need to provide all of them in an initial implementation,
> but we should set the direction.

Yes, initially I am trying to only provide most common thinks, especially what
the DRM Buddy usecase needs. Happy to improve it over time.

thanks,

 - Joel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Alexandre Courbot]

Hi Joel,

On Fri Oct 31, 2025 at 4:06 AM JST, Joel Fernandes wrote:
<snip>
> diff --git a/rust/kernel/clist.rs b/rust/kernel/clist.rs
> new file mode 100644
> index 000000000000..e6a46974b1ba
> --- /dev/null
> +++ b/rust/kernel/clist.rs
> @@ -0,0 +1,296 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! List processing module for C list_head linked lists.
> +//!
> +//! C header: [`include/linux/list.h`](srctree/include/linux/list.h)
> +//!
> +//! Provides a safe interface for iterating over C's intrusive `list_head` structures.
> +//! At the moment, supports only read-only iteration.
> +//!
> +//! # Examples
> +//!
> +//! ```ignore

Examples pull double-duty as unit tests, and making them build and run
ensures that they never fall out-of-date as the code evolves. Please
make sure that they can be built and run. Supporting code that you don't
want to show in the doc can be put behind `#`.

> +//! use core::ptr::NonNull;
> +//! use kernel::{
> +//!     bindings,
> +//!     clist,
> +//!     container_of,
> +//!     prelude::*, //
> +//! };
> +//!
> +//! // Example C-side struct (typically from C bindings):
> +//! // struct c_item {
> +//! //     u64 offset;
> +//! //     struct list_head link;
> +//! //     /* ... other fields ... */
> +//! // };
> +//!
> +//! // Rust-side struct to hold pointer to C-side struct.
> +//! struct Item {
> +//!     ptr: NonNull<bindings::c_item>,
> +//! }

As Danilo suggested, using a e.g. `Entry` structure that just wraps
`Self` inside an `Opaque` would allow capturing the lifetime as well.
Look at how `SGEntry` and its `from_raw` method are done, it should look
very similar.

Doing so would also spare users the trouble of having to define a
dedicated type.

> +//!
> +//! impl clist::FromListHead for Item {
> +//!     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
> +//!         let item_ptr = container_of!(link, bindings::c_item, link) as *mut _;
> +//!         Item { ptr: NonNull::new_unchecked(item_ptr) }
> +//!     }
> +//! }
> +//!
> +//! impl Item {
> +//!     fn offset(&self) -> u64 {
> +//!         unsafe { (*self.ptr.as_ptr()).offset }
> +//!     }
> +//! }
> +//!
> +//! // Get the list head from C code.
> +//! let c_list_head = unsafe { bindings::get_item_list() };
> +//!
> +//! // Rust wraps and iterates over the list.
> +//! let list = unsafe { clist::Clist::<Item>::new(c_list_head) };
> +//!
> +//! // Check if empty.
> +//! if list.is_empty() {
> +//!     pr_info!("List is empty\n");
> +//! }
> +//!
> +//! // Iterate over items.
> +//! for item in list.iter() {
> +//!     pr_info!("Item offset: {}\n", item.offset());
> +//! }
> +//! ```
> +
> +use crate::bindings;
> +use core::marker::PhantomData;
> +
> +/// Trait for types that can be constructed from a C list_head pointer.
> +///
> +/// This typically encapsulates `container_of` logic, allowing iterators to
> +/// work with high-level types without knowing about C struct layout details.
> +///
> +/// # Safety
> +///
> +/// Implementors must ensure that `from_list_head` correctly converts the
> +/// `list_head` pointer to the containing struct pointer using proper offset
> +/// calculations (typically via container_of! macro).
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// impl FromListHead for MyItem {
> +///     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
> +///         let item_ptr = container_of!(link, bindings::my_c_struct, link_field) as *mut _;
> +///         MyItem { ptr: NonNull::new_unchecked(item_ptr) }
> +///     }
> +/// }
> +/// ```
> +pub trait FromListHead: Sized {
> +    /// Create instance from list_head pointer embedded in containing struct.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Caller must ensure `link` points to a valid list_head embedded in the
> +    /// containing struct, and that the containing struct is valid for the
> +    /// lifetime of the returned instance.
> +    unsafe fn from_list_head(link: *const bindings::list_head) -> Self;
> +}

If we go with the `Entry` thing, this method would thus become:

    unsafe fn from_list_head<'a>(link: *const bindings::list_head) -> &'a Entry<Self>;

But that leaves an open question: how do we support items that have
several lists embedded in them? This is a pretty common pattern. Maybe
we can add a const parameter to `Entry` (with a default value) to
discriminate them.

> +
> +/// Iterator over C list items.
> +///
> +/// Uses the [`FromListHead`] trait to convert list_head pointers to
> +/// Rust types and iterate over them.
> +///
> +/// # Invariants

Missing empty line.

> +/// - `head` points to a valid, initialized list_head.
> +/// - `current` points to a valid node in the list.
> +/// - The list is not modified during iteration.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// // Iterate over blocks in a C list_head
> +/// for block in clist::iter_list_head::<Block>(&list_head) {
> +///     // Process block
> +/// }
> +/// ```
> +pub struct ClistIter<'a, T: FromListHead> {
> +    current: *const bindings::list_head,
> +    head: *const bindings::list_head,
> +    _phantom: PhantomData<&'a T>,
> +}
> +
> +// SAFETY: Safe to send iterator if T is Send.
> +unsafe impl<'a, T: FromListHead + Send> Send for ClistIter<'a, T> {}
> +
> +impl<'a, T: FromListHead> Iterator for ClistIter<'a, T> {
> +    type Item = T;
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        // SAFETY: Pointers are valid per the type's invariants. The list
> +        // structure is valid and we traverse according to kernel list semantics.
> +        unsafe {
> +            self.current = (*self.current).next;
> +
> +            if self.current == self.head {
> +                return None;
> +            }
> +
> +            // Use the trait to convert list_head to T.
> +            Some(T::from_list_head(self.current))
> +        }
> +    }
> +}
> +
> +/// Create a read-only iterator over a C list_head.
> +///
> +/// This is a convenience function for creating iterators directly
> +/// from a list_head reference.
> +///
> +/// # Safety
> +///
> +/// Caller must ensure:
> +/// - `head` is a valid, initialized list_head.
> +/// - All items in the list are valid instances that can be converted via [`FromListHead`].
> +/// - The list is not modified during iteration.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// // Iterate over items in a C list.
> +/// for item in clist::iter_list_head::<Item>(&c_list_head) {
> +///     // Process item
> +/// }
> +/// ```
> +pub fn iter_list_head<'a, T: FromListHead>(head: &'a bindings::list_head) -> ClistIter<'a, T> {
> +    ClistIter {
> +        current: head as *const _,
> +        head: head as *const _,
> +        _phantom: PhantomData,
> +    }
> +}

Why do we need a function for this? The correct way to iterate should be
through `CList`, so I'd just move its code to `CList::iter` and make all
the examples use that.

> +
> +/// Check if a C list is empty.
> +///
> +/// # Safety
> +///
> +/// Caller must ensure `head` points to a valid, initialized list_head.
> +#[inline]
> +pub unsafe fn list_empty(head: *const bindings::list_head) -> bool {
> +    // SAFETY: Caller ensures head is valid and initialized.
> +    unsafe { bindings::list_empty(head) }
> +}

Why not call `bindings::list_empty` directly from `is_empty`? I don't
see what having an extra public function brings here.

> +
> +/// Initialize a C list_head to an empty list.
> +///
> +/// # Safety
> +///
> +/// Caller must ensure `head` points to valid memory for a list_head.
> +#[inline]
> +pub unsafe fn init_list_head(head: *mut bindings::list_head) {
> +    // SAFETY: Caller ensures head points to valid memory for a list_head.
> +    unsafe { bindings::INIT_LIST_HEAD(head) }
> +}

Since this patch adds read-only support, what do we need this function
for? It also doesn't appear to be used anywhere in this series.

> +
> +/// An interface to C list_head structures.
> +///
> +/// This provides an iterator-based interface for traversing C's intrusive
> +/// linked lists. Items must implement the [`FromListHead`] trait.
> +///
> +/// Designed for iterating over lists created and managed by C code (e.g.,
> +/// drm_buddy block lists). [`Clist`] does not own the `list_head` or the items.
> +/// It's a non-owning view for iteration.
> +///
> +/// # Invariants

Missing empty line.

> +/// - `head` points to a valid, initialized list_head.
> +/// - All items in the list are valid instances of `T`.
> +/// - The list is not modified while iterating.
> +///
> +/// # Thread Safety

Here as well.

> +/// [`Clist`] can have its ownership transferred between threads ([`Send`]) if `T: Send`.
> +/// But its never [`Sync`] - concurrent Rust threads with `&Clist` could call C FFI unsafely.
> +/// For concurrent access, wrap in a `Mutex` or other synchronization primitive.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// use kernel::clist::Clist;
> +///
> +/// // C code provides the populated list_head.
> +/// let list = unsafe { Clist::<Item>::new(c_list_head) };
> +///
> +/// // Iterate over items.
> +/// for item in list.iter() {
> +///     // Process item.
> +/// }
> +/// ```
> +pub struct Clist<T: FromListHead> {
> +    head: *mut bindings::list_head,
> +    _phantom: PhantomData<T>,
> +}
> +
> +// SAFETY: Safe to send Clist if T is Send (pointer moves, C data stays in place).
> +unsafe impl<T: FromListHead + Send> Send for Clist<T> {}
> +
> +impl<T: FromListHead> Clist<T> {
> +    /// Wrap an existing C list_head pointer without taking ownership.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Caller must ensure:
> +    /// - `head` points to a valid, initialized list_head.
> +    /// - `head` remains valid for the lifetime of the returned [`Clist`].
> +    /// - The list is not modified by C code while [`Clist`] exists. Caller must
> +    ///   implement mutual exclusion algorithms if required, to coordinate with C.
> +    /// - Caller is responsible for requesting or working with C to free `head` if needed.
> +    pub unsafe fn new(head: *mut bindings::list_head) -> Self {
> +        // SAFETY: Caller ensures head is valid and initialized
> +        Self {
> +            head,
> +            _phantom: PhantomData,
> +        }
> +    }

I am wondering whether `CList` serves an actual purpose beyond providing
` CListIter` to iterate on... Would it make sense to merge both types
into a single one that implements `Iterator`?

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Joel Fernandes]

On Sat, Nov 01, 2025 at 12:51:32PM +0900, Alexandre Courbot wrote:
> Hi Joel,
> 
> On Fri Oct 31, 2025 at 4:06 AM JST, Joel Fernandes wrote:
> <snip>
> > diff --git a/rust/kernel/clist.rs b/rust/kernel/clist.rs
> > new file mode 100644
> > index 000000000000..e6a46974b1ba
> > --- /dev/null
> > +++ b/rust/kernel/clist.rs
> > @@ -0,0 +1,296 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! List processing module for C list_head linked lists.
> > +//!
> > +//! C header: [`include/linux/list.h`](srctree/include/linux/list.h)
> > +//!
> > +//! Provides a safe interface for iterating over C's intrusive `list_head` structures.
> > +//! At the moment, supports only read-only iteration.
> > +//!
> > +//! # Examples
> > +//!
> > +//! ```ignore
> 
> Examples pull double-duty as unit tests, and making them build and run
> ensures that they never fall out-of-date as the code evolves. Please
> make sure that they can be built and run. Supporting code that you don't
> want to show in the doc can be put behind `#`.

Sure, the reason I didn't do it was there are a couple of challenges:

1. For clist, there is a "C language" component" as well in the
sample, as these are lists coming from C. I am not sure how to do that as a
doc example, I might have to emulate a C struct containing a list_head in
Rust itself. Is that something we're Ok with? After all the purpose of a
sample, is to show how this could be used to interface with lists coming from
actual C code.

2. For DRM buddy, #1 is not an issue, however CONFIG_DRM_BUDDY has to be
enabled for the test to work. I have to figure out how to make a doc test be
kernel CONFIG dependent. What if the CONFIG required is disabled, is there a
standard way to make doc tests not fail for features that are disabled? Are the
doc tests skipped in such a situation? Just something I have to figure out.
Perhaps wrapping it is #cfg is sufficient.
Btw, I also realize my patch 3 introducing buddy.rs is not dependent on
CONFIG_DRM_BUDDY, which it should be. I was testing with CONFIG_DRM_BUDDY
always enabled, which is probably why I didn't catch this.

> > +//! use core::ptr::NonNull;
> > +//! use kernel::{
> > +//!     bindings,
> > +//!     clist,
> > +//!     container_of,
> > +//!     prelude::*, //
> > +//! };
> > +//!
> > +//! // Example C-side struct (typically from C bindings):
> > +//! // struct c_item {
> > +//! //     u64 offset;
> > +//! //     struct list_head link;
> > +//! //     /* ... other fields ... */
> > +//! // };
> > +//!
> > +//! // Rust-side struct to hold pointer to C-side struct.
> > +//! struct Item {
> > +//!     ptr: NonNull<bindings::c_item>,
> > +//! }
> 
> As Danilo suggested, using a e.g. `Entry` structure that just wraps
> `Self` inside an `Opaque` would allow capturing the lifetime as well.
> Look at how `SGEntry` and its `from_raw` method are done, it should look
> very similar.

Great ideas. I will look at SGEntry, at first look it seems a perfect fit
indeed.

> Doing so would also spare users the trouble of having to define a
> dedicated type.

True!

> > +//!
> > +//! impl clist::FromListHead for Item {
> > +//!     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
> > +//!         let item_ptr = container_of!(link, bindings::c_item, link) as *mut _;
> > +//!         Item { ptr: NonNull::new_unchecked(item_ptr) }
> > +//!     }
> > +//! }
> > +//!
> > +//! impl Item {
> > +//!     fn offset(&self) -> u64 {
> > +//!         unsafe { (*self.ptr.as_ptr()).offset }
> > +//!     }
> > +//! }
> > +//!
> > +//! // Get the list head from C code.
> > +//! let c_list_head = unsafe { bindings::get_item_list() };
> > +//!
> > +//! // Rust wraps and iterates over the list.
> > +//! let list = unsafe { clist::Clist::<Item>::new(c_list_head) };
> > +//!
> > +//! // Check if empty.
> > +//! if list.is_empty() {
> > +//!     pr_info!("List is empty\n");
> > +//! }
> > +//!
> > +//! // Iterate over items.
> > +//! for item in list.iter() {
> > +//!     pr_info!("Item offset: {}\n", item.offset());
> > +//! }
> > +//! ```
> > +
> > +use crate::bindings;
> > +use core::marker::PhantomData;
> > +
> > +/// Trait for types that can be constructed from a C list_head pointer.
> > +///
> > +/// This typically encapsulates `container_of` logic, allowing iterators to
> > +/// work with high-level types without knowing about C struct layout details.
> > +///
> > +/// # Safety
> > +///
> > +/// Implementors must ensure that `from_list_head` correctly converts the
> > +/// `list_head` pointer to the containing struct pointer using proper offset
> > +/// calculations (typically via container_of! macro).
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// impl FromListHead for MyItem {
> > +///     unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
> > +///         let item_ptr = container_of!(link, bindings::my_c_struct, link_field) as *mut _;
> > +///         MyItem { ptr: NonNull::new_unchecked(item_ptr) }
> > +///     }
> > +/// }
> > +/// ```
> > +pub trait FromListHead: Sized {
> > +    /// Create instance from list_head pointer embedded in containing struct.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Caller must ensure `link` points to a valid list_head embedded in the
> > +    /// containing struct, and that the containing struct is valid for the
> > +    /// lifetime of the returned instance.
> > +    unsafe fn from_list_head(link: *const bindings::list_head) -> Self;
> > +}
> 
> If we go with the `Entry` thing, this method would thus become:
> 
>     unsafe fn from_list_head<'a>(link: *const bindings::list_head) -> &'a Entry<Self>;

Sure.

> But that leaves an open question: how do we support items that have
> several lists embedded in them? This is a pretty common pattern. Maybe
> we can add a const parameter to `Entry` (with a default value) to
> discriminate them.

Ah, good point! as you mentioned, we could make it a parameter.

> > +
> > +/// Iterator over C list items.
> > +///
> > +/// Uses the [`FromListHead`] trait to convert list_head pointers to
> > +/// Rust types and iterate over them.
> > +///
> > +/// # Invariants
> 
> Missing empty line.

Ack.

> > +/// - `head` points to a valid, initialized list_head.
> > +/// - `current` points to a valid node in the list.
> > +/// - The list is not modified during iteration.
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// // Iterate over blocks in a C list_head
> > +/// for block in clist::iter_list_head::<Block>(&list_head) {
> > +///     // Process block
> > +/// }
> > +/// ```
> > +pub struct ClistIter<'a, T: FromListHead> {
> > +    current: *const bindings::list_head,
> > +    head: *const bindings::list_head,
> > +    _phantom: PhantomData<&'a T>,
> > +}
> > +
> > +// SAFETY: Safe to send iterator if T is Send.
> > +unsafe impl<'a, T: FromListHead + Send> Send for ClistIter<'a, T> {}
> > +
> > +impl<'a, T: FromListHead> Iterator for ClistIter<'a, T> {
> > +    type Item = T;
> > +
> > +    fn next(&mut self) -> Option<Self::Item> {
> > +        // SAFETY: Pointers are valid per the type's invariants. The list
> > +        // structure is valid and we traverse according to kernel list semantics.
> > +        unsafe {
> > +            self.current = (*self.current).next;
> > +
> > +            if self.current == self.head {
> > +                return None;
> > +            }
> > +
> > +            // Use the trait to convert list_head to T.
> > +            Some(T::from_list_head(self.current))
> > +        }
> > +    }
> > +}
> > +
> > +/// Create a read-only iterator over a C list_head.
> > +///
> > +/// This is a convenience function for creating iterators directly
> > +/// from a list_head reference.
> > +///
> > +/// # Safety
> > +///
> > +/// Caller must ensure:
> > +/// - `head` is a valid, initialized list_head.
> > +/// - All items in the list are valid instances that can be converted via [`FromListHead`].
> > +/// - The list is not modified during iteration.
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// // Iterate over items in a C list.
> > +/// for item in clist::iter_list_head::<Item>(&c_list_head) {
> > +///     // Process item
> > +/// }
> > +/// ```
> > +pub fn iter_list_head<'a, T: FromListHead>(head: &'a bindings::list_head) -> ClistIter<'a, T> {
> > +    ClistIter {
> > +        current: head as *const _,
> > +        head: head as *const _,
> > +        _phantom: PhantomData,
> > +    }
> > +}
> 
> Why do we need a function for this? The correct way to iterate should be
> through `CList`, so I'd just move its code to `CList::iter` and make all
> the examples use that.

Sure, I will move this function there. Are you saying we should also merge
`Clist` and `ClistIter` too or just move the function? I think we still want
to keep the 2 types separate as `ClistIter` stores the interation state
(current/head pointers).

> > +
> > +/// Check if a C list is empty.
> > +///
> > +/// # Safety
> > +///
> > +/// Caller must ensure `head` points to a valid, initialized list_head.
> > +#[inline]
> > +pub unsafe fn list_empty(head: *const bindings::list_head) -> bool {
> > +    // SAFETY: Caller ensures head is valid and initialized.
> > +    unsafe { bindings::list_empty(head) }
> > +}
> 
> Why not call `bindings::list_empty` directly from `is_empty`? I don't
> see what having an extra public function brings here.

Ok sure, yeah no reason not to do so :).

> > +
> > +/// Initialize a C list_head to an empty list.
> > +///
> > +/// # Safety
> > +///
> > +/// Caller must ensure `head` points to valid memory for a list_head.
> > +#[inline]
> > +pub unsafe fn init_list_head(head: *mut bindings::list_head) {
> > +    // SAFETY: Caller ensures head points to valid memory for a list_head.
> > +    unsafe { bindings::INIT_LIST_HEAD(head) }
> > +}
> 
> Since this patch adds read-only support, what do we need this function
> for? It also doesn't appear to be used anywhere in this series.

Ah, yes. I directly called the bindings in the DRM patch, instead of using
the wrapper. hmm from a readability PoV, bindings::INIT_LIST_HEAD() is itself
Ok so I'll just get rid of this function as well then.

> > +
> > +/// An interface to C list_head structures.
> > +///
> > +/// This provides an iterator-based interface for traversing C's intrusive
> > +/// linked lists. Items must implement the [`FromListHead`] trait.
> > +///
> > +/// Designed for iterating over lists created and managed by C code (e.g.,
> > +/// drm_buddy block lists). [`Clist`] does not own the `list_head` or the items.
> > +/// It's a non-owning view for iteration.
> > +///
> > +/// # Invariants
> 
> Missing empty line.

Ack.

> > +/// - `head` points to a valid, initialized list_head.
> > +/// - All items in the list are valid instances of `T`.
> > +/// - The list is not modified while iterating.
> > +///
> > +/// # Thread Safety
> 
> Here as well.

Ack.

> > +/// [`Clist`] can have its ownership transferred between threads ([`Send`]) if `T: Send`.
> > +/// But its never [`Sync`] - concurrent Rust threads with `&Clist` could call C FFI unsafely.
> > +/// For concurrent access, wrap in a `Mutex` or other synchronization primitive.
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// use kernel::clist::Clist;
> > +///
> > +/// // C code provides the populated list_head.
> > +/// let list = unsafe { Clist::<Item>::new(c_list_head) };
> > +///
> > +/// // Iterate over items.
> > +/// for item in list.iter() {
> > +///     // Process item.
> > +/// }
> > +/// ```
> > +pub struct Clist<T: FromListHead> {
> > +    head: *mut bindings::list_head,
> > +    _phantom: PhantomData<T>,
> > +}
> > +
> > +// SAFETY: Safe to send Clist if T is Send (pointer moves, C data stays in place).
> > +unsafe impl<T: FromListHead + Send> Send for Clist<T> {}
> > +
> > +impl<T: FromListHead> Clist<T> {
> > +    /// Wrap an existing C list_head pointer without taking ownership.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Caller must ensure:
> > +    /// - `head` points to a valid, initialized list_head.
> > +    /// - `head` remains valid for the lifetime of the returned [`Clist`].
> > +    /// - The list is not modified by C code while [`Clist`] exists. Caller must
> > +    ///   implement mutual exclusion algorithms if required, to coordinate with C.
> > +    /// - Caller is responsible for requesting or working with C to free `head` if needed.
> > +    pub unsafe fn new(head: *mut bindings::list_head) -> Self {
> > +        // SAFETY: Caller ensures head is valid and initialized
> > +        Self {
> > +            head,
> > +            _phantom: PhantomData,
> > +        }
> > +    }
> 
> I am wondering whether `CList` serves an actual purpose beyond providing
> ` CListIter` to iterate on... Would it make sense to merge both types
> into a single one that implements `Iterator`?
> 

It would force us to store iteration state into `Clist`, I think for that
reason it would be great to keep them separate IMO.

thanks,

 - Joel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Alexandre Courbot]

On Tue Nov 4, 2025 at 9:58 AM JST, Joel Fernandes wrote:
> On Sat, Nov 01, 2025 at 12:51:32PM +0900, Alexandre Courbot wrote:
>> Hi Joel,
>> 
>> On Fri Oct 31, 2025 at 4:06 AM JST, Joel Fernandes wrote:
>> <snip>
>> > diff --git a/rust/kernel/clist.rs b/rust/kernel/clist.rs
>> > new file mode 100644
>> > index 000000000000..e6a46974b1ba
>> > --- /dev/null
>> > +++ b/rust/kernel/clist.rs
>> > @@ -0,0 +1,296 @@
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +
>> > +//! List processing module for C list_head linked lists.
>> > +//!
>> > +//! C header: [`include/linux/list.h`](srctree/include/linux/list.h)
>> > +//!
>> > +//! Provides a safe interface for iterating over C's intrusive `list_head` structures.
>> > +//! At the moment, supports only read-only iteration.
>> > +//!
>> > +//! # Examples
>> > +//!
>> > +//! ```ignore
>> 
>> Examples pull double-duty as unit tests, and making them build and run
>> ensures that they never fall out-of-date as the code evolves. Please
>> make sure that they can be built and run. Supporting code that you don't
>> want to show in the doc can be put behind `#`.
>
> Sure, the reason I didn't do it was there are a couple of challenges:
>
> 1. For clist, there is a "C language" component" as well in the
> sample, as these are lists coming from C. I am not sure how to do that as a
> doc example, I might have to emulate a C struct containing a list_head in
> Rust itself. Is that something we're Ok with? After all the purpose of a
> sample, is to show how this could be used to interface with lists coming from
> actual C code.

Ah, that's a very valid point.

From the point of view of the documentation reader and the test itself,
I guess it's ok if the C struct is constructed manually from the
bindings as that part won't appear in the generated doc if you put it
behind `#`. So it will render just fine.

What I'm more worried about is that it might be a PITA to write. :/ But
maybe the core folks have an example for how this could be done cleanly
and in a reusable way (i.e. we don't want to duplicate the dummy list
creation code for every example).

>
> 2. For DRM buddy, #1 is not an issue, however CONFIG_DRM_BUDDY has to be
> enabled for the test to work. I have to figure out how to make a doc test be
> kernel CONFIG dependent. What if the CONFIG required is disabled, is there a
> standard way to make doc tests not fail for features that are disabled? Are the
> doc tests skipped in such a situation? Just something I have to figure out.
> Perhaps wrapping it is #cfg is sufficient.

Tests are not expected to run if the config option of the feature they
test is not enabled - how could they anyway. :) So this part is working
as intended I think.

<snip>
>> > +/// [`Clist`] can have its ownership transferred between threads ([`Send`]) if `T: Send`.
>> > +/// But its never [`Sync`] - concurrent Rust threads with `&Clist` could call C FFI unsafely.
>> > +/// For concurrent access, wrap in a `Mutex` or other synchronization primitive.
>> > +///
>> > +/// # Examples
>> > +///
>> > +/// ```ignore
>> > +/// use kernel::clist::Clist;
>> > +///
>> > +/// // C code provides the populated list_head.
>> > +/// let list = unsafe { Clist::<Item>::new(c_list_head) };
>> > +///
>> > +/// // Iterate over items.
>> > +/// for item in list.iter() {
>> > +///     // Process item.
>> > +/// }
>> > +/// ```
>> > +pub struct Clist<T: FromListHead> {
>> > +    head: *mut bindings::list_head,
>> > +    _phantom: PhantomData<T>,
>> > +}
>> > +
>> > +// SAFETY: Safe to send Clist if T is Send (pointer moves, C data stays in place).
>> > +unsafe impl<T: FromListHead + Send> Send for Clist<T> {}
>> > +
>> > +impl<T: FromListHead> Clist<T> {
>> > +    /// Wrap an existing C list_head pointer without taking ownership.
>> > +    ///
>> > +    /// # Safety
>> > +    ///
>> > +    /// Caller must ensure:
>> > +    /// - `head` points to a valid, initialized list_head.
>> > +    /// - `head` remains valid for the lifetime of the returned [`Clist`].
>> > +    /// - The list is not modified by C code while [`Clist`] exists. Caller must
>> > +    ///   implement mutual exclusion algorithms if required, to coordinate with C.
>> > +    /// - Caller is responsible for requesting or working with C to free `head` if needed.
>> > +    pub unsafe fn new(head: *mut bindings::list_head) -> Self {
>> > +        // SAFETY: Caller ensures head is valid and initialized
>> > +        Self {
>> > +            head,
>> > +            _phantom: PhantomData,
>> > +        }
>> > +    }
>> 
>> I am wondering whether `CList` serves an actual purpose beyond providing
>> ` CListIter` to iterate on... Would it make sense to merge both types
>> into a single one that implements `Iterator`?
>> 
>
> It would force us to store iteration state into `Clist`, I think for that
> reason it would be great to keep them separate IMO.

My comment was more an intuition than a strongly held opinion, so please
use your judgment as you perform the redesign. :) I.e. if it ends up
that one type collapses completely and ends up being a almost empty,
maybe that means we don't need it at all in the end.

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Miguel Ojeda]

On Tue, Nov 4, 2025 at 2:42 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> What I'm more worried about is that it might be a PITA to write. :/ But
> maybe the core folks have an example for how this could be done cleanly
> and in a reusable way (i.e. we don't want to duplicate the dummy list
> creation code for every example).

Using a shared module/file may be good enough, as long as the `#[path
= ...] mod ...;` or `include!(...)` is hidden with `#`, i.e. as long
as the user does not need to see that to understand the example.

But, yeah, we have already a few places in the tree with fake `mod
bindings` for doctests and things like that.

Cc'ing Guillaume in case there is a better way to do this. The "have
something applied to several parts of docs" has come up before for
references too (the "external references map" I proposed).

In any case, even if the example does not run, it is still way better
to have it at least build instead of completely ignored, because that
means it will not become stale.

Cheers,
Miguel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Guillaume Gomez]

You can use `cfg(doc)` and `cfg(doctest)` to only include parts of the
docs when running doctests (if that's what this is about).

Le mar. 4 nov. 2025 à 15:07, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> a écrit :
>
> On Tue, Nov 4, 2025 at 2:42 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
> >
> > What I'm more worried about is that it might be a PITA to write. :/ But
> > maybe the core folks have an example for how this could be done cleanly
> > and in a reusable way (i.e. we don't want to duplicate the dummy list
> > creation code for every example).
>
> Using a shared module/file may be good enough, as long as the `#[path
> = ...] mod ...;` or `include!(...)` is hidden with `#`, i.e. as long
> as the user does not need to see that to understand the example.
>
> But, yeah, we have already a few places in the tree with fake `mod
> bindings` for doctests and things like that.
>
> Cc'ing Guillaume in case there is a better way to do this. The "have
> something applied to several parts of docs" has come up before for
> references too (the "external references map" I proposed).
>
> In any case, even if the example does not run, it is still way better
> to have it at least build instead of completely ignored, because that
> means it will not become stale.
>
> Cheers,
> Miguel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Miguel Ojeda]

On Tue, Nov 4, 2025 at 3:35 PM Guillaume Gomez
<guillaume1.gomez@gmail.com> wrote:
>
> You can use `cfg(doc)` and `cfg(doctest)` to only include parts of the
> docs when running doctests (if that's what this is about).

Thanks for the quick reply!

I think this is more about having some code essentially "prefixed" to
certain doctests without having to repeat it in every one. Or, more
generally, to provide custom "environments" for certain doctests,
including perhaps a custom prelude and things like that.

So one way would be writing a `mod` (perhaps with a `path` attribute)
or an `include` to manually do that. Or perhaps having an auxiliary
crate or similar that contains those mods/files (that probably only
gets built when the KUnit doctests are enabled).

Orthogonally, the script that generates the doctests could perhaps
help to automate some of that. For instance, we could have a way to
specify an "environment" for a given Rust file or Rust `mod` or
similar, and then every doctests would have the code prefixed to them.
But I prefer to wait until we have real users of this and the new JSON
generation.

Using `cfg(doctest)` in some code in the `kernel` crate wouldn't work,
unless I am missing something, because we wouldn't want to have a
"parallel `kernel` crate" in the same build that only the doctests
would use.

Cheers,
Miguel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Miguel Ojeda]

On Tue, Nov 4, 2025 at 7:35 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Orthogonally, the script that generates the doctests could perhaps
> help to automate some of that. For instance, we could have a way to
> specify an "environment" for a given Rust file or Rust `mod` or
> similar, and then every doctests would have the code prefixed to them.

I guess this could probably best generalized as "tagging" doctests
with custom tags that `rustdoc` just forwards in the generated JSON.

Something like:

    /// ```tag:foo,tag:bar

would give us a:

    "tags": ["foo", "bar"]

in the JSON. Then a custom generator like the one we have could do
whatever it needs with it, including prepending code or other things.

Now, I see there is already an `unknown` field in the attributes which
already give us the unrecognized ones, which is great and we could
potentially use that.

However, should there be a particular way/namespace we should create
our custom tags so that we don't conflict in the future with `rustdoc`
ones?

I have added it to the usual list:

    https://github.com/Rust-for-Linux/linux/issues/350

(There is also the question about supporting the old non-JSON way for
things like this, but I am ignoring that for now)

Cheers,
Miguel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Guillaume Gomez]

You can add your own tags in the doctests, and with our patch waiting
to use the new rustdoc doctests extraction, it should be pretty easy
to plug such a feature into it. We can check it together if you want
when the patch is merged to see if we already have everything needed
or if I need to add more things on rustdoc side.

Le mar. 4 nov. 2025 à 20:06, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> a écrit :
>
> On Tue, Nov 4, 2025 at 7:35 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > Orthogonally, the script that generates the doctests could perhaps
> > help to automate some of that. For instance, we could have a way to
> > specify an "environment" for a given Rust file or Rust `mod` or
> > similar, and then every doctests would have the code prefixed to them.
>
> I guess this could probably best generalized as "tagging" doctests
> with custom tags that `rustdoc` just forwards in the generated JSON.
>
> Something like:
>
>     /// ```tag:foo,tag:bar
>
> would give us a:
>
>     "tags": ["foo", "bar"]
>
> in the JSON. Then a custom generator like the one we have could do
> whatever it needs with it, including prepending code or other things.
>
> Now, I see there is already an `unknown` field in the attributes which
> already give us the unrecognized ones, which is great and we could
> potentially use that.
>
> However, should there be a particular way/namespace we should create
> our custom tags so that we don't conflict in the future with `rustdoc`
> ones?
>
> I have added it to the usual list:
>
>     https://github.com/Rust-for-Linux/linux/issues/350
>
> (There is also the question about supporting the old non-JSON way for
> things like this, but I am ignoring that for now)
>
> Cheers,
> Miguel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Miguel Ojeda]

On Tue, Nov 4, 2025 at 1:58 AM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> Perhaps wrapping it is #cfg is sufficient.

`cfg` attributes and the `cfg!` macro should work in doctests -- we
have already a few instances, e.g. this hidden one:

    /// ```
    /// # #![cfg(CONFIG_OF)]
    /// use kernel::clk::Hertz;

Cheers,
Miguel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Joel Fernandes]

On 11/4/2025 8:52 AM, Miguel Ojeda wrote:
> On Tue, Nov 4, 2025 at 1:58 AM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>> Perhaps wrapping it is #cfg is sufficient.
> 
> `cfg` attributes and the `cfg!` macro should work in doctests -- we
> have already a few instances, e.g. this hidden one:
> 
>     /// ```
>     /// # #![cfg(CONFIG_OF)]
>     /// use kernel::clk::Hertz;

Thanks for the example! I will guard it accordingly in my next posting.

 - Joel

Re: [PATCH RFC 1/4] rust: clist: Add abstraction for iterating over C linked lists

[Danilo Krummrich]

On 11/1/25 4:51 AM, Alexandre Courbot wrote:
> I am wondering whether `CList` serves an actual purpose beyond providing
> ` CListIter` to iterate on... Would it make sense to merge both types
> into a single one that implements `Iterator`?

I think eventually we will have a bunch of iterator types, e.g for
list_for_each_entry_{safe,reverse,continue}() etc. (see also [1]).

Hence, CList has to provide a couple of methods providing different iterator types.

[1] https://lore.kernel.org/lkml/DDVYV1VT441A.11L5C11F8R7C9@kernel.org/

[PATCH RFC 2/4] samples: rust: Add sample demonstrating C linked list iteration

[Joel Fernandes]

Demonstrates usage of the clist module for iterating over
C-managed linked lists. C code creates and populates the list,
Rust code performs safe iteration using the clist abstraction.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 samples/rust/Kconfig            |  11 +++
 samples/rust/Makefile           |   2 +
 samples/rust/rust_clist_c.c     |  54 +++++++++++++
 samples/rust/rust_clist_main.rs | 138 ++++++++++++++++++++++++++++++++
 4 files changed, 205 insertions(+)
 create mode 100644 samples/rust/rust_clist_c.c
 create mode 100644 samples/rust/rust_clist_main.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index c1cc787a9add..b45631e2593c 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -10,6 +10,17 @@ menuconfig SAMPLES_RUST
 
 if SAMPLES_RUST
 
+config SAMPLE_RUST_CLIST
+	tristate "C Linked List sample"
+	help
+	  This option builds the Rust CList sample demonstrating
+	  the clist module for working with C list_head structures.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_clist.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_CONFIGFS
 	tristate "Configfs sample"
 	depends on CONFIGFS_FS
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index cf8422f8f219..f8899c0df762 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 ccflags-y += -I$(src)				# needed for trace events
 
+obj-$(CONFIG_SAMPLE_RUST_CLIST)			+= rust_clist.o
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
@@ -14,6 +15,7 @@ obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_AUXILIARY)	+= rust_driver_auxiliary.o
 obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
 
+rust_clist-y := rust_clist_main.o rust_clist_c.o
 rust_print-y := rust_print_main.o rust_print_events.o
 
 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
diff --git a/samples/rust/rust_clist_c.c b/samples/rust/rust_clist_c.c
new file mode 100644
index 000000000000..7a8f5e6c642a
--- /dev/null
+++ b/samples/rust/rust_clist_c.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/list.h>
+#include <linux/slab.h>
+
+/* Sample item with embedded C list_head */
+struct sample_item_c {
+	int value;
+	struct list_head link;
+};
+
+/* Create a list_head and populate it with items */
+struct list_head *clist_sample_create(int count)
+{
+	struct list_head *head;
+	int i;
+
+	head = kmalloc(sizeof(*head), GFP_KERNEL);
+	if (!head)
+		return NULL;
+
+	INIT_LIST_HEAD(head);
+
+	/* Populate with items */
+	for (i = 0; i < count; i++) {
+		struct sample_item_c *item = kmalloc(sizeof(*item), GFP_KERNEL);
+
+		if (!item)
+			continue;
+
+		item->value = i * 10;
+		INIT_LIST_HEAD(&item->link);
+		list_add_tail(&item->link, head);
+	}
+
+	return head;
+}
+
+/* Free the list_head and all items */
+void clist_sample_free(struct list_head *head)
+{
+	struct sample_item_c *item, *tmp;
+
+	if (!head)
+		return;
+
+	/* Free all items in the list */
+	list_for_each_entry_safe(item, tmp, head, link) {
+		list_del(&item->link);
+		kfree(item);
+	}
+
+	kfree(head);
+}
diff --git a/samples/rust/rust_clist_main.rs b/samples/rust/rust_clist_main.rs
new file mode 100644
index 000000000000..6600b0c79558
--- /dev/null
+++ b/samples/rust/rust_clist_main.rs
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Sample for Rust code interfacing with C linked lists (list_head).
+//!
+//! This sample demonstrates iteration of a C-managed linked list using the [`clist`] module.
+//! C code creates and populates the list, Rust code performs iteration.
+
+use core::ptr::NonNull;
+use kernel::{
+    bindings,
+    clist,
+    container_of,
+    prelude::*, //
+};
+
+module! {
+    type: RustClistModule,
+    name: "rust_clist",
+    authors: ["Joel Fernandes"],
+    description: "Rust Clist sample",
+    license: "GPL",
+}
+
+// FFI declarations for C functions
+extern "C" {
+    fn clist_sample_create(count: i32) -> *mut bindings::list_head;
+    fn clist_sample_free(head: *mut bindings::list_head);
+}
+
+/// Sample item with embedded C list_head (This would typically be a C struct).
+#[repr(C)]
+struct SampleItemC {
+    value: i32,
+    link: bindings::list_head,
+}
+
+/// Rust wrapper for SampleItemC object pointer allocated on the C side.
+///
+/// # Invariants
+///
+/// `ptr` points to a valid [`SampleItemC`] with an initialized embedded `list_head`.
+struct SampleItem {
+    ptr: NonNull<SampleItemC>,
+}
+
+impl clist::FromListHead for SampleItem {
+    unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
+        // SAFETY: Caller ensures link points to a valid, initialized list_head.
+        unsafe {
+            let item_ptr = container_of!(link, SampleItemC, link) as *mut _;
+            SampleItem {
+                ptr: NonNull::new_unchecked(item_ptr),
+            }
+        }
+    }
+}
+
+impl SampleItem {
+    /// Get the value from the sample item.
+    fn value(&self) -> i32 {
+        // SAFETY: self.ptr is valid as per the SampleItem invariants.
+        unsafe { (*self.ptr.as_ptr()).value }
+    }
+}
+
+/// Clist struct - holds the C list_head and manages its lifecycle.
+#[repr(C)]
+struct RustClist {
+    list_head: NonNull<bindings::list_head>,
+}
+
+// SAFETY: RustClist can be sent between threads since it only holds a pointer
+// which can be accessed from any thread with proper synchronization.
+unsafe impl Send for RustClist {}
+
+impl RustClist {
+    /// Create a container for a pointer to a C-allocated list_head.
+    fn new(list_head: *mut bindings::list_head) -> Self {
+        // SAFETY: Caller ensures list_head is a valid, non-null pointer.
+        Self {
+            list_head: unsafe { NonNull::new_unchecked(list_head) },
+        }
+    }
+
+    /// Demonstrate iteration over the list.
+    fn do_iteration(&self) {
+        // Wrap the C list_head with a Rust [`Clist`].
+        // SAFETY: list_head is a valid, initialized, populated list_head.
+        let list = unsafe { clist::Clist::<SampleItem>::new(self.list_head.as_ptr()) };
+        pr_info!("Created C list with items, is_empty: {}\n", list.is_empty());
+
+        // Iterate over the list.
+        pr_info!("Iterating over C list from Rust:\n");
+        for item in list.iter() {
+            pr_info!("  Item value: {}\n", item.value());
+        }
+
+        pr_info!("Iteration complete\n");
+    }
+}
+
+impl Drop for RustClist {
+    fn drop(&mut self) {
+        // Free the list and all items using C FFI.
+        // SAFETY: list_head was allocated by clist_sample_create.
+        // C side handles freeing both the list_head and all items.
+        unsafe {
+            clist_sample_free(self.list_head.as_ptr());
+        }
+    }
+}
+
+struct RustClistModule;
+
+impl kernel::Module for RustClistModule {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        pr_info!("Rust Clist sample (init)\n");
+
+        // C creates and populates a list_head with items.
+        // SAFETY: clist_sample_create allocates, initializes, and populates a list.
+        let c_list_head = unsafe { clist_sample_create(6) };
+        if c_list_head.is_null() {
+            pr_err!("Failed to create and populate C list\n");
+            return Err(ENOMEM);
+        }
+
+        // Create the list container (separate from module).
+        let rust_clist = RustClist::new(c_list_head);
+
+        // Demonstrate list operations.
+        rust_clist.do_iteration();
+
+        // rust_clist is dropped here, which frees the C list via Drop impl.
+        pr_info!("Rust Clist sample (exit)\n");
+
+        Ok(RustClistModule {})
+    }
+}
-- 
2.34.1

Re: [PATCH RFC 2/4] samples: rust: Add sample demonstrating C linked list iteration

[Danilo Krummrich]

On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
> Demonstrates usage of the clist module for iterating over
> C-managed linked lists. C code creates and populates the list,
> Rust code performs safe iteration using the clist abstraction.

I don't think a sample module is the correct choice for this. It makes it look a
bit like this is an API intended for drivers. I think kunit tests might be a
better fit.

Re: [PATCH RFC 2/4] samples: rust: Add sample demonstrating C linked list iteration

[Joel Fernandes]

On 10/30/2025 5:15 PM, Danilo Krummrich wrote:
> On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
>> Demonstrates usage of the clist module for iterating over
>> C-managed linked lists. C code creates and populates the list,
>> Rust code performs safe iteration using the clist abstraction.
> 
> I don't think a sample module is the correct choice for this. It makes it look a
> bit like this is an API intended for drivers. I think kunit tests might be a
> better fit.

Sure, kunit tests indeed sound better. I will convert it over (this and the
other one). Thanks!

 - Joel

Re: [PATCH RFC 2/4] samples: rust: Add sample demonstrating C linked list iteration

[Alexandre Courbot]

On Fri Oct 31, 2025 at 6:15 AM JST, Danilo Krummrich wrote:
> On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
>> Demonstrates usage of the clist module for iterating over
>> C-managed linked lists. C code creates and populates the list,
>> Rust code performs safe iteration using the clist abstraction.
>
> I don't think a sample module is the correct choice for this. It makes it look a
> bit like this is an API intended for drivers. I think kunit tests might be a
> better fit.

Yup, we can probably move this into the doctest examples and have them
serve as examples as well.

Re: [PATCH RFC 2/4] samples: rust: Add sample demonstrating C linked list iteration

[Miguel Ojeda]

On Sat, Nov 1, 2025 at 4:53 AM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Yup, we can probably move this into the doctest examples and have them
> serve as examples as well.

 +1, in general, one should consider whether a test makes sense as an
example first.

Cheers,
Miguel

[PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Joel Fernandes]

Add safe Rust abstractions over the Linux kernel's DRM buddy
allocator for physical memory management. The DRM buddy allocator
implements a binary buddy system for useful for GPU physical memory
allocation. nova-core will use it for physical memory allocation.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 rust/bindings/bindings_helper.h |  11 +
 rust/helpers/drm.c              |  23 +-
 rust/kernel/drm/buddy.rs        | 357 ++++++++++++++++++++++++++++++++
 rust/kernel/drm/mod.rs          |   1 +
 4 files changed, 391 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/drm/buddy.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 6b973589a546..747d4c7ef935 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -29,6 +29,7 @@
 #include <linux/hrtimer_types.h>
 
 #include <linux/acpi.h>
+#include <drm/drm_buddy.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
@@ -112,6 +113,16 @@ const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC = XA_FLAGS_ALLOC;
 const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC1 = XA_FLAGS_ALLOC1;
 const vm_flags_t RUST_CONST_HELPER_VM_MERGEABLE = VM_MERGEABLE;
 
+#if IS_ENABLED(CONFIG_DRM_BUDDY)
+const unsigned long RUST_CONST_HELPER_DRM_BUDDY_RANGE_ALLOCATION = DRM_BUDDY_RANGE_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_DRM_BUDDY_TOPDOWN_ALLOCATION = DRM_BUDDY_TOPDOWN_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_DRM_BUDDY_CONTIGUOUS_ALLOCATION =
+								DRM_BUDDY_CONTIGUOUS_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_DRM_BUDDY_CLEAR_ALLOCATION = DRM_BUDDY_CLEAR_ALLOCATION;
+const unsigned long RUST_CONST_HELPER_DRM_BUDDY_CLEARED = DRM_BUDDY_CLEARED;
+const unsigned long RUST_CONST_HELPER_DRM_BUDDY_TRIM_DISABLE = DRM_BUDDY_TRIM_DISABLE;
+#endif
+
 #if IS_ENABLED(CONFIG_ANDROID_BINDER_IPC_RUST)
 #include "../../drivers/android/binder/rust_binder.h"
 #include "../../drivers/android/binder/rust_binder_events.h"
diff --git a/rust/helpers/drm.c b/rust/helpers/drm.c
index 450b406c6f27..bd8748ade3f5 100644
--- a/rust/helpers/drm.c
+++ b/rust/helpers/drm.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <drm/drm_buddy.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_vma_manager.h>
 
@@ -20,4 +21,24 @@ __u64 rust_helper_drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
 	return drm_vma_node_offset_addr(node);
 }
 
-#endif
+#ifdef CONFIG_DRM_BUDDY
+
+u64 rust_helper_drm_buddy_block_offset(const struct drm_buddy_block *block)
+{
+	return drm_buddy_block_offset(block);
+}
+
+unsigned int rust_helper_drm_buddy_block_order(struct drm_buddy_block *block)
+{
+	return drm_buddy_block_order(block);
+}
+
+u64 rust_helper_drm_buddy_block_size(struct drm_buddy *mm,
+				     struct drm_buddy_block *block)
+{
+	return drm_buddy_block_size(mm, block);
+}
+
+#endif /* CONFIG_DRM_BUDDY */
+
+#endif /* CONFIG_DRM */
diff --git a/rust/kernel/drm/buddy.rs b/rust/kernel/drm/buddy.rs
new file mode 100644
index 000000000000..b1cd23f81838
--- /dev/null
+++ b/rust/kernel/drm/buddy.rs
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! DRM buddy allocator bindings.
+//!
+//! C header: [`include/drm/drm_buddy.h`](srctree/include/drm/drm_buddy.h)
+//!
+//! This module provides Rust abstractions over the Linux kernel's DRM buddy
+//! allocator, which implements a binary buddy memory allocation system.
+//!
+//! The buddy allocator manages a contiguous address space and allocates blocks
+//! in power-of-two sizes. It's commonly used for physical memory management.
+//!
+//! # Examples
+//!
+//! ```ignore
+//! use kernel::{
+//!     drm::buddy::{BuddyFlags, DrmBuddy},
+//!     prelude::*,
+//!     sizes::*, //
+//! };
+//!
+//! let buddy = DrmBuddy::new(SZ_1G, SZ_4K)?;
+//! let allocated = buddy.alloc_blocks(
+//!     0, 0, SZ_16M, SZ_4K,
+//!     BuddyFlags::RANGE_ALLOCATION,
+//!     GFP_KERNEL,
+//! )?;
+//!
+//! for block in &allocated {
+//!     // Use block.
+//! }
+//! // Blocks are automatically freed when `allocated` goes out of scope.
+//! ```
+
+use crate::{
+    alloc::Flags,
+    bindings,
+    clist,
+    container_of,
+    error::{
+        to_result,
+        Result, //
+    },
+    prelude::KBox,
+    types::Opaque, //
+};
+use core::ptr::NonNull;
+
+/// Flags for DRM buddy allocator operations.
+///
+/// These flags control the allocation behavior of the buddy allocator.
+#[derive(Clone, Copy, PartialEq)]
+pub struct BuddyFlags(u64);
+
+impl BuddyFlags {
+    /// Range-based allocation from start to end addresses.
+    pub const RANGE_ALLOCATION: BuddyFlags =
+        BuddyFlags(bindings::DRM_BUDDY_RANGE_ALLOCATION as u64);
+
+    /// Allocate from top of address space downward.
+    pub const TOPDOWN_ALLOCATION: BuddyFlags =
+        BuddyFlags(bindings::DRM_BUDDY_TOPDOWN_ALLOCATION as u64);
+
+    /// Allocate physically contiguous blocks.
+    pub const CONTIGUOUS_ALLOCATION: BuddyFlags =
+        BuddyFlags(bindings::DRM_BUDDY_CONTIGUOUS_ALLOCATION as u64);
+
+    /// Clear allocated blocks (zero them).
+    pub const CLEAR_ALLOCATION: BuddyFlags =
+        BuddyFlags(bindings::DRM_BUDDY_CLEAR_ALLOCATION as u64);
+
+    /// Block has been cleared - internal flag.
+    pub const CLEARED: BuddyFlags = BuddyFlags(bindings::DRM_BUDDY_CLEARED as u64);
+
+    /// Disable trimming of partially used blocks.
+    pub const TRIM_DISABLE: BuddyFlags = BuddyFlags(bindings::DRM_BUDDY_TRIM_DISABLE as u64);
+
+    /// Get raw value for FFI.
+    pub(crate) fn as_raw(self) -> u64 {
+        self.0
+    }
+}
+
+impl core::ops::BitOr for BuddyFlags {
+    type Output = Self;
+
+    fn bitor(self, rhs: Self) -> Self::Output {
+        Self(self.0 | rhs.0)
+    }
+}
+
+/// DRM buddy allocator instance.
+///
+/// This structure wraps the C `drm_buddy` allocator.
+///
+/// # Safety
+///
+/// Not thread-safe. Concurrent alloc/free operations require external
+/// synchronization (e.g., wrapping in `Arc<Mutex<DrmBuddy>>`).
+///
+/// # Invariants
+///
+/// - `mm` is initialized via `drm_buddy_init()` and remains valid until Drop.
+pub struct DrmBuddy {
+    mm: Opaque<bindings::drm_buddy>,
+}
+
+impl DrmBuddy {
+    /// Create a new buddy allocator.
+    ///
+    /// Creates a buddy allocator that manages a contiguous address space of the given
+    /// size, with the specified minimum allocation unit (chunk_size must be at least 4KB).
+    ///
+    /// # Examples
+    ///
+    /// See the complete example in the documentation comments for [`AllocatedBlocks`].
+    pub fn new(size: usize, chunk_size: usize) -> Result<Self> {
+        // Create buddy allocator with zeroed memory.
+        let buddy = Self {
+            mm: Opaque::zeroed(),
+        };
+
+        // Initialize the C buddy structure.
+        // SAFETY: buddy.mm points to valid, zeroed memory.
+        unsafe {
+            to_result(bindings::drm_buddy_init(
+                buddy.mm.get(),
+                size as u64,
+                chunk_size as u64,
+            ))?;
+        }
+
+        Ok(buddy)
+    }
+
+    /// Get a raw pointer to the underlying C drm_buddy structure.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure the returned pointer is not used after this
+    /// structure is dropped.
+    pub unsafe fn as_raw(&self) -> *mut bindings::drm_buddy {
+        self.mm.get()
+    }
+
+    /// Get the chunk size (minimum allocation unit).
+    pub fn chunk_size(&self) -> u64 {
+        // SAFETY: mm is initialized and valid per struct invariant.
+        unsafe { (*self.as_raw()).chunk_size }
+    }
+
+    /// Get the total managed size.
+    pub fn size(&self) -> u64 {
+        // SAFETY: mm is initialized and valid per struct invariant.
+        unsafe { (*self.as_raw()).size }
+    }
+
+    /// Get the available (free) memory.
+    pub fn avail(&self) -> u64 {
+        // SAFETY: mm is initialized and valid per struct invariant.
+        unsafe { (*self.as_raw()).avail }
+    }
+
+    /// Allocate blocks from the buddy allocator.
+    ///
+    /// Returns an [`AllocatedBlocks`] structure that owns the allocated blocks and automatically
+    /// frees them when dropped. Allocation of `list_head` uses the `gfp` flags passed.
+    pub fn alloc_blocks(
+        &self,
+        start: usize,
+        end: usize,
+        size: usize,
+        min_block_size: usize,
+        flags: BuddyFlags,
+        gfp: Flags,
+    ) -> Result<AllocatedBlocks<'_>> {
+        // Allocate list_head on the heap.
+        let mut list_head = KBox::new(bindings::list_head::default(), gfp)?;
+
+        // SAFETY: list_head is valid and heap-allocated.
+        unsafe {
+            bindings::INIT_LIST_HEAD(&mut *list_head as *mut _);
+        }
+
+        // SAFETY: mm is a valid DrmBuddy object per the type's invariants.
+        unsafe {
+            to_result(bindings::drm_buddy_alloc_blocks(
+                self.as_raw(),
+                start as u64,
+                end as u64,
+                size as u64,
+                min_block_size as u64,
+                &mut *list_head as *mut _,
+                flags.as_raw() as usize,
+            ))?;
+        }
+
+        // `list_head` is now the head of a list that contains allocated blocks
+        // from C code. The allocated blocks will be automatically freed when
+        // `AllocatedBlocks` is dropped.
+        Ok(AllocatedBlocks {
+            list_head,
+            buddy: self,
+        })
+    }
+}
+
+impl Drop for DrmBuddy {
+    fn drop(&mut self) {
+        // SAFETY: self.mm is initialized and valid. drm_buddy_fini properly
+        // cleans up all resources. This is called exactly once during Drop.
+        unsafe {
+            bindings::drm_buddy_fini(self.as_raw());
+        }
+    }
+}
+
+// SAFETY: DrmBuddy can be sent between threads. Caller is responsible for
+// ensuring thread-safe access if needed (e.g., via Mutex).
+unsafe impl Send for DrmBuddy {}
+
+/// Allocated blocks from the buddy allocator with automatic cleanup.
+///
+/// This structure owns a list of allocated blocks and ensures they are
+/// automatically freed when dropped. Blocks may be iterated over and are
+/// read-only after allocation (iteration via [`IntoIterator`] and
+/// automatic cleanup via [`Drop`] only). To share across threads, wrap
+/// in `Arc<AllocatedBlocks>`. Rust owns the head list head of the
+/// allocated blocks; C allocates blocks and links them to the head
+/// list head. Clean up of the allocated blocks is handled by C code.
+///
+/// # Invariants
+///
+/// - `list_head` is an owned, valid, initialized list_head.
+/// - `buddy` points to a valid, initialized [`DrmBuddy`].
+pub struct AllocatedBlocks<'a> {
+    list_head: KBox<bindings::list_head>,
+    buddy: &'a DrmBuddy,
+}
+
+impl Drop for AllocatedBlocks<'_> {
+    fn drop(&mut self) {
+        // Free all blocks automatically when dropped.
+        // SAFETY: list_head is a valid list of blocks per the type's invariants.
+        unsafe {
+            bindings::drm_buddy_free_list(self.buddy.as_raw(), &mut *self.list_head as *mut _, 0);
+        }
+    }
+}
+
+impl<'a> AllocatedBlocks<'a> {
+    /// Check if the block list is empty.
+    pub fn is_empty(&self) -> bool {
+        // SAFETY: list_head is a valid list of blocks per the type's invariants.
+        unsafe { clist::list_empty(&*self.list_head as *const _) }
+    }
+
+    /// Iterate over allocated blocks.
+    pub fn iter(&self) -> clist::ClistIter<'_, Block> {
+        // SAFETY: list_head is a valid list of blocks per the type's invariants.
+        clist::iter_list_head::<Block>(&*self.list_head)
+    }
+}
+
+/// Iteration support for allocated blocks.
+///
+/// # Examples
+///
+/// ```ignore
+/// for block in &allocated_blocks {
+///     // Use block.
+/// }
+/// ```
+impl<'a> IntoIterator for &'a AllocatedBlocks<'_> {
+    type Item = Block;
+    type IntoIter = clist::ClistIter<'a, Block>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.iter()
+    }
+}
+
+/// A DRM buddy block.
+///
+/// Wraps a pointer to a C `drm_buddy_block` structure. This is returned
+/// from allocation operations and used to free blocks.
+///
+/// # Invariants
+///
+/// `drm_buddy_block_ptr` points to a valid `drm_buddy_block` managed by the buddy allocator.
+pub struct Block {
+    drm_buddy_block_ptr: NonNull<bindings::drm_buddy_block>,
+}
+
+impl Block {
+    /// Get the block's offset in the address space.
+    pub fn offset(&self) -> u64 {
+        // SAFETY: drm_buddy_block_ptr is valid per the type's invariants.
+        unsafe { bindings::drm_buddy_block_offset(self.drm_buddy_block_ptr.as_ptr()) }
+    }
+
+    /// Get the block order (size = chunk_size << order).
+    pub fn order(&self) -> u32 {
+        // SAFETY: drm_buddy_block_ptr is valid per the type's invariants.
+        unsafe { bindings::drm_buddy_block_order(self.drm_buddy_block_ptr.as_ptr()) }
+    }
+
+    /// Get the block's size in bytes.
+    ///
+    /// Requires the buddy allocator to calculate size from order.
+    pub fn size(&self, buddy: &DrmBuddy) -> u64 {
+        // SAFETY: Both pointers are valid per the type's invariants.
+        unsafe { bindings::drm_buddy_block_size(buddy.as_raw(), self.drm_buddy_block_ptr.as_ptr()) }
+    }
+
+    /// Get a raw pointer to the underlying C block.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure the pointer is not used after the block is freed.
+    pub unsafe fn as_ptr(&self) -> *mut bindings::drm_buddy_block {
+        self.drm_buddy_block_ptr.as_ptr()
+    }
+}
+
+impl clist::FromListHead for Block {
+    unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
+        // SAFETY: link points to a valid list_head embedded in drm_buddy_block.
+        // The container_of macro calculates the containing struct pointer.
+        // We need to account for the union field __bindgen_anon_1.link.
+        //
+        // The link is embedded in a union within drm_buddy_block:
+        //     struct drm_buddy_block {
+        //         [...]
+        //         union {
+        //             struct rb_node rb;
+        //             struct list_head link;
+        //         };
+        //     }
+        //
+        // This is why we perform a double container_of calculation: first to get
+        // to the union, then to get to the containing drm_buddy_block.
+        unsafe {
+            // First get to the union.
+            let union_ptr = container_of!(link, bindings::drm_buddy_block__bindgen_ty_1, link);
+            // Then get to the containing drm_buddy_block.
+            let block_ptr =
+                container_of!(union_ptr, bindings::drm_buddy_block, __bindgen_anon_1) as *mut _;
+            Block {
+                drm_buddy_block_ptr: NonNull::new_unchecked(block_ptr),
+            }
+        }
+    }
+}
+
+// SAFETY: Block is just a pointer wrapper and can be safely sent between threads.
+unsafe impl Send for Block {}
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index 1b82b6945edf..ee173729eac5 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -2,6 +2,7 @@
 
 //! DRM subsystem abstractions.
 
+pub mod buddy;
 pub mod device;
 pub mod driver;
 pub mod file;
-- 
2.34.1

Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Danilo Krummrich]

On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
> +    ///
> +    /// Returns an [`AllocatedBlocks`] structure that owns the allocated blocks and automatically
> +    /// frees them when dropped. Allocation of `list_head` uses the `gfp` flags passed.
> +    pub fn alloc_blocks(
> +        &self,
> +        start: usize,
> +        end: usize,
> +        size: usize,
> +        min_block_size: usize,
> +        flags: BuddyFlags,
> +        gfp: Flags,
> +    ) -> Result<AllocatedBlocks<'_>> {
> +        // Allocate list_head on the heap.
> +        let mut list_head = KBox::new(bindings::list_head::default(), gfp)?;
> +
> +        // SAFETY: list_head is valid and heap-allocated.
> +        unsafe {
> +            bindings::INIT_LIST_HEAD(&mut *list_head as *mut _);
> +        }

Not a full review, but a quick drive-by comment:

bindings::list_head has to be pinned in memory it should be

	let list_head = KBox::pin_init(Opaque::ffi_init(|slot: *mut bindings::list_head| {
	    // SAFETY: `slot` is a valid pointer to uninitialized memory.
	    unsafe { bindings::INIT_LIST_HEAD(slot) };
	}), gfp)?;

if you're doing it by hand, but as mentioned in a previous patch, I think it
would be nice to have a transparent wrapper type, CListHead, for this.

Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Joel Fernandes]

Hi Danilo,

On 10/30/2025 5:27 PM, Danilo Krummrich wrote:
> On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
>> +    ///
>> +    /// Returns an [`AllocatedBlocks`] structure that owns the allocated blocks and automatically
>> +    /// frees them when dropped. Allocation of `list_head` uses the `gfp` flags passed.
>> +    pub fn alloc_blocks(
>> +        &self,
>> +        start: usize,
>> +        end: usize,
>> +        size: usize,
>> +        min_block_size: usize,
>> +        flags: BuddyFlags,
>> +        gfp: Flags,
>> +    ) -> Result<AllocatedBlocks<'_>> {
>> +        // Allocate list_head on the heap.
>> +        let mut list_head = KBox::new(bindings::list_head::default(), gfp)?;
>> +
>> +        // SAFETY: list_head is valid and heap-allocated.
>> +        unsafe {
>> +            bindings::INIT_LIST_HEAD(&mut *list_head as *mut _);
>> +        }
> 
> Not a full review, but a quick drive-by comment:
> 
> bindings::list_head has to be pinned in memory it should be
> 
> 	let list_head = KBox::pin_init(Opaque::ffi_init(|slot: *mut bindings::list_head| {
> 	    // SAFETY: `slot` is a valid pointer to uninitialized memory.
> 	    unsafe { bindings::INIT_LIST_HEAD(slot) };
> 	}), gfp)?;
> 

Sure. I will use pin_init here.

> if you're doing it by hand, but as mentioned in a previous patch, I think it
> would be nice to have a transparent wrapper type, CListHead, for this.
I like your CListHead idea. Let me sketch this out a bit more and see what the
lifetime relationships look like, thanks! - Joel

Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Alice Ryhl]

On Thu, Oct 30, 2025 at 03:06:12PM -0400, Joel Fernandes wrote:
> Add safe Rust abstractions over the Linux kernel's DRM buddy
> allocator for physical memory management. The DRM buddy allocator
> implements a binary buddy system for useful for GPU physical memory
> allocation. nova-core will use it for physical memory allocation.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>  rust/bindings/bindings_helper.h |  11 +
>  rust/helpers/drm.c              |  23 +-
>  rust/kernel/drm/buddy.rs        | 357 ++++++++++++++++++++++++++++++++
>  rust/kernel/drm/mod.rs          |   1 +
>  4 files changed, 391 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/drm/buddy.rs
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 6b973589a546..747d4c7ef935 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -29,6 +29,7 @@
>  #include <linux/hrtimer_types.h>
>  
>  #include <linux/acpi.h>
> +#include <drm/drm_buddy.h>
>  #include <drm/drm_device.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_file.h>
> @@ -112,6 +113,16 @@ const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC = XA_FLAGS_ALLOC;
>  const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC1 = XA_FLAGS_ALLOC1;
>  const vm_flags_t RUST_CONST_HELPER_VM_MERGEABLE = VM_MERGEABLE;
>  
> +#if IS_ENABLED(CONFIG_DRM_BUDDY)
> +const unsigned long RUST_CONST_HELPER_DRM_BUDDY_RANGE_ALLOCATION = DRM_BUDDY_RANGE_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_DRM_BUDDY_TOPDOWN_ALLOCATION = DRM_BUDDY_TOPDOWN_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_DRM_BUDDY_CONTIGUOUS_ALLOCATION =
> +								DRM_BUDDY_CONTIGUOUS_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_DRM_BUDDY_CLEAR_ALLOCATION = DRM_BUDDY_CLEAR_ALLOCATION;
> +const unsigned long RUST_CONST_HELPER_DRM_BUDDY_CLEARED = DRM_BUDDY_CLEARED;
> +const unsigned long RUST_CONST_HELPER_DRM_BUDDY_TRIM_DISABLE = DRM_BUDDY_TRIM_DISABLE;
> +#endif
> +
>  #if IS_ENABLED(CONFIG_ANDROID_BINDER_IPC_RUST)
>  #include "../../drivers/android/binder/rust_binder.h"
>  #include "../../drivers/android/binder/rust_binder_events.h"
> diff --git a/rust/helpers/drm.c b/rust/helpers/drm.c
> index 450b406c6f27..bd8748ade3f5 100644
> --- a/rust/helpers/drm.c
> +++ b/rust/helpers/drm.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +#include <drm/drm_buddy.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_vma_manager.h>
>  
> @@ -20,4 +21,24 @@ __u64 rust_helper_drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
>  	return drm_vma_node_offset_addr(node);
>  }
>  
> -#endif
> +#ifdef CONFIG_DRM_BUDDY
> +
> +u64 rust_helper_drm_buddy_block_offset(const struct drm_buddy_block *block)
> +{
> +	return drm_buddy_block_offset(block);
> +}
> +
> +unsigned int rust_helper_drm_buddy_block_order(struct drm_buddy_block *block)
> +{
> +	return drm_buddy_block_order(block);
> +}
> +
> +u64 rust_helper_drm_buddy_block_size(struct drm_buddy *mm,
> +				     struct drm_buddy_block *block)
> +{
> +	return drm_buddy_block_size(mm, block);
> +}
> +
> +#endif /* CONFIG_DRM_BUDDY */
> +
> +#endif /* CONFIG_DRM */
> diff --git a/rust/kernel/drm/buddy.rs b/rust/kernel/drm/buddy.rs
> new file mode 100644
> index 000000000000..b1cd23f81838
> --- /dev/null
> +++ b/rust/kernel/drm/buddy.rs
> @@ -0,0 +1,357 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! DRM buddy allocator bindings.
> +//!
> +//! C header: [`include/drm/drm_buddy.h`](srctree/include/drm/drm_buddy.h)
> +//!
> +//! This module provides Rust abstractions over the Linux kernel's DRM buddy
> +//! allocator, which implements a binary buddy memory allocation system.
> +//!
> +//! The buddy allocator manages a contiguous address space and allocates blocks
> +//! in power-of-two sizes. It's commonly used for physical memory management.
> +//!
> +//! # Examples
> +//!
> +//! ```ignore
> +//! use kernel::{
> +//!     drm::buddy::{BuddyFlags, DrmBuddy},
> +//!     prelude::*,
> +//!     sizes::*, //
> +//! };
> +//!
> +//! let buddy = DrmBuddy::new(SZ_1G, SZ_4K)?;
> +//! let allocated = buddy.alloc_blocks(
> +//!     0, 0, SZ_16M, SZ_4K,
> +//!     BuddyFlags::RANGE_ALLOCATION,
> +//!     GFP_KERNEL,
> +//! )?;
> +//!
> +//! for block in &allocated {
> +//!     // Use block.
> +//! }
> +//! // Blocks are automatically freed when `allocated` goes out of scope.
> +//! ```
> +
> +use crate::{
> +    alloc::Flags,
> +    bindings,
> +    clist,
> +    container_of,
> +    error::{
> +        to_result,
> +        Result, //
> +    },
> +    prelude::KBox,
> +    types::Opaque, //
> +};
> +use core::ptr::NonNull;
> +
> +/// Flags for DRM buddy allocator operations.
> +///
> +/// These flags control the allocation behavior of the buddy allocator.
> +#[derive(Clone, Copy, PartialEq)]
> +pub struct BuddyFlags(u64);
> +
> +impl BuddyFlags {
> +    /// Range-based allocation from start to end addresses.
> +    pub const RANGE_ALLOCATION: BuddyFlags =
> +        BuddyFlags(bindings::DRM_BUDDY_RANGE_ALLOCATION as u64);
> +
> +    /// Allocate from top of address space downward.
> +    pub const TOPDOWN_ALLOCATION: BuddyFlags =
> +        BuddyFlags(bindings::DRM_BUDDY_TOPDOWN_ALLOCATION as u64);
> +
> +    /// Allocate physically contiguous blocks.
> +    pub const CONTIGUOUS_ALLOCATION: BuddyFlags =
> +        BuddyFlags(bindings::DRM_BUDDY_CONTIGUOUS_ALLOCATION as u64);
> +
> +    /// Clear allocated blocks (zero them).
> +    pub const CLEAR_ALLOCATION: BuddyFlags =
> +        BuddyFlags(bindings::DRM_BUDDY_CLEAR_ALLOCATION as u64);
> +
> +    /// Block has been cleared - internal flag.
> +    pub const CLEARED: BuddyFlags = BuddyFlags(bindings::DRM_BUDDY_CLEARED as u64);
> +
> +    /// Disable trimming of partially used blocks.
> +    pub const TRIM_DISABLE: BuddyFlags = BuddyFlags(bindings::DRM_BUDDY_TRIM_DISABLE as u64);
> +
> +    /// Get raw value for FFI.
> +    pub(crate) fn as_raw(self) -> u64 {
> +        self.0
> +    }
> +}
> +
> +impl core::ops::BitOr for BuddyFlags {
> +    type Output = Self;
> +
> +    fn bitor(self, rhs: Self) -> Self::Output {
> +        Self(self.0 | rhs.0)
> +    }
> +}
> +
> +/// DRM buddy allocator instance.
> +///
> +/// This structure wraps the C `drm_buddy` allocator.
> +///
> +/// # Safety
> +///
> +/// Not thread-safe. Concurrent alloc/free operations require external
> +/// synchronization (e.g., wrapping in `Arc<Mutex<DrmBuddy>>`).
> +///
> +/// # Invariants
> +///
> +/// - `mm` is initialized via `drm_buddy_init()` and remains valid until Drop.

Usually an invariant is a statement about the present rather than about
the past. I would say that `mm` is a valid buddy allocator.

> +pub struct DrmBuddy {
> +    mm: Opaque<bindings::drm_buddy>,
> +}
> +
> +impl DrmBuddy {
> +    /// Create a new buddy allocator.
> +    ///
> +    /// Creates a buddy allocator that manages a contiguous address space of the given
> +    /// size, with the specified minimum allocation unit (chunk_size must be at least 4KB).
> +    ///
> +    /// # Examples
> +    ///
> +    /// See the complete example in the documentation comments for [`AllocatedBlocks`].
> +    pub fn new(size: usize, chunk_size: usize) -> Result<Self> {
> +        // Create buddy allocator with zeroed memory.
> +        let buddy = Self {
> +            mm: Opaque::zeroed(),
> +        };
> +
> +        // Initialize the C buddy structure.
> +        // SAFETY: buddy.mm points to valid, zeroed memory.
> +        unsafe {
> +            to_result(bindings::drm_buddy_init(
> +                buddy.mm.get(),

After this call to drm_buddy_init, you return it which moves the struct.
Is the struct safe to move from one location to another?

Also I usually put the to_result outside of the unsafe block.

> +                size as u64,
> +                chunk_size as u64,
> +            ))?;
> +        }
> +
> +        Ok(buddy)
> +    }
> +
> +    /// Get a raw pointer to the underlying C drm_buddy structure.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Caller must ensure the returned pointer is not used after this
> +    /// structure is dropped.
> +    pub unsafe fn as_raw(&self) -> *mut bindings::drm_buddy {
> +        self.mm.get()
> +    }
> +
> +    /// Get the chunk size (minimum allocation unit).
> +    pub fn chunk_size(&self) -> u64 {
> +        // SAFETY: mm is initialized and valid per struct invariant.
> +        unsafe { (*self.as_raw()).chunk_size }
> +    }
> +
> +    /// Get the total managed size.
> +    pub fn size(&self) -> u64 {
> +        // SAFETY: mm is initialized and valid per struct invariant.
> +        unsafe { (*self.as_raw()).size }
> +    }
> +
> +    /// Get the available (free) memory.
> +    pub fn avail(&self) -> u64 {
> +        // SAFETY: mm is initialized and valid per struct invariant.
> +        unsafe { (*self.as_raw()).avail }
> +    }
> +
> +    /// Allocate blocks from the buddy allocator.
> +    ///
> +    /// Returns an [`AllocatedBlocks`] structure that owns the allocated blocks and automatically
> +    /// frees them when dropped. Allocation of `list_head` uses the `gfp` flags passed.
> +    pub fn alloc_blocks(
> +        &self,
> +        start: usize,
> +        end: usize,
> +        size: usize,
> +        min_block_size: usize,
> +        flags: BuddyFlags,
> +        gfp: Flags,
> +    ) -> Result<AllocatedBlocks<'_>> {
> +        // Allocate list_head on the heap.
> +        let mut list_head = KBox::new(bindings::list_head::default(), gfp)?;
> +
> +        // SAFETY: list_head is valid and heap-allocated.
> +        unsafe {
> +            bindings::INIT_LIST_HEAD(&mut *list_head as *mut _);
> +        }
> +
> +        // SAFETY: mm is a valid DrmBuddy object per the type's invariants.
> +        unsafe {
> +            to_result(bindings::drm_buddy_alloc_blocks(
> +                self.as_raw(),
> +                start as u64,
> +                end as u64,
> +                size as u64,
> +                min_block_size as u64,
> +                &mut *list_head as *mut _,
> +                flags.as_raw() as usize,
> +            ))?;
> +        }
> +
> +        // `list_head` is now the head of a list that contains allocated blocks
> +        // from C code. The allocated blocks will be automatically freed when
> +        // `AllocatedBlocks` is dropped.
> +        Ok(AllocatedBlocks {
> +            list_head,
> +            buddy: self,
> +        })
> +    }
> +}
> +
> +impl Drop for DrmBuddy {
> +    fn drop(&mut self) {
> +        // SAFETY: self.mm is initialized and valid. drm_buddy_fini properly
> +        // cleans up all resources. This is called exactly once during Drop.
> +        unsafe {
> +            bindings::drm_buddy_fini(self.as_raw());
> +        }
> +    }
> +}
> +
> +// SAFETY: DrmBuddy can be sent between threads. Caller is responsible for
> +// ensuring thread-safe access if needed (e.g., via Mutex).
> +unsafe impl Send for DrmBuddy {}

Generally, we should implement both Send and Sync unless we really can't
do so. If methods require external synchronization, then those methods
should be marked &mut self and then you implement Sync.

If you instead omit Sync and make the methods &self, then the caller is
severely restricted and can't e.g. store it in an Arc.

> +/// Allocated blocks from the buddy allocator with automatic cleanup.
> +///
> +/// This structure owns a list of allocated blocks and ensures they are
> +/// automatically freed when dropped. Blocks may be iterated over and are
> +/// read-only after allocation (iteration via [`IntoIterator`] and
> +/// automatic cleanup via [`Drop`] only). To share across threads, wrap
> +/// in `Arc<AllocatedBlocks>`. Rust owns the head list head of the
> +/// allocated blocks; C allocates blocks and links them to the head
> +/// list head. Clean up of the allocated blocks is handled by C code.
> +///
> +/// # Invariants
> +///
> +/// - `list_head` is an owned, valid, initialized list_head.
> +/// - `buddy` points to a valid, initialized [`DrmBuddy`].
> +pub struct AllocatedBlocks<'a> {
> +    list_head: KBox<bindings::list_head>,
> +    buddy: &'a DrmBuddy,
> +}
> +
> +impl Drop for AllocatedBlocks<'_> {
> +    fn drop(&mut self) {
> +        // Free all blocks automatically when dropped.
> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> +        unsafe {
> +            bindings::drm_buddy_free_list(self.buddy.as_raw(), &mut *self.list_head as *mut _, 0);
> +        }
> +    }
> +}
> +
> +impl<'a> AllocatedBlocks<'a> {
> +    /// Check if the block list is empty.
> +    pub fn is_empty(&self) -> bool {
> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> +        unsafe { clist::list_empty(&*self.list_head as *const _) }
> +    }
> +
> +    /// Iterate over allocated blocks.
> +    pub fn iter(&self) -> clist::ClistIter<'_, Block> {
> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> +        clist::iter_list_head::<Block>(&*self.list_head)
> +    }
> +}
> +
> +/// Iteration support for allocated blocks.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// for block in &allocated_blocks {
> +///     // Use block.
> +/// }
> +/// ```
> +impl<'a> IntoIterator for &'a AllocatedBlocks<'_> {
> +    type Item = Block;
> +    type IntoIter = clist::ClistIter<'a, Block>;
> +
> +    fn into_iter(self) -> Self::IntoIter {
> +        self.iter()
> +    }
> +}
> +
> +/// A DRM buddy block.
> +///
> +/// Wraps a pointer to a C `drm_buddy_block` structure. This is returned
> +/// from allocation operations and used to free blocks.
> +///
> +/// # Invariants
> +///
> +/// `drm_buddy_block_ptr` points to a valid `drm_buddy_block` managed by the buddy allocator.
> +pub struct Block {
> +    drm_buddy_block_ptr: NonNull<bindings::drm_buddy_block>,
> +}

This type is exposed to the user by ownership (as opposed to being
exposed behind a reference), and has no lifetime annotation. This
implies that the caller is allowed to keep it alive for arbitrary
amounts of time.

However, it looks like dropping AllocatedBlock would also free this
Block object. That is a problem.

The ownership of Block should probably be tied to AllocatedBlock so that
the borrow-checker prevents dropping AllocatedBlock while Block objects
exist. Or this code should be changed so that Block keeps the underlying
AllocatedBlock alive using a refcount. Or similar. It depends on how it
will be used - if Block is stored long-term in structs, then you should
avoid lifetimes, but if it's a view into AllocatedBlock that is not
stored long-term, then lifetimes are the right choice.

> +impl Block {
> +    /// Get the block's offset in the address space.
> +    pub fn offset(&self) -> u64 {
> +        // SAFETY: drm_buddy_block_ptr is valid per the type's invariants.
> +        unsafe { bindings::drm_buddy_block_offset(self.drm_buddy_block_ptr.as_ptr()) }
> +    }
> +
> +    /// Get the block order (size = chunk_size << order).
> +    pub fn order(&self) -> u32 {
> +        // SAFETY: drm_buddy_block_ptr is valid per the type's invariants.
> +        unsafe { bindings::drm_buddy_block_order(self.drm_buddy_block_ptr.as_ptr()) }
> +    }
> +
> +    /// Get the block's size in bytes.
> +    ///
> +    /// Requires the buddy allocator to calculate size from order.
> +    pub fn size(&self, buddy: &DrmBuddy) -> u64 {
> +        // SAFETY: Both pointers are valid per the type's invariants.
> +        unsafe { bindings::drm_buddy_block_size(buddy.as_raw(), self.drm_buddy_block_ptr.as_ptr()) }
> +    }
> +
> +    /// Get a raw pointer to the underlying C block.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Caller must ensure the pointer is not used after the block is freed.
> +    pub unsafe fn as_ptr(&self) -> *mut bindings::drm_buddy_block {
> +        self.drm_buddy_block_ptr.as_ptr()
> +    }
> +}
> +
> +impl clist::FromListHead for Block {
> +    unsafe fn from_list_head(link: *const bindings::list_head) -> Self {
> +        // SAFETY: link points to a valid list_head embedded in drm_buddy_block.
> +        // The container_of macro calculates the containing struct pointer.
> +        // We need to account for the union field __bindgen_anon_1.link.
> +        //
> +        // The link is embedded in a union within drm_buddy_block:
> +        //     struct drm_buddy_block {
> +        //         [...]
> +        //         union {
> +        //             struct rb_node rb;
> +        //             struct list_head link;
> +        //         };
> +        //     }
> +        //
> +        // This is why we perform a double container_of calculation: first to get
> +        // to the union, then to get to the containing drm_buddy_block.
> +        unsafe {
> +            // First get to the union.
> +            let union_ptr = container_of!(link, bindings::drm_buddy_block__bindgen_ty_1, link);
> +            // Then get to the containing drm_buddy_block.
> +            let block_ptr =
> +                container_of!(union_ptr, bindings::drm_buddy_block, __bindgen_anon_1) as *mut _;
> +            Block {
> +                drm_buddy_block_ptr: NonNull::new_unchecked(block_ptr),
> +            }
> +        }
> +    }
> +}
> +
> +// SAFETY: Block is just a pointer wrapper and can be safely sent between threads.
> +unsafe impl Send for Block {}
> diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
> index 1b82b6945edf..ee173729eac5 100644
> --- a/rust/kernel/drm/mod.rs
> +++ b/rust/kernel/drm/mod.rs
> @@ -2,6 +2,7 @@
>  
>  //! DRM subsystem abstractions.
>  
> +pub mod buddy;
>  pub mod device;
>  pub mod driver;
>  pub mod file;
> -- 
> 2.34.1
> 

Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Joel Fernandes]

Hi Alice,

On 10/31/2025 5:25 AM, Alice Ryhl wrote:
> On Thu, Oct 30, 2025 at 03:06:12PM -0400, Joel Fernandes wrote:
>> Add safe Rust abstractions over the Linux kernel's DRM buddy
>> allocator for physical memory management. The DRM buddy allocator
>> implements a binary buddy system for useful for GPU physical memory
>> allocation. nova-core will use it for physical memory allocation.
>>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
[...]>> +
>> +/// DRM buddy allocator instance.
>> +///
>> +/// This structure wraps the C `drm_buddy` allocator.
>> +///
>> +/// # Safety
>> +///
>> +/// Not thread-safe. Concurrent alloc/free operations require external
>> +/// synchronization (e.g., wrapping in `Arc<Mutex<DrmBuddy>>`).
>> +///
>> +/// # Invariants
>> +///
>> +/// - `mm` is initialized via `drm_buddy_init()` and remains valid until Drop.
> 
> Usually an invariant is a statement about the present rather than about
> the past. I would say that `mm` is a valid buddy allocator.
Noted, and I will change it to that, thanks!

>> +pub struct DrmBuddy {
>> +    mm: Opaque<bindings::drm_buddy>,
>> +}
>> +
>> +impl DrmBuddy {
>> +    /// Create a new buddy allocator.
>> +    ///
>> +    /// Creates a buddy allocator that manages a contiguous address space of the given
>> +    /// size, with the specified minimum allocation unit (chunk_size must be at least 4KB).
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// See the complete example in the documentation comments for [`AllocatedBlocks`].
>> +    pub fn new(size: usize, chunk_size: usize) -> Result<Self> {
>> +        // Create buddy allocator with zeroed memory.
>> +        let buddy = Self {
>> +            mm: Opaque::zeroed(),
>> +        };
>> +
>> +        // Initialize the C buddy structure.
>> +        // SAFETY: buddy.mm points to valid, zeroed memory.
>> +        unsafe {
>> +            to_result(bindings::drm_buddy_init(
>> +                buddy.mm.get(),
> 
> After this call to drm_buddy_init, you return it which moves the struct.
> Is the struct safe to move from one location to another?

It is safe to move this struct since it does not contain anything
self-referential or DMA buffers. But we should pin it for more robustness/future
proofing. I will do that.

> Also I usually put the to_result outside of the unsafe block.

Will do.

[...]
>> +
>> +    /// Allocate blocks from the buddy allocator.
>> +    ///
>> +    /// Returns an [`AllocatedBlocks`] structure that owns the allocated blocks and automatically
>> +    /// frees them when dropped. Allocation of `list_head` uses the `gfp` flags passed.
>> +    pub fn alloc_blocks(
>> +        &self,
>> +        start: usize,
>> +        end: usize,
>> +        size: usize,
>> +        min_block_size: usize,
>> +        flags: BuddyFlags,
>> +        gfp: Flags,
>> +    ) -> Result<AllocatedBlocks<'_>> {
>> +        // Allocate list_head on the heap.
>> +        let mut list_head = KBox::new(bindings::list_head::default(), gfp)?;
>> +
>> +        // SAFETY: list_head is valid and heap-allocated.
>> +        unsafe {
>> +            bindings::INIT_LIST_HEAD(&mut *list_head as *mut _);
>> +        }
>> +
>> +        // SAFETY: mm is a valid DrmBuddy object per the type's invariants.
>> +        unsafe {
>> +            to_result(bindings::drm_buddy_alloc_blocks(
>> +                self.as_raw(),
>> +                start as u64,
>> +                end as u64,
>> +                size as u64,
>> +                min_block_size as u64,
>> +                &mut *list_head as *mut _,
>> +                flags.as_raw() as usize,
>> +            ))?;
>> +        }
>> +
>> +        // `list_head` is now the head of a list that contains allocated blocks
>> +        // from C code. The allocated blocks will be automatically freed when
>> +        // `AllocatedBlocks` is dropped.
>> +        Ok(AllocatedBlocks {
>> +            list_head,
>> +            buddy: self,
>> +        })
>> +    }
>> +}
>> +
>> +impl Drop for DrmBuddy {
>> +    fn drop(&mut self) {
>> +        // SAFETY: self.mm is initialized and valid. drm_buddy_fini properly
>> +        // cleans up all resources. This is called exactly once during Drop.
>> +        unsafe {
>> +            bindings::drm_buddy_fini(self.as_raw());
>> +        }
>> +    }
>> +}
>> +
>> +// SAFETY: DrmBuddy can be sent between threads. Caller is responsible for
>> +// ensuring thread-safe access if needed (e.g., via Mutex).
>> +unsafe impl Send for DrmBuddy {}
> 
> Generally, we should implement both Send and Sync unless we really can't
> do so. If methods require external synchronization, then those methods
> should be marked &mut self and then you implement Sync.

Thanks for letting me know the convention. Just to clarify, when you say "then
you implement Sync", you mean "then you implement mutual exclusion, say via
locks" correct?
> If you instead omit Sync and make the methods &self, then the caller is
> severely restricted and can't e.g. store it in an Arc.
Ok so I will implement Sync and keep the methods as &self since at the moment,
mutation is not required. Let me know if I missed something though.

>> +/// Allocated blocks from the buddy allocator with automatic cleanup.
>> +///
>> +/// This structure owns a list of allocated blocks and ensures they are
>> +/// automatically freed when dropped. Blocks may be iterated over and are
>> +/// read-only after allocation (iteration via [`IntoIterator`] and
>> +/// automatic cleanup via [`Drop`] only). To share across threads, wrap
>> +/// in `Arc<AllocatedBlocks>`. Rust owns the head list head of the
>> +/// allocated blocks; C allocates blocks and links them to the head
>> +/// list head. Clean up of the allocated blocks is handled by C code.
>> +///
>> +/// # Invariants
>> +///
>> +/// - `list_head` is an owned, valid, initialized list_head.
>> +/// - `buddy` points to a valid, initialized [`DrmBuddy`].
>> +pub struct AllocatedBlocks<'a> {
>> +    list_head: KBox<bindings::list_head>,
>> +    buddy: &'a DrmBuddy,
>> +}
>> +
>> +impl Drop for AllocatedBlocks<'_> {
>> +    fn drop(&mut self) {
>> +        // Free all blocks automatically when dropped.
>> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
>> +        unsafe {
>> +            bindings::drm_buddy_free_list(self.buddy.as_raw(), &mut *self.list_head as *mut _, 0);
>> +        }
>> +    }
>> +}
>> +
>> +impl<'a> AllocatedBlocks<'a> {
>> +    /// Check if the block list is empty.
>> +    pub fn is_empty(&self) -> bool {
>> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
>> +        unsafe { clist::list_empty(&*self.list_head as *const _) }
>> +    }
>> +
>> +    /// Iterate over allocated blocks.
>> +    pub fn iter(&self) -> clist::ClistIter<'_, Block> {
>> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
>> +        clist::iter_list_head::<Block>(&*self.list_head)
>> +    }
>> +}
>> +
>> +/// Iteration support for allocated blocks.
>> +///
>> +/// # Examples
>> +///
>> +/// ```ignore
>> +/// for block in &allocated_blocks {
>> +///     // Use block.
>> +/// }
>> +/// ```
>> +impl<'a> IntoIterator for &'a AllocatedBlocks<'_> {
>> +    type Item = Block;
>> +    type IntoIter = clist::ClistIter<'a, Block>;
>> +
>> +    fn into_iter(self) -> Self::IntoIter {
>> +        self.iter()
>> +    }
>> +}
>> +
>> +/// A DRM buddy block.
>> +///
>> +/// Wraps a pointer to a C `drm_buddy_block` structure. This is returned
>> +/// from allocation operations and used to free blocks.
>> +///
>> +/// # Invariants
>> +///
>> +/// `drm_buddy_block_ptr` points to a valid `drm_buddy_block` managed by the buddy allocator.
>> +pub struct Block {
>> +    drm_buddy_block_ptr: NonNull<bindings::drm_buddy_block>,
>> +}
> 
> This type is exposed to the user by ownership (as opposed to being
> exposed behind a reference), and has no lifetime annotation. This
> implies that the caller is allowed to keep it alive for arbitrary
> amounts of time.
> 
> However, it looks like dropping AllocatedBlock would also free this
> Block object. That is a problem.
> 
> The ownership of Block should probably be tied to AllocatedBlock so that
> the borrow-checker prevents dropping AllocatedBlock while Block objects
> exist. Or this code should be changed so that Block keeps the underlying
> AllocatedBlock alive using a refcount. Or similar. It depends on how it
> will be used - if Block is stored long-term in structs, then you should
> avoid lifetimes, but if it's a view into AllocatedBlock that is not
> stored long-term, then lifetimes are the right choice.
> 
You're right! Thanks for catching this, I am leaning to the ref count approach,
but I'll look into the use cases more and designing it accordingly.

thanks,

 - Joel

Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Alexandre Courbot]

On Fri Oct 31, 2025 at 4:06 AM JST, Joel Fernandes wrote:
<snip>
> +/// DRM buddy allocator instance.
> +///
> +/// This structure wraps the C `drm_buddy` allocator.
> +///
> +/// # Safety
> +///
> +/// Not thread-safe. Concurrent alloc/free operations require external
> +/// synchronization (e.g., wrapping in `Arc<Mutex<DrmBuddy>>`).
> +///
> +/// # Invariants
> +///
> +/// - `mm` is initialized via `drm_buddy_init()` and remains valid until Drop.
> +pub struct DrmBuddy {
> +    mm: Opaque<bindings::drm_buddy>,
> +}

not a big deal, but usually such wrapping structures are defined as
follows:

pub struct DrmBuddy(Opaque<bindings::drm_buddy>);

> +
> +impl DrmBuddy {
> +    /// Create a new buddy allocator.
> +    ///
> +    /// Creates a buddy allocator that manages a contiguous address space of the given
> +    /// size, with the specified minimum allocation unit (chunk_size must be at least 4KB).
> +    ///
> +    /// # Examples
> +    ///
> +    /// See the complete example in the documentation comments for [`AllocatedBlocks`].
> +    pub fn new(size: usize, chunk_size: usize) -> Result<Self> {
> +        // Create buddy allocator with zeroed memory.
> +        let buddy = Self {
> +            mm: Opaque::zeroed(),

Isn't `Opaque::uninit` more appropriate here, since `drm_buddy_init`
below will overwrite the data?

<snip>
> +// SAFETY: DrmBuddy can be sent between threads. Caller is responsible for
> +// ensuring thread-safe access if needed (e.g., via Mutex).
> +unsafe impl Send for DrmBuddy {}
> +
> +/// Allocated blocks from the buddy allocator with automatic cleanup.
> +///
> +/// This structure owns a list of allocated blocks and ensures they are
> +/// automatically freed when dropped. Blocks may be iterated over and are
> +/// read-only after allocation (iteration via [`IntoIterator`] and
> +/// automatic cleanup via [`Drop`] only). To share across threads, wrap
> +/// in `Arc<AllocatedBlocks>`. Rust owns the head list head of the
> +/// allocated blocks; C allocates blocks and links them to the head
> +/// list head. Clean up of the allocated blocks is handled by C code.
> +///
> +/// # Invariants
> +///
> +/// - `list_head` is an owned, valid, initialized list_head.
> +/// - `buddy` points to a valid, initialized [`DrmBuddy`].
> +pub struct AllocatedBlocks<'a> {
> +    list_head: KBox<bindings::list_head>,
> +    buddy: &'a DrmBuddy,
> +}

Isn't the lifetime going to severely restrict how this can be used?

For instance, after allocating a list of blocks I suppose you will want
to store it somewhere, do some other business, and free it much later in
a completely different code path. The lifetime is going to make this
very difficult.

For instance, try and adapt the unit test in the following patch to
allocate some driver object on the heap (representing a bound device),
and store both the `DrmBuddy` and the allocated blocks into it. I don't
think the borrow checker will let you do that.

I think this calls for a reference-counted design instead - this will
move lifetime management to runtime, and solve the issue.

> +
> +impl Drop for AllocatedBlocks<'_> {
> +    fn drop(&mut self) {
> +        // Free all blocks automatically when dropped.
> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> +        unsafe {
> +            bindings::drm_buddy_free_list(self.buddy.as_raw(), &mut *self.list_head as *mut _, 0);
> +        }
> +    }
> +}
> +
> +impl<'a> AllocatedBlocks<'a> {
> +    /// Check if the block list is empty.
> +    pub fn is_empty(&self) -> bool {
> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> +        unsafe { clist::list_empty(&*self.list_head as *const _) }
> +    }
> +
> +    /// Iterate over allocated blocks.
> +    pub fn iter(&self) -> clist::ClistIter<'_, Block> {
> +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> +        clist::iter_list_head::<Block>(&*self.list_head)
> +    }
> +}
> +
> +/// Iteration support for allocated blocks.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// for block in &allocated_blocks {
> +///     // Use block.
> +/// }
> +/// ```
> +impl<'a> IntoIterator for &'a AllocatedBlocks<'_> {
> +    type Item = Block;
> +    type IntoIter = clist::ClistIter<'a, Block>;
> +
> +    fn into_iter(self) -> Self::IntoIter {
> +        self.iter()
> +    }
> +}
> +
> +/// A DRM buddy block.
> +///
> +/// Wraps a pointer to a C `drm_buddy_block` structure. This is returned
> +/// from allocation operations and used to free blocks.
> +///
> +/// # Invariants
> +///
> +/// `drm_buddy_block_ptr` points to a valid `drm_buddy_block` managed by the buddy allocator.
> +pub struct Block {
> +    drm_buddy_block_ptr: NonNull<bindings::drm_buddy_block>,
> +}

This also looks like a good change to use a transparent struct with an
opaque. I guess once you adapt the CList design as suggested it will
come to this naturally.

Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Joel Fernandes]

On Sat, Nov 01, 2025 at 02:08:56PM +0900, Alexandre Courbot wrote:
> On Fri Oct 31, 2025 at 4:06 AM JST, Joel Fernandes wrote:
> <snip>
> > +/// DRM buddy allocator instance.
> > +///
> > +/// This structure wraps the C `drm_buddy` allocator.
> > +///
> > +/// # Safety
> > +///
> > +/// Not thread-safe. Concurrent alloc/free operations require external
> > +/// synchronization (e.g., wrapping in `Arc<Mutex<DrmBuddy>>`).
> > +///
> > +/// # Invariants
> > +///
> > +/// - `mm` is initialized via `drm_buddy_init()` and remains valid until Drop.
> > +pub struct DrmBuddy {
> > +    mm: Opaque<bindings::drm_buddy>,
> > +}
> 
> not a big deal, but usually such wrapping structures are defined as
> follows:
> 
> pub struct DrmBuddy(Opaque<bindings::drm_buddy>);

Sure.

> 
> > +
> > +impl DrmBuddy {
> > +    /// Create a new buddy allocator.
> > +    ///
> > +    /// Creates a buddy allocator that manages a contiguous address space of the given
> > +    /// size, with the specified minimum allocation unit (chunk_size must be at least 4KB).
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// See the complete example in the documentation comments for [`AllocatedBlocks`].
> > +    pub fn new(size: usize, chunk_size: usize) -> Result<Self> {
> > +        // Create buddy allocator with zeroed memory.
> > +        let buddy = Self {
> > +            mm: Opaque::zeroed(),
> 
> Isn't `Opaque::uninit` more appropriate here, since `drm_buddy_init`
> below will overwrite the data?

Sure.

> 
> <snip>
> > +// SAFETY: DrmBuddy can be sent between threads. Caller is responsible for
> > +// ensuring thread-safe access if needed (e.g., via Mutex).
> > +unsafe impl Send for DrmBuddy {}
> > +
> > +/// Allocated blocks from the buddy allocator with automatic cleanup.
> > +///
> > +/// This structure owns a list of allocated blocks and ensures they are
> > +/// automatically freed when dropped. Blocks may be iterated over and are
> > +/// read-only after allocation (iteration via [`IntoIterator`] and
> > +/// automatic cleanup via [`Drop`] only). To share across threads, wrap
> > +/// in `Arc<AllocatedBlocks>`. Rust owns the head list head of the
> > +/// allocated blocks; C allocates blocks and links them to the head
> > +/// list head. Clean up of the allocated blocks is handled by C code.
> > +///
> > +/// # Invariants
> > +///
> > +/// - `list_head` is an owned, valid, initialized list_head.
> > +/// - `buddy` points to a valid, initialized [`DrmBuddy`].
> > +pub struct AllocatedBlocks<'a> {
> > +    list_head: KBox<bindings::list_head>,
> > +    buddy: &'a DrmBuddy,
> > +}
> 
> Isn't the lifetime going to severely restrict how this can be used?
> 
> For instance, after allocating a list of blocks I suppose you will want
> to store it somewhere, do some other business, and free it much later in
> a completely different code path. The lifetime is going to make this
> very difficult.
> 
> For instance, try and adapt the unit test in the following patch to
> allocate some driver object on the heap (representing a bound device),
> and store both the `DrmBuddy` and the allocated blocks into it. I don't
> think the borrow checker will let you do that.
> 
> I think this calls for a reference-counted design instead - this will
> move lifetime management to runtime, and solve the issue.
> 

Agreed, I will use refcounting. I am also looking into Alice's suggestion
about doing the same between the individual blocks and the AllocatedBlocks.

> > +
> > +impl Drop for AllocatedBlocks<'_> {
> > +    fn drop(&mut self) {
> > +        // Free all blocks automatically when dropped.
> > +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> > +        unsafe {
> > +            bindings::drm_buddy_free_list(self.buddy.as_raw(), &mut *self.list_head as *mut _, 0);
> > +        }
> > +    }
> > +}
> > +
> > +impl<'a> AllocatedBlocks<'a> {
> > +    /// Check if the block list is empty.
> > +    pub fn is_empty(&self) -> bool {
> > +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> > +        unsafe { clist::list_empty(&*self.list_head as *const _) }
> > +    }
> > +
> > +    /// Iterate over allocated blocks.
> > +    pub fn iter(&self) -> clist::ClistIter<'_, Block> {
> > +        // SAFETY: list_head is a valid list of blocks per the type's invariants.
> > +        clist::iter_list_head::<Block>(&*self.list_head)
> > +    }
> > +}
> > +
> > +/// Iteration support for allocated blocks.
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// for block in &allocated_blocks {
> > +///     // Use block.
> > +/// }
> > +/// ```
> > +impl<'a> IntoIterator for &'a AllocatedBlocks<'_> {
> > +    type Item = Block;
> > +    type IntoIter = clist::ClistIter<'a, Block>;
> > +
> > +    fn into_iter(self) -> Self::IntoIter {
> > +        self.iter()
> > +    }
> > +}
> > +
> > +/// A DRM buddy block.
> > +///
> > +/// Wraps a pointer to a C `drm_buddy_block` structure. This is returned
> > +/// from allocation operations and used to free blocks.
> > +///
> > +/// # Invariants
> > +///
> > +/// `drm_buddy_block_ptr` points to a valid `drm_buddy_block` managed by the buddy allocator.
> > +pub struct Block {
> > +    drm_buddy_block_ptr: NonNull<bindings::drm_buddy_block>,
> > +}
> 
> This also looks like a good change to use a transparent struct with an
> opaque. I guess once you adapt the CList design as suggested it will
> come to this naturally.
> 

Sure, sounds good, thanks!

 - Joel

Re: [PATCH RFC 3/4] rust: drm: Add DRM buddy allocator bindings

[Alexandre Courbot]

On Fri Oct 31, 2025 at 4:06 AM JST, Joel Fernandes wrote:
<snip>
> +    /// Allocate blocks from the buddy allocator.
> +    ///
> +    /// Returns an [`AllocatedBlocks`] structure that owns the allocated blocks and automatically
> +    /// frees them when dropped. Allocation of `list_head` uses the `gfp` flags passed.
> +    pub fn alloc_blocks(
> +        &self,
> +        start: usize,
> +        end: usize,
> +        size: usize,
> +        min_block_size: usize,
> +        flags: BuddyFlags,
> +        gfp: Flags,
> +    ) -> Result<AllocatedBlocks<'_>> {

It looks like some of the flags heavily modify the behavior of the
allocator, and make some of the parameters irrelevant (for instance,
IIUC `start` and `end` only make sense if `RANGE_ALLOCATION` is
passed?).

In that case, we should either have several allocation methods, or have
an `AllocationRequest` enum which variants are the allocation type and its
relevant parameters. E.g (very naively):

enum AllocationRange {
    Whole,
    Ranged(Range<u64>),
    ...
}

struct AllocationRequest {
    range: AllocationRange,
    top_down: bool,
    contiguous: bool,
    clear: bool,
}

I.e. use the type system to make sure we can only express things that
make sense, and never pass data that will end up being ignored.

If we do that well, I think we can drop the `BuddyFlags` type
altogether, which is great as it seems to serve several different
purposes.

[PATCH RFC 4/4] samples: rust: Add sample demonstrating DRM buddy allocator

[Joel Fernandes]

Demonstrates usage of the DRM buddy allocator bindings through
a simple test module that initializes the allocator, performs
allocations, and prints information about the allocated blocks.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 samples/rust/Kconfig           |  14 +++++
 samples/rust/Makefile          |   1 +
 samples/rust/rust_drm_buddy.rs | 106 +++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+)
 create mode 100644 samples/rust/rust_drm_buddy.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b45631e2593c..8ccb4064ba91 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -21,6 +21,20 @@ config SAMPLE_RUST_CLIST
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRM_BUDDY
+	tristate "DRM buddy allocator sample"
+	depends on DRM_BUDDY
+	help
+	  This option builds the Rust DRM buddy allocator sample.
+
+	  The sample demonstrates using the DRM buddy allocator bindings
+	  to allocate and free memory blocks.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_drm_buddy.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_CONFIGFS
 	tristate "Configfs sample"
 	depends on CONFIGFS_FS
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index f8899c0df762..a56204ee4e96 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,6 +2,7 @@
 ccflags-y += -I$(src)				# needed for trace events
 
 obj-$(CONFIG_SAMPLE_RUST_CLIST)			+= rust_clist.o
+obj-$(CONFIG_SAMPLE_RUST_DRM_BUDDY)		+= rust_drm_buddy.o
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
diff --git a/samples/rust/rust_drm_buddy.rs b/samples/rust/rust_drm_buddy.rs
new file mode 100644
index 000000000000..96907bc19243
--- /dev/null
+++ b/samples/rust/rust_drm_buddy.rs
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust DRM buddy allocator sample.
+//!
+//! This sample demonstrates using the DRM buddy allocator from Rust.
+
+use kernel::{
+    drm::buddy::{
+        BuddyFlags,
+        DrmBuddy, //
+    },
+    prelude::*,
+    sizes::*, //
+};
+
+module! {
+    type: RustDrmBuddySample,
+    name: "rust_drm_buddy",
+    authors: ["Joel Fernandes"],
+    description: "DRM buddy allocator sample",
+    license: "GPL",
+}
+
+struct RustDrmBuddySample;
+
+impl kernel::Module for RustDrmBuddySample {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        // Create a buddy allocator managing 1GB with 4KB chunks.
+        let buddy = DrmBuddy::new(SZ_1G, SZ_4K)?;
+
+        pr_info!("=== Test 1: Single 16MB block ===\n");
+        let allocated = buddy.alloc_blocks(
+            0,
+            0,
+            SZ_16M,
+            SZ_4K,
+            BuddyFlags::RANGE_ALLOCATION,
+            GFP_KERNEL,
+        )?;
+
+        let mut count = 0;
+        for block in &allocated {
+            pr_info!(
+                "  Block {}: offset=0x{:x}, order={}, size={}\n",
+                count,
+                block.offset(),
+                block.order(),
+                block.size(&buddy)
+            );
+            count += 1;
+        }
+        pr_info!("  Total: {} blocks\n", count);
+        drop(allocated);
+
+        pr_info!("=== Test 2: Three 4MB blocks ===\n");
+        let allocated = buddy.alloc_blocks(
+            0,
+            0,
+            SZ_4M * 3,
+            SZ_4K,
+            BuddyFlags::RANGE_ALLOCATION,
+            GFP_KERNEL,
+        )?;
+
+        count = 0;
+        for block in &allocated {
+            pr_info!(
+                "  Block {}: offset=0x{:x}, order={}, size={}\n",
+                count,
+                block.offset(),
+                block.order(),
+                block.size(&buddy)
+            );
+            count += 1;
+        }
+        pr_info!("  Total: {} blocks\n", count);
+        drop(allocated);
+
+        pr_info!("=== Test 3: Two 8MB blocks ===\n");
+        let allocated = buddy.alloc_blocks(
+            0,
+            0,
+            SZ_8M * 2,
+            SZ_4K,
+            BuddyFlags::RANGE_ALLOCATION,
+            GFP_KERNEL,
+        )?;
+
+        count = 0;
+        for block in &allocated {
+            pr_info!(
+                "  Block {}: offset=0x{:x}, order={}, size={}\n",
+                count,
+                block.offset(),
+                block.order(),
+                block.size(&buddy)
+            );
+            count += 1;
+        }
+        pr_info!("  Total: {} blocks\n", count);
+
+        pr_info!("=== All tests passed! ===\n");
+
+        Ok(RustDrmBuddySample {})
+    }
+}
-- 
2.34.1

Re: [PATCH RFC 4/4] samples: rust: Add sample demonstrating DRM buddy allocator

[Danilo Krummrich]

On Thu Oct 30, 2025 at 8:06 PM CET, Joel Fernandes wrote:
> Demonstrates usage of the DRM buddy allocator bindings through
> a simple test module that initializes the allocator, performs
> allocations, and prints information about the allocated blocks.

I don't think this should be a sample module either, the code looks a bit like
it tries to reinvents kunit tests.

Re: [PATCH RFC 4/4] samples: rust: Add sample demonstrating DRM buddy allocator

[Matthew Auld]

On 30/10/2025 19:06, Joel Fernandes wrote:
> Demonstrates usage of the DRM buddy allocator bindings through
> a simple test module that initializes the allocator, performs
> allocations, and prints information about the allocated blocks.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>   samples/rust/Kconfig           |  14 +++++
>   samples/rust/Makefile          |   1 +
>   samples/rust/rust_drm_buddy.rs | 106 +++++++++++++++++++++++++++++++++
>   3 files changed, 121 insertions(+)
>   create mode 100644 samples/rust/rust_drm_buddy.rs
> 
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index b45631e2593c..8ccb4064ba91 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -21,6 +21,20 @@ config SAMPLE_RUST_CLIST
>   
>   	  If unsure, say N.
>   
> +config SAMPLE_RUST_DRM_BUDDY
> +	tristate "DRM buddy allocator sample"
> +	depends on DRM_BUDDY
> +	help
> +	  This option builds the Rust DRM buddy allocator sample.
> +
> +	  The sample demonstrates using the DRM buddy allocator bindings
> +	  to allocate and free memory blocks.
> +
> +	  To compile this as a module, choose M here:
> +	  the module will be called rust_drm_buddy.
> +
> +	  If unsure, say N.
> +
>   config SAMPLE_RUST_CONFIGFS
>   	tristate "Configfs sample"
>   	depends on CONFIGFS_FS
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index f8899c0df762..a56204ee4e96 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -2,6 +2,7 @@
>   ccflags-y += -I$(src)				# needed for trace events
>   
>   obj-$(CONFIG_SAMPLE_RUST_CLIST)			+= rust_clist.o
> +obj-$(CONFIG_SAMPLE_RUST_DRM_BUDDY)		+= rust_drm_buddy.o
>   obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
>   obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
>   obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
> diff --git a/samples/rust/rust_drm_buddy.rs b/samples/rust/rust_drm_buddy.rs
> new file mode 100644
> index 000000000000..96907bc19243
> --- /dev/null
> +++ b/samples/rust/rust_drm_buddy.rs
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust DRM buddy allocator sample.
> +//!
> +//! This sample demonstrates using the DRM buddy allocator from Rust.
> +
> +use kernel::{
> +    drm::buddy::{
> +        BuddyFlags,
> +        DrmBuddy, //
> +    },
> +    prelude::*,
> +    sizes::*, //
> +};
> +
> +module! {
> +    type: RustDrmBuddySample,
> +    name: "rust_drm_buddy",
> +    authors: ["Joel Fernandes"],
> +    description: "DRM buddy allocator sample",
> +    license: "GPL",
> +}
> +
> +struct RustDrmBuddySample;
> +
> +impl kernel::Module for RustDrmBuddySample {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        // Create a buddy allocator managing 1GB with 4KB chunks.
> +        let buddy = DrmBuddy::new(SZ_1G, SZ_4K)?;
> +
> +        pr_info!("=== Test 1: Single 16MB block ===\n");
> +        let allocated = buddy.alloc_blocks(
> +            0,
> +            0,

Does this map to the start/end? Surprised that this works with 
RANGE_ALLOCATION below. I guess it works because of the end-1, but I'm 
not sure if that was intended.

Anyway, probably you didn't really want RANGE_ALLOCATION here? That is 
only if you want something at a specific offset or within a special bias 
range. So here I think it will give you a massive bias range covering 
everything due to end-1, but all you wanted was any available 16M block, 
which is the typical flow? It still technically works, but looks a bit 
non-standard and will internally take the bias range path, which is not 
ideal :)

Also I guess worth updating the example in buddy.rs, which also does this?

> +            SZ_16M,
> +            SZ_4K,
> +            BuddyFlags::RANGE_ALLOCATION,
> +            GFP_KERNEL,
> +        )?;
> +
> +        let mut count = 0;
> +        for block in &allocated {
> +            pr_info!(
> +                "  Block {}: offset=0x{:x}, order={}, size={}\n",
> +                count,
> +                block.offset(),
> +                block.order(),
> +                block.size(&buddy)
> +            );
> +            count += 1;
> +        }
> +        pr_info!("  Total: {} blocks\n", count);
> +        drop(allocated);
> +
> +        pr_info!("=== Test 2: Three 4MB blocks ===\n");
> +        let allocated = buddy.alloc_blocks(
> +            0,
> +            0,
> +            SZ_4M * 3,
> +            SZ_4K,
> +            BuddyFlags::RANGE_ALLOCATION,
> +            GFP_KERNEL,
> +        )?;
> +
> +        count = 0;
> +        for block in &allocated {
> +            pr_info!(
> +                "  Block {}: offset=0x{:x}, order={}, size={}\n",
> +                count,
> +                block.offset(),
> +                block.order(),
> +                block.size(&buddy)
> +            );
> +            count += 1;
> +        }
> +        pr_info!("  Total: {} blocks\n", count);
> +        drop(allocated);
> +
> +        pr_info!("=== Test 3: Two 8MB blocks ===\n");
> +        let allocated = buddy.alloc_blocks(
> +            0,
> +            0,
> +            SZ_8M * 2,
> +            SZ_4K,
> +            BuddyFlags::RANGE_ALLOCATION,
> +            GFP_KERNEL,
> +        )?;
> +
> +        count = 0;
> +        for block in &allocated {
> +            pr_info!(
> +                "  Block {}: offset=0x{:x}, order={}, size={}\n",
> +                count,
> +                block.offset(),
> +                block.order(),
> +                block.size(&buddy)
> +            );
> +            count += 1;
> +        }
> +        pr_info!("  Total: {} blocks\n", count);
> +
> +        pr_info!("=== All tests passed! ===\n");
> +
> +        Ok(RustDrmBuddySample {})
> +    }
> +}

Re: [PATCH RFC 4/4] samples: rust: Add sample demonstrating DRM buddy allocator

[Alexandre Courbot]

On Fri Oct 31, 2025 at 4:06 AM JST, Joel Fernandes wrote:
> Demonstrates usage of the DRM buddy allocator bindings through
> a simple test module that initializes the allocator, performs
> allocations, and prints information about the allocated blocks.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

If this can be run as a kunit test (and it looks like it does?), I think
this would be more useful as a module-level doctest of `buddy.rs`, where
it can also be useful for educational purposes.