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

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`?

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/

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

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

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

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.

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

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

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

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

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

On Wed, Nov 5, 2025 at 11:54 AM Guillaume Gomez
<guillaume1.gomez@gmail.com> wrote:
>
> 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.

If you mean the `unknown` field (in the JSON) that I mentioned in my
message, then yeah, I think that is good enough (though we should
probably still prevent collisions with future `rustdoc` ones).

If you mean something else that I may be missing, please let me know, of course!

And yeah, we can take a look together (likely after LPC).

Thanks!

Cheers,
Miguel

Yeah that's what I meant. Ping me whenever you want to talk about it.

Le mar. 11 nov. 2025 à 21:32, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> a écrit :
>
> On Wed, Nov 5, 2025 at 11:54 AM Guillaume Gomez
> <guillaume1.gomez@gmail.com> wrote:
> >
> > 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.
>
> If you mean the `unknown` field (in the JSON) that I mentioned in my
> message, then yeah, I think that is good enough (though we should
> probably still prevent collisions with future `rustdoc` ones).
>
> If you mean something else that I may be missing, please let me know, of course!
>
> And yeah, we can take a look together (likely after LPC).
>
> Thanks!
>
> Cheers,
> Miguel

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.

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