[PATCH v3] rust: clist: Add support to interface with C linked lists

[PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

Add a new module `clist` for working with C's doubly circular linked
lists. Provide low-level iteration over list_head nodes and high-level
iteration over typed list items.

Provide a `clist_create` macro to assist in creation of the `Clist` type.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
Thanks to Alex, Daniel, Danilo, John, and Miguel for reviewing v1/v2!

Changes since v2:
  - Squashed the 3 patches into a single patch due to dependencies and reducing helpers.
  - Changed Clist to Clist<'a, T> using const generic offset (Alex).
  - Simplified C helpers to only list_add_tail (Alex, John).
  - Added init_list_head() for INIT_LIST_HEAD functionality and dropped it from C helpers (Alex).
  - Added FusedIterator impl for both ClistHeadIter and ClistIter.
  - Added PartialEq/Eq for ClistHead (Alex)
  - Rust style improvements, comment improvements (Daniel).
  - Added MAINTAINERS entry (Miguel).

Link to v2: https://lore.kernel.org/all/20251111171315.2196103-2-joelagnelf@nvidia.com/
Link to v1 (RFC): https://lore.kernel.org/all/20251030190613.1224287-2-joelagnelf@nvidia.com/

 MAINTAINERS            |   7 +
 rust/helpers/helpers.c |   1 +
 rust/helpers/list.c    |  12 ++
 rust/kernel/clist.rs   | 349 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs     |   1 +
 5 files changed, 370 insertions(+)
 create mode 100644 rust/helpers/list.c
 create mode 100644 rust/kernel/clist.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 5f7aa6a8a9a1..fb2ff877b6d1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22666,6 +22666,13 @@ F:	rust/kernel/init.rs
 F:	rust/pin-init/
 K:	\bpin-init\b|pin_init\b|PinInit
 
+RUST TO C LIST INTERFACES
+M:	Joel Fernandes <joelagnelf@nvidia.com>
+M:	Alexandre Courbot <acourbot@nvidia.com>
+L:	rust-for-linux@vger.kernel.org
+S:	Maintained
+F:	rust/kernel/clist.rs
+
 RXRPC SOCKETS (AF_RXRPC)
 M:	David Howells <dhowells@redhat.com>
 M:	Marc Dionne <marc.dionne@auristor.com>
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..6044979c7a2e
--- /dev/null
+++ b/rust/helpers/list.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Helpers for C Circular doubly linked list implementation.
+ */
+
+#include <linux/list.h>
+
+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..361a6132299b
--- /dev/null
+++ b/rust/kernel/clist.rs
@@ -0,0 +1,349 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! A C doubly circular intrusive linked list interface for rust code.
+//!
+//! # Examples
+//!
+//! ```
+//! use kernel::{
+//!     bindings,
+//!     clist::init_list_head,
+//!     clist_create,
+//!     types::Opaque, //
+//! };
+//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
+//! # // emulated here for doctests using the C bindings.
+//! # use core::mem::MaybeUninit;
+//! #
+//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
+//! # #[repr(C)]
+//! # pub(crate) struct SampleItemC {
+//! #     pub value: i32,
+//! #     pub link: bindings::list_head,
+//! # }
+//! #
+//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
+//! #
+//! # // SAFETY: head and all the items are test objects allocated in this scope.
+//! # unsafe { init_list_head(head.as_mut_ptr()) };
+//! # // SAFETY: head is a test object allocated in this scope.
+//! # let mut head = unsafe { head.assume_init() };
+//! # let mut items = [
+//! #     MaybeUninit::<SampleItemC>::uninit(),
+//! #     MaybeUninit::<SampleItemC>::uninit(),
+//! #     MaybeUninit::<SampleItemC>::uninit(),
+//! # ];
+//! #
+//! # for (i, item) in items.iter_mut().enumerate() {
+//! #     let ptr = item.as_mut_ptr();
+//! #     // SAFETY: pointers are to allocated test objects with a list_head field.
+//! #     unsafe {
+//! #         (*ptr).value = i as i32 * 10;
+//! #         // addr_of_mut!() computes address of link directly as link is uninitialized.
+//! #         init_list_head(core::ptr::addr_of_mut!((*ptr).link));
+//! #         bindings::list_add_tail(&mut (*ptr).link, &mut head);
+//! #     }
+//! # }
+//!
+//! // Rust wrapper for the C struct.
+//! // The list item struct in this example is defined in C code as:
+//! //   struct SampleItemC {
+//! //       int value;
+//! //       struct list_head link;
+//! //   };
+//! //
+//! #[repr(transparent)]
+//! pub(crate) struct Item(Opaque<SampleItemC>);
+//!
+//! impl Item {
+//!     pub(crate) fn value(&self) -> i32 {
+//!         // SAFETY: `Item` has same layout as `SampleItemC`.
+//!         unsafe { (*self.0.get()).value }
+//!     }
+//! }
+//!
+//! // Create typed Clist from sentinel head.
+//! // SAFETY: head is valid, items are `SampleItemC` with embedded `link` field.
+//! let list = unsafe { clist_create!(&mut head, Item, SampleItemC, link) };
+//!
+//! // Iterate directly over typed items.
+//! let mut found_0 = false;
+//! let mut found_10 = false;
+//! let mut found_20 = false;
+//!
+//! for item in list.iter() {
+//!     let val = item.value();
+//!     if val == 0 { found_0 = true; }
+//!     if val == 10 { found_10 = true; }
+//!     if val == 20 { found_20 = true; }
+//! }
+//!
+//! assert!(found_0 && found_10 && found_20);
+//! ```
+
+use core::{
+    iter::FusedIterator,
+    marker::PhantomData, //
+};
+
+use crate::{
+    bindings,
+    types::Opaque, //
+};
+
+/// Initialize a `list_head` object to point to itself.
+///
+/// # Safety
+///
+/// `list` must be a valid pointer to a `list_head` object.
+#[inline]
+pub unsafe fn init_list_head(list: *mut bindings::list_head) {
+    // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
+    unsafe {
+        (*list).next = list;
+        (*list).prev = list;
+    }
+}
+
+/// Wraps a `list_head` object for use in intrusive linked lists.
+///
+/// # Invariants
+///
+/// - `ClistHead` represents an allocated and valid `list_head` structure.
+///
+/// # Safety
+///
+/// - All `list_head` nodes must not be modified by C code for the lifetime of `ClistHead`.
+#[repr(transparent)]
+pub struct ClistHead(Opaque<bindings::list_head>);
+
+impl ClistHead {
+    /// Create a `&ClistHead` reference from a raw `list_head` pointer.
+    ///
+    /// # Safety
+    ///
+    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure.
+    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
+        // SAFETY:
+        // - `ClistHead` has same layout as `list_head`.
+        // - `ptr` is valid and unmodified for 'a.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Get the raw `list_head` pointer.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::list_head {
+        self.0.get()
+    }
+
+    /// Get the next `ClistHead` in the list.
+    #[inline]
+    pub fn next(&self) -> &Self {
+        let raw = self.as_raw();
+        // SAFETY:
+        // - `self.as_raw()` is valid per type invariants.
+        // - The `next` pointer is guaranteed to be non-NULL.
+        unsafe { Self::from_raw((*raw).next) }
+    }
+
+    /// Get the previous `ClistHead` in the list.
+    #[inline]
+    pub fn prev(&self) -> &Self {
+        let raw = self.as_raw();
+        // SAFETY:
+        // - self.as_raw() is valid per type invariants.
+        // - The `prev` pointer is guaranteed to be non-NULL.
+        unsafe { Self::from_raw((*raw).prev) }
+    }
+
+    /// Check if this node is linked in a list (not isolated).
+    #[inline]
+    pub fn is_linked(&self) -> bool {
+        let raw = self.as_raw();
+        // SAFETY: self.as_raw() is valid per type invariants.
+        unsafe { (*raw).next != raw && (*raw).prev != raw }
+    }
+}
+
+// SAFETY: `ClistHead` can be sent to any thread.
+unsafe impl Send for ClistHead {}
+
+// SAFETY: `ClistHead` can be shared among threads as it is not modified by C per type invariants.
+unsafe impl Sync for ClistHead {}
+
+impl PartialEq for ClistHead {
+    fn eq(&self, other: &Self) -> bool {
+        self.as_raw() == other.as_raw()
+    }
+}
+
+impl Eq for ClistHead {}
+
+/// Low-level iterator over `list_head` nodes.
+///
+/// An iterator used to iterate over a C intrusive linked list (`list_head`). Caller has to
+/// perform conversion of returned `ClistHead` to an item (using `container_of` macro or similar).
+///
+/// # Invariants
+///
+/// `ClistHeadIter` is iterating over an allocated, initialized and valid list.
+struct ClistHeadIter<'a> {
+    current_head: &'a ClistHead,
+    list_head: &'a ClistHead,
+    exhausted: bool,
+}
+
+impl<'a> Iterator for ClistHeadIter<'a> {
+    type Item = &'a ClistHead;
+
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.exhausted {
+            return None;
+        }
+
+        // Advance to next node.
+        self.current_head = self.current_head.next();
+
+        // Check if we've circled back to the sentinel head.
+        if self.current_head == self.list_head {
+            self.exhausted = true;
+            return None;
+        }
+
+        Some(self.current_head)
+    }
+}
+
+impl<'a> FusedIterator for ClistHeadIter<'a> {}
+
+/// A typed C linked list with a sentinel head.
+///
+/// A sentinel head represents the entire linked list and can be used for
+/// iteration over items of type `T`, it is not associated with a specific item.
+///
+/// # Invariants
+///
+/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
+/// - `offset` is the byte offset of the `list_head` field within the C struct that `T` wraps.
+///
+/// # Safety
+///
+/// - All the list's `list_head` nodes must be allocated and have valid next/prev pointers.
+/// - The underlying `list_head` (and entire list) must not be modified by C for the
+///   lifetime 'a of `Clist`.
+pub struct Clist<'a, T> {
+    head: &'a ClistHead,
+    offset: usize,
+    _phantom: PhantomData<&'a T>,
+}
+
+impl<'a, T> Clist<'a, T> {
+    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
+    ///
+    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
+    /// the C struct that `T` wraps.
+    ///
+    /// # Safety
+    ///
+    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
+    ///   representing a list sentinel.
+    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
+    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
+    /// - `T` must be `#[repr(transparent)]` over the C struct.
+    #[inline]
+    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {
+        Self {
+            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
+            head: unsafe { ClistHead::from_raw(ptr) },
+            offset: OFFSET,
+            _phantom: PhantomData,
+        }
+    }
+
+    /// Get the raw sentinel `list_head` pointer.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::list_head {
+        self.head.as_raw()
+    }
+
+    /// Check if the list is empty.
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        let raw = self.as_raw();
+        // SAFETY: self.as_raw() is valid per type invariants.
+        unsafe { (*raw).next == raw }
+    }
+
+    /// Create an iterator over typed items.
+    #[inline]
+    pub fn iter(&self) -> ClistIter<'a, T> {
+        ClistIter {
+            head_iter: ClistHeadIter {
+                current_head: self.head,
+                list_head: self.head,
+                exhausted: false,
+            },
+            offset: self.offset,
+            _phantom: PhantomData,
+        }
+    }
+}
+
+/// High-level iterator over typed list items.
+pub struct ClistIter<'a, T> {
+    head_iter: ClistHeadIter<'a>,
+    offset: usize,
+    _phantom: PhantomData<&'a T>,
+}
+
+impl<'a, T> Iterator for ClistIter<'a, T> {
+    type Item = &'a T;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let head = self.head_iter.next()?;
+
+        // Convert to item using offset.
+        // SAFETY:
+        // - `item_ptr` calculation from `offset` (calculated using offset_of!)
+        //    is valid per invariants.
+        Some(unsafe { &*head.as_raw().cast::<u8>().sub(self.offset).cast::<T>() })
+    }
+}
+
+impl<'a, T> FusedIterator for ClistIter<'a, T> {}
+
+/// Create a C doubly-circular linked list interface `Clist` from a raw `list_head` pointer.
+///
+/// This macro creates a `Clist<T>` that can iterate over items of type `$rust_type` linked
+/// via the `$field` field in the underlying C struct `$c_type`.
+///
+/// # Arguments
+///
+/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
+/// - `$rust_type`: Each item's rust wrapper type.
+/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
+/// - `$field`: The name of the `list_head` field within the C struct.
+///
+/// # Safety
+///
+/// The caller must ensure:
+/// - `$head` is a valid, initialized sentinel `list_head` pointing to a list that remains
+///   unmodified for the lifetime of the rust `Clist`.
+/// - The list contains items of type `$c_type` linked via an embedded `$field`.
+/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
+/// - The macro is called from an unsafe block.
+///
+/// # Examples
+///
+/// Refer to the examples in the [crate::clist] module documentation.
+#[macro_export]
+macro_rules! clist_create {
+    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
+        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
+            { ::core::mem::offset_of!($c_type, $field) },
+        >($head)
+    };
+}
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 v3] rust: clist: Add support to interface with C linked lists

[John Hubbard]

On 11/29/25 1:30 PM, Joel Fernandes wrote:
> Add a new module `clist` for working with C's doubly circular linked
> lists. Provide low-level iteration over list_head nodes and high-level
> iteration over typed list items.
...
> 
>  MAINTAINERS            |   7 +
>  rust/helpers/helpers.c |   1 +
>  rust/helpers/list.c    |  12 ++
>  rust/kernel/clist.rs   | 349 +++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs     |   1 +
>  5 files changed, 370 insertions(+)
>  create mode 100644 rust/helpers/list.c
>  create mode 100644 rust/kernel/clist.rs

Hi Joel,

This is sufficiently tricky that I think it needs some code to exercise it.

Lately I'm not sure what to recommend, there are several choices, each with
trade-offs: kunit, samples/rust, or even new DRM Rust code. Maybe the last
one is especially nice, because it doesn't really have many downsides.

Rather than wait for any of that, I wrote a quick samples/rust/rust_clist.rs
and used it to sanity check my review findings, which are below.

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5f7aa6a8a9a1..fb2ff877b6d1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22666,6 +22666,13 @@ F:	rust/kernel/init.rs
>  F:	rust/pin-init/
>  K:	\bpin-init\b|pin_init\b|PinInit
>  
> +RUST TO C LIST INTERFACES
> +M:	Joel Fernandes <joelagnelf@nvidia.com>
> +M:	Alexandre Courbot <acourbot@nvidia.com>
> +L:	rust-for-linux@vger.kernel.org
> +S:	Maintained
> +F:	rust/kernel/clist.rs
> +
>  RXRPC SOCKETS (AF_RXRPC)
>  M:	David Howells <dhowells@redhat.com>
>  M:	Marc Dionne <marc.dionne@auristor.com>
> 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..6044979c7a2e
> --- /dev/null
> +++ b/rust/helpers/list.c
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Helpers for C Circular doubly linked list implementation.

s/Circular/circular/

...but:

> + */
> +
> +#include <linux/list.h>
> +
> +void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
> +{
> +	list_add_tail(new, head);
> +}

This is, so far, not used. Let's remove it, until/unless you have some
code in this patch(set) to use it, please.

> diff --git a/rust/kernel/clist.rs b/rust/kernel/clist.rs
> new file mode 100644
> index 000000000000..361a6132299b
> --- /dev/null
> +++ b/rust/kernel/clist.rs
> @@ -0,0 +1,349 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A C doubly circular intrusive linked list interface for rust code.
> +//!
> +//! # Examples
> +//!
> +//! ```
> +//! use kernel::{
> +//!     bindings,
> +//!     clist::init_list_head,
> +//!     clist_create,
> +//!     types::Opaque, //
> +//! };
> +//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
> +//! # // emulated here for doctests using the C bindings.
> +//! # use core::mem::MaybeUninit;
> +//! #
> +//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
> +//! # #[repr(C)]
> +//! # pub(crate) struct SampleItemC {
> +//! #     pub value: i32,
> +//! #     pub link: bindings::list_head,
> +//! # }
> +//! #
> +//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
> +//! #
> +//! # // SAFETY: head and all the items are test objects allocated in this scope.
> +//! # unsafe { init_list_head(head.as_mut_ptr()) };
> +//! # // SAFETY: head is a test object allocated in this scope.
> +//! # let mut head = unsafe { head.assume_init() };

This is a bug that leads to a corrupted list. I have the test code (and
the kernel hang/crash) to prove it.

The problem is that any move after init_list_head() invalidates the
list: the next/prev pointers still point to the old address.

The fix requires two-step initialization, like this, for example:

//! # // Two-step init: create uninit first (can be moved), then init after.
//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
//! # let mut items = [
//! #     MaybeUninit::<SampleItemC>::uninit(),
//! #     MaybeUninit::<SampleItemC>::uninit(),
//! #     MaybeUninit::<SampleItemC>::uninit(),
//! # ];
//! #
//! # // Step 2: Now init after they're in their final location
//! # // SAFETY: head is in its final stack location.
//! # unsafe { init_list_head(head.as_mut_ptr()) };

> +//! # let mut items = [
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! # ];
> +//! #
> +//! # for (i, item) in items.iter_mut().enumerate() {
> +//! #     let ptr = item.as_mut_ptr();
> +//! #     // SAFETY: pointers are to allocated test objects with a list_head field.
> +//! #     unsafe {
> +//! #         (*ptr).value = i as i32 * 10;
> +//! #         // addr_of_mut!() computes address of link directly as link is uninitialized.
> +//! #         init_list_head(core::ptr::addr_of_mut!((*ptr).link));
> +//! #         bindings::list_add_tail(&mut (*ptr).link, &mut head);
> +//! #     }
> +//! # }
> +//!
> +//! // Rust wrapper for the C struct.
> +//! // The list item struct in this example is defined in C code as:
> +//! //   struct SampleItemC {
> +//! //       int value;
> +//! //       struct list_head link;
> +//! //   };
> +//! //
> +//! #[repr(transparent)]
> +//! pub(crate) struct Item(Opaque<SampleItemC>);
> +//!
> +//! impl Item {
> +//!     pub(crate) fn value(&self) -> i32 {
> +//!         // SAFETY: `Item` has same layout as `SampleItemC`.
> +//!         unsafe { (*self.0.get()).value }
> +//!     }
> +//! }
> +//!
> +//! // Create typed Clist from sentinel head.
> +//! // SAFETY: head is valid, items are `SampleItemC` with embedded `link` field.
> +//! let list = unsafe { clist_create!(&mut head, Item, SampleItemC, link) };
> +//!
> +//! // Iterate directly over typed items.
> +//! let mut found_0 = false;
> +//! let mut found_10 = false;
> +//! let mut found_20 = false;
> +//!
> +//! for item in list.iter() {
> +//!     let val = item.value();
> +//!     if val == 0 { found_0 = true; }
> +//!     if val == 10 { found_10 = true; }
> +//!     if val == 20 { found_20 = true; }
> +//! }
> +//!
> +//! assert!(found_0 && found_10 && found_20);
> +//! ```
> +
> +use core::{
> +    iter::FusedIterator,
> +    marker::PhantomData, //
> +};
> +
> +use crate::{
> +    bindings,
> +    types::Opaque, //
> +};
> +
> +/// Initialize a `list_head` object to point to itself.
> +///
> +/// # Safety
> +///
> +/// `list` must be a valid pointer to a `list_head` object.
> +#[inline]
> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
> +    // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
> +    unsafe {
> +        (*list).next = list;
> +        (*list).prev = list;
> +    }
> +}
> +
> +/// Wraps a `list_head` object for use in intrusive linked lists.
> +///
> +/// # Invariants
> +///
> +/// - `ClistHead` represents an allocated and valid `list_head` structure.
> +///
> +/// # Safety
> +///
> +/// - All `list_head` nodes must not be modified by C code for the lifetime of `ClistHead`.
> +#[repr(transparent)]
> +pub struct ClistHead(Opaque<bindings::list_head>);
> +
> +impl ClistHead {
> +    /// Create a `&ClistHead` reference from a raw `list_head` pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure.
> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
> +        // SAFETY:
> +        // - `ClistHead` has same layout as `list_head`.
> +        // - `ptr` is valid and unmodified for 'a.
> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    /// Get the raw `list_head` pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::list_head {
> +        self.0.get()
> +    }
> +
> +    /// Get the next `ClistHead` in the list.
> +    #[inline]
> +    pub fn next(&self) -> &Self {
> +        let raw = self.as_raw();
> +        // SAFETY:
> +        // - `self.as_raw()` is valid per type invariants.
> +        // - The `next` pointer is guaranteed to be non-NULL.
> +        unsafe { Self::from_raw((*raw).next) }
> +    }
> +
> +    /// Get the previous `ClistHead` in the list.
> +    #[inline]
> +    pub fn prev(&self) -> &Self {
> +        let raw = self.as_raw();
> +        // SAFETY:
> +        // - self.as_raw() is valid per type invariants.
> +        // - The `prev` pointer is guaranteed to be non-NULL.
> +        unsafe { Self::from_raw((*raw).prev) }
> +    }
> +
> +    /// Check if this node is linked in a list (not isolated).
> +    #[inline]
> +    pub fn is_linked(&self) -> bool {
> +        let raw = self.as_raw();
> +        // SAFETY: self.as_raw() is valid per type invariants.
> +        unsafe { (*raw).next != raw && (*raw).prev != raw }
> +    }
> +}
> +
> +// SAFETY: `ClistHead` can be sent to any thread.
> +unsafe impl Send for ClistHead {}
> +
> +// SAFETY: `ClistHead` can be shared among threads as it is not modified by C per type invariants.
> +unsafe impl Sync for ClistHead {}
> +
> +impl PartialEq for ClistHead {
> +    fn eq(&self, other: &Self) -> bool {
> +        self.as_raw() == other.as_raw()
> +    }
> +}
> +
> +impl Eq for ClistHead {}
> +
> +/// Low-level iterator over `list_head` nodes.
> +///
> +/// An iterator used to iterate over a C intrusive linked list (`list_head`). Caller has to
> +/// perform conversion of returned `ClistHead` to an item (using `container_of` macro or similar).
> +///
> +/// # Invariants
> +///
> +/// `ClistHeadIter` is iterating over an allocated, initialized and valid list.
> +struct ClistHeadIter<'a> {
> +    current_head: &'a ClistHead,
> +    list_head: &'a ClistHead,
> +    exhausted: bool,
> +}
> +
> +impl<'a> Iterator for ClistHeadIter<'a> {
> +    type Item = &'a ClistHead;
> +
> +    #[inline]
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if self.exhausted {
> +            return None;
> +        }
> +
> +        // Advance to next node.
> +        self.current_head = self.current_head.next();
> +
> +        // Check if we've circled back to the sentinel head.
> +        if self.current_head == self.list_head {
> +            self.exhausted = true;
> +            return None;
> +        }
> +
> +        Some(self.current_head)
> +    }
> +}
> +
> +impl<'a> FusedIterator for ClistHeadIter<'a> {}
> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head represents the entire linked list and can be used for
> +/// iteration over items of type `T`, it is not associated with a specific item.
> +///
> +/// # Invariants
> +///
> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
> +/// - `offset` is the byte offset of the `list_head` field within the C struct that `T` wraps.
> +///
> +/// # Safety
> +///
> +/// - All the list's `list_head` nodes must be allocated and have valid next/prev pointers.
> +/// - The underlying `list_head` (and entire list) must not be modified by C for the
> +///   lifetime 'a of `Clist`.
> +pub struct Clist<'a, T> {
> +    head: &'a ClistHead,
> +    offset: usize,
> +    _phantom: PhantomData<&'a T>,
> +}

This discards build-time (const generic) information, and demotes it to 
runtime (.offset), without any real benefit. I believe it's better to keep
it as a const generic, like this:

pub struct Clist<'a, T, const OFFSET: usize> {
    head: &'a ClistHead,
    _phantom: PhantomData<&'a T>,
}

> +
> +impl<'a, T> Clist<'a, T> {

And here, the above becomes:

impl<'a, T, const OFFSET: usize> Clist<'a, T, OFFSET> {

...etc.

I've tested all of this locally, and the diffs look nice and all my tests
still pass, of course.

> +    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
> +    ///
> +    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
> +    /// the C struct that `T` wraps.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
> +    ///   representing a list sentinel.
> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> +    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
> +    /// - `T` must be `#[repr(transparent)]` over the C struct.
> +    #[inline]
> +    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {
> +        Self {
> +            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
> +            head: unsafe { ClistHead::from_raw(ptr) },
> +            offset: OFFSET,
> +            _phantom: PhantomData,
> +        }
> +    }
> +
> +    /// Get the raw sentinel `list_head` pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::list_head {
> +        self.head.as_raw()
> +    }
> +
> +    /// Check if the list is empty.
> +    #[inline]
> +    pub fn is_empty(&self) -> bool {
> +        let raw = self.as_raw();
> +        // SAFETY: self.as_raw() is valid per type invariants.
> +        unsafe { (*raw).next == raw }
> +    }
> +
> +    /// Create an iterator over typed items.
> +    #[inline]
> +    pub fn iter(&self) -> ClistIter<'a, T> {
> +        ClistIter {
> +            head_iter: ClistHeadIter {
> +                current_head: self.head,
> +                list_head: self.head,
> +                exhausted: false,
> +            },
> +            offset: self.offset,
> +            _phantom: PhantomData,
> +        }
> +    }
> +}
> +
> +/// High-level iterator over typed list items.
> +pub struct ClistIter<'a, T> {
> +    head_iter: ClistHeadIter<'a>,
> +    offset: usize,
> +    _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T> Iterator for ClistIter<'a, T> {
> +    type Item = &'a T;
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        let head = self.head_iter.next()?;
> +
> +        // Convert to item using offset.
> +        // SAFETY:
> +        // - `item_ptr` calculation from `offset` (calculated using offset_of!)
> +        //    is valid per invariants.
> +        Some(unsafe { &*head.as_raw().cast::<u8>().sub(self.offset).cast::<T>() })
> +    }
> +}
> +
> +impl<'a, T> FusedIterator for ClistIter<'a, T> {}
> +
> +/// Create a C doubly-circular linked list interface `Clist` from a raw `list_head` pointer.
> +///
> +/// This macro creates a `Clist<T>` that can iterate over items of type `$rust_type` linked
> +/// via the `$field` field in the underlying C struct `$c_type`.
> +///
> +/// # Arguments
> +///
> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
> +/// - `$rust_type`: Each item's rust wrapper type.
> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
> +/// - `$field`: The name of the `list_head` field within the C struct.
> +///
> +/// # Safety
> +///
> +/// The caller must ensure:
> +/// - `$head` is a valid, initialized sentinel `list_head` pointing to a list that remains
> +///   unmodified for the lifetime of the rust `Clist`.
> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
> +/// - The macro is called from an unsafe block.
> +///
> +/// # Examples
> +///
> +/// Refer to the examples in the [crate::clist] module documentation.
> +#[macro_export]
> +macro_rules! clist_create {
> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
> +        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
> +            { ::core::mem::offset_of!($c_type, $field) },
> +        >($head)
> +    };
> +}

Unlike the corresponding C container_of() macro, this one here has no
compile-time verification that the field is actually a list_head.

How about this, to add that check:

--- a/rust/kernel/clist.rs
+++ b/rust/kernel/clist.rs
 macro_rules! clist_create {
-    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
-        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
+    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {{
+        // Compile-time check: $field must be a list_head.
+        const _: () = {
+            let _check: fn(*const $c_type) -> *const $crate::bindings::list_head =
+                |p| unsafe { ::core::ptr::addr_of!((*p).$field) };
+        };
+        $crate::clist::Clist::<$rust_type, { ::core::mem::offset_of!($c_type, $field) }>::from_raw(
             $head,
         )
-    };
+    }};
 }

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

thanks,
-- 
John Hubbard

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Alice Ryhl]

On Sat, Nov 29, 2025 at 04:30:56PM -0500, Joel Fernandes wrote:
> Add a new module `clist` for working with C's doubly circular linked
> lists. Provide low-level iteration over list_head nodes and high-level
> iteration over typed list items.
> 
> Provide a `clist_create` macro to assist in creation of the `Clist` type.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> Thanks to Alex, Daniel, Danilo, John, and Miguel for reviewing v1/v2!
> 
> Changes since v2:
>   - Squashed the 3 patches into a single patch due to dependencies and reducing helpers.
>   - Changed Clist to Clist<'a, T> using const generic offset (Alex).
>   - Simplified C helpers to only list_add_tail (Alex, John).
>   - Added init_list_head() for INIT_LIST_HEAD functionality and dropped it from C helpers (Alex).
>   - Added FusedIterator impl for both ClistHeadIter and ClistIter.
>   - Added PartialEq/Eq for ClistHead (Alex)
>   - Rust style improvements, comment improvements (Daniel).
>   - Added MAINTAINERS entry (Miguel).
> 
> Link to v2: https://lore.kernel.org/all/20251111171315.2196103-2-joelagnelf@nvidia.com/
> Link to v1 (RFC): https://lore.kernel.org/all/20251030190613.1224287-2-joelagnelf@nvidia.com/
> 
>  MAINTAINERS            |   7 +
>  rust/helpers/helpers.c |   1 +
>  rust/helpers/list.c    |  12 ++
>  rust/kernel/clist.rs   | 349 +++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs     |   1 +
>  5 files changed, 370 insertions(+)
>  create mode 100644 rust/helpers/list.c
>  create mode 100644 rust/kernel/clist.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5f7aa6a8a9a1..fb2ff877b6d1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22666,6 +22666,13 @@ F:	rust/kernel/init.rs
>  F:	rust/pin-init/
>  K:	\bpin-init\b|pin_init\b|PinInit
>  
> +RUST TO C LIST INTERFACES
> +M:	Joel Fernandes <joelagnelf@nvidia.com>
> +M:	Alexandre Courbot <acourbot@nvidia.com>
> +L:	rust-for-linux@vger.kernel.org
> +S:	Maintained
> +F:	rust/kernel/clist.rs
> +
>  RXRPC SOCKETS (AF_RXRPC)
>  M:	David Howells <dhowells@redhat.com>
>  M:	Marc Dionne <marc.dionne@auristor.com>
> 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..6044979c7a2e
> --- /dev/null
> +++ b/rust/helpers/list.c
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Helpers for C Circular doubly linked list implementation.
> + */
> +
> +#include <linux/list.h>
> +
> +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..361a6132299b
> --- /dev/null
> +++ b/rust/kernel/clist.rs
> @@ -0,0 +1,349 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A C doubly circular intrusive linked list interface for rust code.
> +//!
> +//! # Examples
> +//!
> +//! ```
> +//! use kernel::{
> +//!     bindings,
> +//!     clist::init_list_head,
> +//!     clist_create,
> +//!     types::Opaque, //
> +//! };
> +//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
> +//! # // emulated here for doctests using the C bindings.
> +//! # use core::mem::MaybeUninit;
> +//! #
> +//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
> +//! # #[repr(C)]
> +//! # pub(crate) struct SampleItemC {
> +//! #     pub value: i32,
> +//! #     pub link: bindings::list_head,
> +//! # }
> +//! #
> +//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
> +//! #
> +//! # // SAFETY: head and all the items are test objects allocated in this scope.
> +//! # unsafe { init_list_head(head.as_mut_ptr()) };
> +//! # // SAFETY: head is a test object allocated in this scope.
> +//! # let mut head = unsafe { head.assume_init() };
> +//! # let mut items = [
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! # ];
> +//! #
> +//! # for (i, item) in items.iter_mut().enumerate() {
> +//! #     let ptr = item.as_mut_ptr();
> +//! #     // SAFETY: pointers are to allocated test objects with a list_head field.
> +//! #     unsafe {
> +//! #         (*ptr).value = i as i32 * 10;
> +//! #         // addr_of_mut!() computes address of link directly as link is uninitialized.
> +//! #         init_list_head(core::ptr::addr_of_mut!((*ptr).link));
> +//! #         bindings::list_add_tail(&mut (*ptr).link, &mut head);
> +//! #     }
> +//! # }
> +//!
> +//! // Rust wrapper for the C struct.
> +//! // The list item struct in this example is defined in C code as:
> +//! //   struct SampleItemC {
> +//! //       int value;
> +//! //       struct list_head link;
> +//! //   };
> +//! //
> +//! #[repr(transparent)]
> +//! pub(crate) struct Item(Opaque<SampleItemC>);
> +//!
> +//! impl Item {
> +//!     pub(crate) fn value(&self) -> i32 {
> +//!         // SAFETY: `Item` has same layout as `SampleItemC`.
> +//!         unsafe { (*self.0.get()).value }
> +//!     }
> +//! }
> +//!
> +//! // Create typed Clist from sentinel head.
> +//! // SAFETY: head is valid, items are `SampleItemC` with embedded `link` field.
> +//! let list = unsafe { clist_create!(&mut head, Item, SampleItemC, link) };
> +//!
> +//! // Iterate directly over typed items.
> +//! let mut found_0 = false;
> +//! let mut found_10 = false;
> +//! let mut found_20 = false;
> +//!
> +//! for item in list.iter() {
> +//!     let val = item.value();
> +//!     if val == 0 { found_0 = true; }
> +//!     if val == 10 { found_10 = true; }
> +//!     if val == 20 { found_20 = true; }
> +//! }
> +//!
> +//! assert!(found_0 && found_10 && found_20);
> +//! ```
> +
> +use core::{
> +    iter::FusedIterator,
> +    marker::PhantomData, //
> +};
> +
> +use crate::{
> +    bindings,
> +    types::Opaque, //
> +};
> +
> +/// Initialize a `list_head` object to point to itself.
> +///
> +/// # Safety
> +///
> +/// `list` must be a valid pointer to a `list_head` object.
> +#[inline]
> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
> +    // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
> +    unsafe {
> +        (*list).next = list;
> +        (*list).prev = list;
> +    }
> +}

It may make sense to move such manual reimplementations into the
bindings crate so that other abstractions take advantage of them by
default when they write bindings::init_list_head.

Of course you can still have a re-export here.

> +/// Wraps a `list_head` object for use in intrusive linked lists.
> +///
> +/// # Invariants
> +///
> +/// - `ClistHead` represents an allocated and valid `list_head` structure.
> +///
> +/// # Safety
> +///
> +/// - All `list_head` nodes must not be modified by C code for the lifetime of `ClistHead`.

So if I modify the list from Rust code, it's okay?

I think the actual requirement you want is just that nobody modifies it.

> +#[repr(transparent)]
> +pub struct ClistHead(Opaque<bindings::list_head>);
> +
> +impl ClistHead {
> +    /// Create a `&ClistHead` reference from a raw `list_head` pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure.
> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
> +        // SAFETY:
> +        // - `ClistHead` has same layout as `list_head`.
> +        // - `ptr` is valid and unmodified for 'a.
> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    /// Get the raw `list_head` pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::list_head {
> +        self.0.get()
> +    }
> +
> +    /// Get the next `ClistHead` in the list.
> +    #[inline]
> +    pub fn next(&self) -> &Self {
> +        let raw = self.as_raw();
> +        // SAFETY:
> +        // - `self.as_raw()` is valid per type invariants.
> +        // - The `next` pointer is guaranteed to be non-NULL.
> +        unsafe { Self::from_raw((*raw).next) }
> +    }
> +
> +    /// Get the previous `ClistHead` in the list.
> +    #[inline]
> +    pub fn prev(&self) -> &Self {
> +        let raw = self.as_raw();
> +        // SAFETY:
> +        // - self.as_raw() is valid per type invariants.
> +        // - The `prev` pointer is guaranteed to be non-NULL.
> +        unsafe { Self::from_raw((*raw).prev) }
> +    }
> +
> +    /// Check if this node is linked in a list (not isolated).
> +    #[inline]
> +    pub fn is_linked(&self) -> bool {
> +        let raw = self.as_raw();
> +        // SAFETY: self.as_raw() is valid per type invariants.
> +        unsafe { (*raw).next != raw && (*raw).prev != raw }
> +    }
> +}
> +
> +// SAFETY: `ClistHead` can be sent to any thread.
> +unsafe impl Send for ClistHead {}
> +
> +// SAFETY: `ClistHead` can be shared among threads as it is not modified by C per type invariants.
> +unsafe impl Sync for ClistHead {}
> +
> +impl PartialEq for ClistHead {
> +    fn eq(&self, other: &Self) -> bool {
> +        self.as_raw() == other.as_raw()
> +    }
> +}
> +
> +impl Eq for ClistHead {}
> +
> +/// Low-level iterator over `list_head` nodes.
> +///
> +/// An iterator used to iterate over a C intrusive linked list (`list_head`). Caller has to
> +/// perform conversion of returned `ClistHead` to an item (using `container_of` macro or similar).
> +///
> +/// # Invariants
> +///
> +/// `ClistHeadIter` is iterating over an allocated, initialized and valid list.
> +struct ClistHeadIter<'a> {
> +    current_head: &'a ClistHead,
> +    list_head: &'a ClistHead,
> +    exhausted: bool,
> +}
> +
> +impl<'a> Iterator for ClistHeadIter<'a> {
> +    type Item = &'a ClistHead;
> +
> +    #[inline]
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if self.exhausted {
> +            return None;
> +        }
> +
> +        // Advance to next node.
> +        self.current_head = self.current_head.next();
> +
> +        // Check if we've circled back to the sentinel head.
> +        if self.current_head == self.list_head {
> +            self.exhausted = true;
> +            return None;
> +        }
> +
> +        Some(self.current_head)
> +    }
> +}
> +
> +impl<'a> FusedIterator for ClistHeadIter<'a> {}
> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head represents the entire linked list and can be used for
> +/// iteration over items of type `T`, it is not associated with a specific item.
> +///
> +/// # Invariants
> +///
> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
> +/// - `offset` is the byte offset of the `list_head` field within the C struct that `T` wraps.
> +///
> +/// # Safety
> +///
> +/// - All the list's `list_head` nodes must be allocated and have valid next/prev pointers.
> +/// - The underlying `list_head` (and entire list) must not be modified by C for the
> +///   lifetime 'a of `Clist`.

Here and elsewhere: We don't generally have Safety sections on structs.
It looks like these should just be invariants.

> +pub struct Clist<'a, T> {
> +    head: &'a ClistHead,
> +    offset: usize,
> +    _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T> Clist<'a, T> {
> +    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
> +    ///
> +    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
> +    /// the C struct that `T` wraps.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
> +    ///   representing a list sentinel.
> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> +    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
> +    /// - `T` must be `#[repr(transparent)]` over the C struct.
> +    #[inline]
> +    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {

I think OFFSET should probably be a constant on the struct rather than a
field.

> +        Self {
> +            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
> +            head: unsafe { ClistHead::from_raw(ptr) },
> +            offset: OFFSET,
> +            _phantom: PhantomData,
> +        }
> +    }
> +
> +    /// Get the raw sentinel `list_head` pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::list_head {
> +        self.head.as_raw()
> +    }
> +
> +    /// Check if the list is empty.
> +    #[inline]
> +    pub fn is_empty(&self) -> bool {
> +        let raw = self.as_raw();
> +        // SAFETY: self.as_raw() is valid per type invariants.
> +        unsafe { (*raw).next == raw }
> +    }
> +
> +    /// Create an iterator over typed items.
> +    #[inline]
> +    pub fn iter(&self) -> ClistIter<'a, T> {
> +        ClistIter {
> +            head_iter: ClistHeadIter {
> +                current_head: self.head,
> +                list_head: self.head,
> +                exhausted: false,
> +            },
> +            offset: self.offset,
> +            _phantom: PhantomData,
> +        }
> +    }
> +}
> +
> +/// High-level iterator over typed list items.
> +pub struct ClistIter<'a, T> {
> +    head_iter: ClistHeadIter<'a>,
> +    offset: usize,
> +    _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T> Iterator for ClistIter<'a, T> {
> +    type Item = &'a T;
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        let head = self.head_iter.next()?;
> +
> +        // Convert to item using offset.
> +        // SAFETY:
> +        // - `item_ptr` calculation from `offset` (calculated using offset_of!)
> +        //    is valid per invariants.
> +        Some(unsafe { &*head.as_raw().cast::<u8>().sub(self.offset).cast::<T>() })

Can be simplified to:
head.as_raw().byte_sub(OFFSET).cast::<T>()

> +    }
> +}
> +
> +impl<'a, T> FusedIterator for ClistIter<'a, T> {}
> +
> +/// Create a C doubly-circular linked list interface `Clist` from a raw `list_head` pointer.
> +///
> +/// This macro creates a `Clist<T>` that can iterate over items of type `$rust_type` linked
> +/// via the `$field` field in the underlying C struct `$c_type`.
> +///
> +/// # Arguments
> +///
> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
> +/// - `$rust_type`: Each item's rust wrapper type.
> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
> +/// - `$field`: The name of the `list_head` field within the C struct.
> +///
> +/// # Safety
> +///
> +/// The caller must ensure:
> +/// - `$head` is a valid, initialized sentinel `list_head` pointing to a list that remains
> +///   unmodified for the lifetime of the rust `Clist`.
> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
> +/// - The macro is called from an unsafe block.
> +///
> +/// # Examples
> +///
> +/// Refer to the examples in the [crate::clist] module documentation.
> +#[macro_export]
> +macro_rules! clist_create {
> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
> +        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
> +            { ::core::mem::offset_of!($c_type, $field) },
> +        >($head)
> +    };
> +}
> 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 v3] rust: clist: Add support to interface with C linked lists

[Daniel Almeida]

> On 29 Nov 2025, at 18:30, Joel Fernandes <joelagnelf@nvidia.com> wrote:
> 
> Add a new module `clist` for working with C's doubly circular linked
> lists. Provide low-level iteration over list_head nodes and high-level
> iteration over typed list items.
> 
> Provide a `clist_create` macro to assist in creation of the `Clist` type.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> Thanks to Alex, Daniel, Danilo, John, and Miguel for reviewing v1/v2!
> 
> Changes since v2:
>  - Squashed the 3 patches into a single patch due to dependencies and reducing helpers.
>  - Changed Clist to Clist<'a, T> using const generic offset (Alex).
>  - Simplified C helpers to only list_add_tail (Alex, John).
>  - Added init_list_head() for INIT_LIST_HEAD functionality and dropped it from C helpers (Alex).
>  - Added FusedIterator impl for both ClistHeadIter and ClistIter.
>  - Added PartialEq/Eq for ClistHead (Alex)
>  - Rust style improvements, comment improvements (Daniel).
>  - Added MAINTAINERS entry (Miguel).
> 
> Link to v2: https://lore.kernel.org/all/20251111171315.2196103-2-joelagnelf@nvidia.com/
> Link to v1 (RFC): https://lore.kernel.org/all/20251030190613.1224287-2-joelagnelf@nvidia.com/
> 
> MAINTAINERS            |   7 +
> rust/helpers/helpers.c |   1 +
> rust/helpers/list.c    |  12 ++
> rust/kernel/clist.rs   | 349 +++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs     |   1 +
> 5 files changed, 370 insertions(+)
> create mode 100644 rust/helpers/list.c
> create mode 100644 rust/kernel/clist.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5f7aa6a8a9a1..fb2ff877b6d1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22666,6 +22666,13 @@ F: rust/kernel/init.rs
> F: rust/pin-init/
> K: \bpin-init\b|pin_init\b|PinInit
> 
> +RUST TO C LIST INTERFACES
> +M: Joel Fernandes <joelagnelf@nvidia.com>
> +M: Alexandre Courbot <acourbot@nvidia.com>
> +L: rust-for-linux@vger.kernel.org
> +S: Maintained
> +F: rust/kernel/clist.rs
> +
> RXRPC SOCKETS (AF_RXRPC)
> M: David Howells <dhowells@redhat.com>
> M: Marc Dionne <marc.dionne@auristor.com>
> 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..6044979c7a2e
> --- /dev/null
> +++ b/rust/helpers/list.c
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Helpers for C Circular doubly linked list implementation.
> + */
> +
> +#include <linux/list.h>
> +
> +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..361a6132299b
> --- /dev/null
> +++ b/rust/kernel/clist.rs
> @@ -0,0 +1,349 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A C doubly circular intrusive linked list interface for rust code.
> +//!
> +//! # Examples
> +//!
> +//! ```
> +//! use kernel::{
> +//!     bindings,
> +//!     clist::init_list_head,
> +//!     clist_create,
> +//!     types::Opaque, //
> +//! };
> +//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
> +//! # // emulated here for doctests using the C bindings.
> +//! # use core::mem::MaybeUninit;
> +//! #
> +//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
> +//! # #[repr(C)]
> +//! # pub(crate) struct SampleItemC {
> +//! #     pub value: i32,
> +//! #     pub link: bindings::list_head,
> +//! # }
> +//! #
> +//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
> +//! #
> +//! # // SAFETY: head and all the items are test objects allocated in this scope.
> +//! # unsafe { init_list_head(head.as_mut_ptr()) };
> +//! # // SAFETY: head is a test object allocated in this scope.
> +//! # let mut head = unsafe { head.assume_init() };
> +//! # let mut items = [
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! #     MaybeUninit::<SampleItemC>::uninit(),
> +//! # ];
> +//! #
> +//! # for (i, item) in items.iter_mut().enumerate() {
> +//! #     let ptr = item.as_mut_ptr();
> +//! #     // SAFETY: pointers are to allocated test objects with a list_head field.
> +//! #     unsafe {
> +//! #         (*ptr).value = i as i32 * 10;
> +//! #         // addr_of_mut!() computes address of link directly as link is uninitialized.
> +//! #         init_list_head(core::ptr::addr_of_mut!((*ptr).link));
> +//! #         bindings::list_add_tail(&mut (*ptr).link, &mut head);
> +//! #     }
> +//! # }
> +//!
> +//! // Rust wrapper for the C struct.
> +//! // The list item struct in this example is defined in C code as:
> +//! //   struct SampleItemC {
> +//! //       int value;
> +//! //       struct list_head link;
> +//! //   };
> +//! //
> +//! #[repr(transparent)]
> +//! pub(crate) struct Item(Opaque<SampleItemC>);
> +//!
> +//! impl Item {
> +//!     pub(crate) fn value(&self) -> i32 {
> +//!         // SAFETY: `Item` has same layout as `SampleItemC`.
> +//!         unsafe { (*self.0.get()).value }
> +//!     }
> +//! }
> +//!
> +//! // Create typed Clist from sentinel head.
> +//! // SAFETY: head is valid, items are `SampleItemC` with embedded `link` field.
> +//! let list = unsafe { clist_create!(&mut head, Item, SampleItemC, link) };
> +//!
> +//! // Iterate directly over typed items.
> +//! let mut found_0 = false;
> +//! let mut found_10 = false;
> +//! let mut found_20 = false;
> +//!
> +//! for item in list.iter() {
> +//!     let val = item.value();
> +//!     if val == 0 { found_0 = true; }
> +//!     if val == 10 { found_10 = true; }
> +//!     if val == 20 { found_20 = true; }
> +//! }
> +//!
> +//! assert!(found_0 && found_10 && found_20);
> +//! ```
> +
> +use core::{
> +    iter::FusedIterator,
> +    marker::PhantomData, //
> +};
> +
> +use crate::{
> +    bindings,
> +    types::Opaque, //
> +};
> +
> +/// Initialize a `list_head` object to point to itself.
> +///
> +/// # Safety
> +///
> +/// `list` must be a valid pointer to a `list_head` object.
> +#[inline]
> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
> +    // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
> +    unsafe {
> +        (*list).next = list;
> +        (*list).prev = list;
> +    }
> +}
> +
> +/// Wraps a `list_head` object for use in intrusive linked lists.
> +///
> +/// # Invariants
> +///
> +/// - `ClistHead` represents an allocated and valid `list_head` structure.
> +///
> +/// # Safety
> +///
> +/// - All `list_head` nodes must not be modified by C code for the lifetime of `ClistHead`.
> +#[repr(transparent)]
> +pub struct ClistHead(Opaque<bindings::list_head>);

I still think we should call this CList. IMHO, it does not make sense to have a
Clist, and a ClistHead (notice the capitalization). CList and CListHead are
easier to read and reason about.

Did anyone push back on this?


> +
> +impl ClistHead {
> +    /// Create a `&ClistHead` reference from a raw `list_head` pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure.
> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
> +        // SAFETY:
> +        // - `ClistHead` has same layout as `list_head`.
> +        // - `ptr` is valid and unmodified for 'a.
> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    /// Get the raw `list_head` pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::list_head {
> +        self.0.get()
> +    }
> +
> +    /// Get the next `ClistHead` in the list.
> +    #[inline]
> +    pub fn next(&self) -> &Self {

> +        let raw = self.as_raw();
> +        // SAFETY:
> +        // - `self.as_raw()` is valid per type invariants.
> +        // - The `next` pointer is guaranteed to be non-NULL.
> +        unsafe { Self::from_raw((*raw).next) }
> +    }
> +
> +    /// Get the previous `ClistHead` in the list.
> +    #[inline]
> +    pub fn prev(&self) -> &Self {

> +        let raw = self.as_raw();
> +        // SAFETY:
> +        // - self.as_raw() is valid per type invariants.
> +        // - The `prev` pointer is guaranteed to be non-NULL.
> +        unsafe { Self::from_raw((*raw).prev) }
> +    }
> +
> +    /// Check if this node is linked in a list (not isolated).
> +    #[inline]
> +    pub fn is_linked(&self) -> bool {
> +        let raw = self.as_raw();
> +        // SAFETY: self.as_raw() is valid per type invariants.
> +        unsafe { (*raw).next != raw && (*raw).prev != raw }
> +    }
> +}
> +
> +// SAFETY: `ClistHead` can be sent to any thread.
> +unsafe impl Send for ClistHead {}
> +
> +// SAFETY: `ClistHead` can be shared among threads as it is not modified by C per type invariants.
> +unsafe impl Sync for ClistHead {}
> +
> +impl PartialEq for ClistHead {
> +    fn eq(&self, other: &Self) -> bool {
> +        self.as_raw() == other.as_raw()
> +    }
> +}
> +
> +impl Eq for ClistHead {}
> +
> +/// Low-level iterator over `list_head` nodes.
> +///
> +/// An iterator used to iterate over a C intrusive linked list (`list_head`). Caller has to
> +/// perform conversion of returned `ClistHead` to an item (using `container_of` macro or similar).
> +///
> +/// # Invariants
> +///
> +/// `ClistHeadIter` is iterating over an allocated, initialized and valid list.
> +struct ClistHeadIter<'a> {
> +    current_head: &'a ClistHead,
> +    list_head: &'a ClistHead,
> +    exhausted: bool,
> +}
> +
> +impl<'a> Iterator for ClistHeadIter<'a> {
> +    type Item = &'a ClistHead;
> +
> +    #[inline]
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if self.exhausted {
> +            return None;
> +        }
> +
> +        // Advance to next node.
> +        self.current_head = self.current_head.next();

> +
> +        // Check if we've circled back to the sentinel head.
> +        if self.current_head == self.list_head {
> +            self.exhausted = true;
> +            return None;
> +        }
> +
> +        Some(self.current_head)
> +    }
> +}
> +
> +impl<'a> FusedIterator for ClistHeadIter<'a> {}
> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head represents the entire linked list and can be used for
> +/// iteration over items of type `T`, it is not associated with a specific item.
> +///
> +/// # Invariants
> +///
> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
> +/// - `offset` is the byte offset of the `list_head` field within the C struct that `T` wraps.
> +///
> +/// # Safety
> +///
> +/// - All the list's `list_head` nodes must be allocated and have valid next/prev pointers.
> +/// - The underlying `list_head` (and entire list) must not be modified by C for the
> +///   lifetime 'a of `Clist`.
> +pub struct Clist<'a, T> {
> +    head: &'a ClistHead,
> +    offset: usize,
> +    _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T> Clist<'a, T> {
> +    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
> +    ///
> +    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
> +    /// the C struct that `T` wraps.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
> +    ///   representing a list sentinel.
> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> +    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
> +    /// - `T` must be `#[repr(transparent)]` over the C struct.
> +    #[inline]
> +    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {
> +        Self {
> +            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
> +            head: unsafe { ClistHead::from_raw(ptr) },
> +            offset: OFFSET,
> +            _phantom: PhantomData,
> +        }
> +    }
> +
> +    /// Get the raw sentinel `list_head` pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::list_head {
> +        self.head.as_raw()
> +    }
> +
> +    /// Check if the list is empty.
> +    #[inline]
> +    pub fn is_empty(&self) -> bool {
> +        let raw = self.as_raw();
> +        // SAFETY: self.as_raw() is valid per type invariants.
> +        unsafe { (*raw).next == raw }
> +    }
> +
> +    /// Create an iterator over typed items.
> +    #[inline]
> +    pub fn iter(&self) -> ClistIter<'a, T> {
> +        ClistIter {
> +            head_iter: ClistHeadIter {
> +                current_head: self.head,
> +                list_head: self.head,
> +                exhausted: false,
> +            },
> +            offset: self.offset,
> +            _phantom: PhantomData,
> +        }
> +    }
> +}
> +
> +/// High-level iterator over typed list items.
> +pub struct ClistIter<'a, T> {
> +    head_iter: ClistHeadIter<'a>,
> +    offset: usize,
> +    _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T> Iterator for ClistIter<'a, T> {
> +    type Item = &'a T;
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        let head = self.head_iter.next()?;
> +
> +        // Convert to item using offset.
> +        // SAFETY:
> +        // - `item_ptr` calculation from `offset` (calculated using offset_of!)
> +        //    is valid per invariants.
> +        Some(unsafe { &*head.as_raw().cast::<u8>().sub(self.offset).cast::<T>() })
> +    }
> +}
> +
> +impl<'a, T> FusedIterator for ClistIter<'a, T> {}
> +
> +/// Create a C doubly-circular linked list interface `Clist` from a raw `list_head` pointer.
> +///
> +/// This macro creates a `Clist<T>` that can iterate over items of type `$rust_type` linked
> +/// via the `$field` field in the underlying C struct `$c_type`.
> +///
> +/// # Arguments
> +///
> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
> +/// - `$rust_type`: Each item's rust wrapper type.
> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
> +/// - `$field`: The name of the `list_head` field within the C struct.
> +///
> +/// # Safety
> +///
> +/// The caller must ensure:
> +/// - `$head` is a valid, initialized sentinel `list_head` pointing to a list that remains
> +///   unmodified for the lifetime of the rust `Clist`.
> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
> +/// - The macro is called from an unsafe block.
> +///
> +/// # Examples
> +///
> +/// Refer to the examples in the [crate::clist] module documentation.

Missing backticks?

> +#[macro_export]
> +macro_rules! clist_create {
> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {

I think this needs a SAFETY comment, or otherwise the linter will complain.

> +        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
> +            { ::core::mem::offset_of!($c_type, $field) },
> +        >($head)
> +    };
> +}
> 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 v3] rust: clist: Add support to interface with C linked lists

[John Hubbard]

On 12/1/25 8:51 AM, Daniel Almeida wrote:
>> On 29 Nov 2025, at 18:30, Joel Fernandes <joelagnelf@nvidia.com> wrote:
...
>> +#[repr(transparent)]
>> +pub struct ClistHead(Opaque<bindings::list_head>);
> 
> I still think we should call this CList. IMHO, it does not make sense to have a

I am guessing you meant to place this comment after Clist, rather than here
(after ClistHead)? Otherwise I don't know what you are actually suggesting?

> Clist, and a ClistHead (notice the capitalization). CList and CListHead are
> easier to read and reason about.
> 
> Did anyone push back on this?
 
If you are simply recommending renaming:
    Clist     --> CList
    ClistHead --> CListHead

...then I'd say "+1" for that suggestion.

thanks,
-- 
John Hubbard

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

On 12/1/2025 2:35 PM, John Hubbard wrote:
> On 12/1/25 8:51 AM, Daniel Almeida wrote:
>>> On 29 Nov 2025, at 18:30, Joel Fernandes <joelagnelf@nvidia.com> wrote:
> ...
>>> +#[repr(transparent)]
>>> +pub struct ClistHead(Opaque<bindings::list_head>);
>>
>> I still think we should call this CList. IMHO, it does not make sense to have a
> 
> I am guessing you meant to place this comment after Clist, rather than here
> (after ClistHead)? Otherwise I don't know what you are actually suggesting?
> 
>> Clist, and a ClistHead (notice the capitalization). CList and CListHead are
>> easier to read and reason about.
>>
>> Did anyone push back on this?
>  
> If you are simply recommending renaming:
>     Clist     --> CList
>     ClistHead --> CListHead
> 
> ...then I'd say "+1" for that suggestion.

I am not fond of the suggestion but I don't oppose it either. I honestly don't
like the triple capitalization with CListHead though. Lets see where all of us
stand and then take a call on it. Opinions?

Thanks.

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

Hi John,

On 11/30/2025 7:34 PM, John Hubbard wrote:
> On 11/29/25 1:30 PM, Joel Fernandes wrote:
>> Add a new module `clist` for working with C's doubly circular linked
>> lists. Provide low-level iteration over list_head nodes and high-level
>> iteration over typed list items.
> ...
>>
>>  MAINTAINERS            |   7 +
>>  rust/helpers/helpers.c |   1 +
>>  rust/helpers/list.c    |  12 ++
>>  rust/kernel/clist.rs   | 349 +++++++++++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs     |   1 +
>>  5 files changed, 370 insertions(+)
>>  create mode 100644 rust/helpers/list.c
>>  create mode 100644 rust/kernel/clist.rs
> 
> Hi Joel,
> 
> This is sufficiently tricky that I think it needs some code to exercise it.
> 
> Lately I'm not sure what to recommend, there are several choices, each with
> trade-offs: kunit, samples/rust, or even new DRM Rust code. Maybe the last
> one is especially nice, because it doesn't really have many downsides.
> 
> Rather than wait for any of that, I wrote a quick samples/rust/rust_clist.rs
> and used it to sanity check my review findings, which are below.

In v1, I had a samples/rust/ patch, but everyone's opinion almost unanimously
was this does not belong in a sample, but rather in doctests. What in the sample
is not supported by the current doctest? If something is missing, I think I can
add it in. Plus yes, DRM_BUDDY is going to be a consumer shortly.

>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5f7aa6a8a9a1..fb2ff877b6d1 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -22666,6 +22666,13 @@ F:	rust/kernel/init.rs
>>  F:	rust/pin-init/
>>  K:	\bpin-init\b|pin_init\b|PinInit
>>  
>> +RUST TO C LIST INTERFACES
>> +M:	Joel Fernandes <joelagnelf@nvidia.com>
>> +M:	Alexandre Courbot <acourbot@nvidia.com>
>> +L:	rust-for-linux@vger.kernel.org
>> +S:	Maintained
>> +F:	rust/kernel/clist.rs
>> +
>>  RXRPC SOCKETS (AF_RXRPC)
>>  M:	David Howells <dhowells@redhat.com>
>>  M:	Marc Dionne <marc.dionne@auristor.com>
>> 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..6044979c7a2e
>> --- /dev/null
>> +++ b/rust/helpers/list.c
>> @@ -0,0 +1,12 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/*
>> + * Helpers for C Circular doubly linked list implementation.
> 
> s/Circular/circular/
> 
> ...but:
> 
>> + */
>> +
>> +#include <linux/list.h>
>> +
>> +void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
>> +{
>> +	list_add_tail(new, head);
>> +}
> 
> This is, so far, not used. Let's remove it, until/unless you have some
> code in this patch(set) to use it, please.

Did you try to remove it and build the doctest? :)

  CC      rust/doctests_kernel_generated_kunit.o
error[E0425]: cannot find function `list_add_tail` in crate `bindings`
     --> rust/doctests_kernel_generated.rs:3619:19
      |
3619  |         bindings::list_add_tail(&mut (*ptr).link, head);

>> diff --git a/rust/kernel/clist.rs b/rust/kernel/clist.rs
>> new file mode 100644
>> index 000000000000..361a6132299b
>> --- /dev/null
>> +++ b/rust/kernel/clist.rs
>> @@ -0,0 +1,349 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! A C doubly circular intrusive linked list interface for rust code.
>> +//!
>> +//! # Examples
>> +//!
>> +//! ```
>> +//! use kernel::{
>> +//!     bindings,
>> +//!     clist::init_list_head,
>> +//!     clist_create,
>> +//!     types::Opaque, //
>> +//! };
>> +//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
>> +//! # // emulated here for doctests using the C bindings.
>> +//! # use core::mem::MaybeUninit;
>> +//! #
>> +//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
>> +//! # #[repr(C)]
>> +//! # pub(crate) struct SampleItemC {
>> +//! #     pub value: i32,
>> +//! #     pub link: bindings::list_head,
>> +//! # }
>> +//! #
>> +//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
>> +//! #
>> +//! # // SAFETY: head and all the items are test objects allocated in this scope.
>> +//! # unsafe { init_list_head(head.as_mut_ptr()) };
>> +//! # // SAFETY: head is a test object allocated in this scope.
>> +//! # let mut head = unsafe { head.assume_init() };
> 
> This is a bug that leads to a corrupted list. I have the test code (and
> the kernel hang/crash) to prove it.

Good find, actually it is a bug only in the example (the list construction in
your sample should be coming from C code, not rust - this code here is just for
doctest setup). That said, see below for fix.

> The problem is that any move after init_list_head() invalidates the
> list: the next/prev pointers still point to the old address.
> 
> The fix requires two-step initialization, like this, for example:

It has nothing to do with 2-step initialization. The issue is only related to
the HEAD (and not the items) right? The issue is `assume_init()` should not be
used on self-referential structures, the fix just one line:

-//! # unsafe { init_list_head(head.as_mut_ptr()) };
-//! # let mut head = unsafe { head.assume_init() };

+//! # let head = head.as_mut_ptr();
+//! # unsafe { init_list_head(head) };

Does that fix the issue in your private sample test too?

Or did I miss what you're suggesting?

> 
> //! # // Two-step init: create uninit first (can be moved), then init after.
> //! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
> //! # let mut items = [
> //! #     MaybeUninit::<SampleItemC>::uninit(),
> //! #     MaybeUninit::<SampleItemC>::uninit(),
> //! #     MaybeUninit::<SampleItemC>::uninit(),
> //! # ];
> //! #
> //! # // Step 2: Now init after they're in their final location
> //! # // SAFETY: head is in its final stack location.
> //! # unsafe { init_list_head(head.as_mut_ptr()) };

Until the items are added, the items have nothing to do with the head. So I am
not sure why you have to order it this way.

> 
>> +//! # let mut items = [
>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>> +//! # ];
>> +//! #
>> +//! # for (i, item) in items.iter_mut().enumerate() {
>> +//! #     let ptr = item.as_mut_ptr();
>> +//! #     // SAFETY: pointers are to allocated test objects with a list_head field.
>> +//! #     unsafe {
>> +//! #         (*ptr).value = i as i32 * 10;
>> +//! #         // addr_of_mut!() computes address of link directly as link is uninitialized.
>> +//! #         init_list_head(core::ptr::addr_of_mut!((*ptr).link));
>> +//! #         bindings::list_add_tail(&mut (*ptr).link, &mut head);
>> +//! #     }
>> +//! # }
>> +//!
>> +//! // Rust wrapper for the C struct.
>> +//! // The list item struct in this example is defined in C code as:
>> +//! //   struct SampleItemC {
>> +//! //       int value;
>> +//! //       struct list_head link;
>> +//! //   };
>> +//! //
>> +//! #[repr(transparent)]
>> +//! pub(crate) struct Item(Opaque<SampleItemC>);
>> +//!
>> +//! impl Item {
>> +//!     pub(crate) fn value(&self) -> i32 {
>> +//!         // SAFETY: `Item` has same layout as `SampleItemC`.
>> +//!         unsafe { (*self.0.get()).value }
>> +//!     }
>> +//! }
>> +//!
>> +//! // Create typed Clist from sentinel head.
>> +//! // SAFETY: head is valid, items are `SampleItemC` with embedded `link` field.
>> +//! let list = unsafe { clist_create!(&mut head, Item, SampleItemC, link) };
>> +//!
>> +//! // Iterate directly over typed items.
>> +//! let mut found_0 = false;
>> +//! let mut found_10 = false;
>> +//! let mut found_20 = false;
>> +//!
>> +//! for item in list.iter() {
>> +//!     let val = item.value();
>> +//!     if val == 0 { found_0 = true; }
>> +//!     if val == 10 { found_10 = true; }
>> +//!     if val == 20 { found_20 = true; }
>> +//! }
>> +//!
>> +//! assert!(found_0 && found_10 && found_20);
>> +//! ```
>> +

[...]

>> +impl<'a> Iterator for ClistHeadIter<'a> {
>> +    type Item = &'a ClistHead;
>> +
>> +    #[inline]
>> +    fn next(&mut self) -> Option<Self::Item> {
>> +        if self.exhausted {
>> +            return None;
>> +        }
>> +
>> +        // Advance to next node.
>> +        self.current_head = self.current_head.next();
>> +
>> +        // Check if we've circled back to the sentinel head.
>> +        if self.current_head == self.list_head {
>> +            self.exhausted = true;
>> +            return None;
>> +        }
>> +
>> +        Some(self.current_head)
>> +    }
>> +}
>> +
>> +impl<'a> FusedIterator for ClistHeadIter<'a> {}
>> +
>> +/// A typed C linked list with a sentinel head.
>> +///
>> +/// A sentinel head represents the entire linked list and can be used for
>> +/// iteration over items of type `T`, it is not associated with a specific item.
>> +///
>> +/// # Invariants
>> +///
>> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
>> +/// - `offset` is the byte offset of the `list_head` field within the C struct that `T` wraps.
>> +///
>> +/// # Safety
>> +///
>> +/// - All the list's `list_head` nodes must be allocated and have valid next/prev pointers.
>> +/// - The underlying `list_head` (and entire list) must not be modified by C for the
>> +///   lifetime 'a of `Clist`.
>> +pub struct Clist<'a, T> {
>> +    head: &'a ClistHead,
>> +    offset: usize,
>> +    _phantom: PhantomData<&'a T>,
>> +}
> 
> This discards build-time (const generic) information, and demotes it to 
> runtime (.offset), without any real benefit. I believe it's better to keep
> it as a const generic, like this:
> 
> pub struct Clist<'a, T, const OFFSET: usize> {
>     head: &'a ClistHead,
>     _phantom: PhantomData<&'a T>,
> }
> 
>> +
>> +impl<'a, T> Clist<'a, T> {
> 
> And here, the above becomes:
> 
> impl<'a, T, const OFFSET: usize> Clist<'a, T, OFFSET> {
> 
> ...etc.

It is not ignored, the const-generic part only applies to the constructor method
in my patch. I didn't want to add another argument to the diamond brackets, the
type name looks really ugly then.

The only advantage I think of doing this (inspite of the obvious aesthetic
disadvantage) is that a mutable `Clist` cannot have its offset modified. Let me
see if I can get Alice's suggestion to make it a const in the struct work to
solve that.

[..]
>> +impl<'a, T> FusedIterator for ClistIter<'a, T> {}
>> +
>> +/// Create a C doubly-circular linked list interface `Clist` from a raw `list_head` pointer.
>> +///
>> +/// This macro creates a `Clist<T>` that can iterate over items of type `$rust_type` linked
>> +/// via the `$field` field in the underlying C struct `$c_type`.
>> +///
>> +/// # Arguments
>> +///
>> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
>> +/// - `$rust_type`: Each item's rust wrapper type.
>> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
>> +/// - `$field`: The name of the `list_head` field within the C struct.
>> +///
>> +/// # Safety
>> +///
>> +/// The caller must ensure:
>> +/// - `$head` is a valid, initialized sentinel `list_head` pointing to a list that remains
>> +///   unmodified for the lifetime of the rust `Clist`.
>> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
>> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
>> +/// - The macro is called from an unsafe block.
>> +///
>> +/// # Examples
>> +///
>> +/// Refer to the examples in the [crate::clist] module documentation.
>> +#[macro_export]
>> +macro_rules! clist_create {
>> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
>> +        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
>> +            { ::core::mem::offset_of!($c_type, $field) },
>> +        >($head)
>> +    };
>> +}
> 
> Unlike the corresponding C container_of() macro, this one here has no
> compile-time verification that the field is actually a list_head.
> 
> How about this, to add that check:
> 
> --- a/rust/kernel/clist.rs
> +++ b/rust/kernel/clist.rs
>  macro_rules! clist_create {
> -    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
> -        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {{
> +        // Compile-time check: $field must be a list_head.
> +        const _: () = {
> +            let _check: fn(*const $c_type) -> *const $crate::bindings::list_head =
> +                |p| unsafe { ::core::ptr::addr_of!((*p).$field) };
> +        };
> +        $crate::clist::Clist::<$rust_type, { ::core::mem::offset_of!($c_type, $field) }>::from_raw(
>              $head,
>          )
> -    };
> +    }};
Sure I will play with your suggested snippet and add that, thanks.

 - Joel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

On 12/1/2025 3:32 PM, Joel Fernandes wrote:
> On 11/30/2025 7:34 PM, John Hubbard wrote:

[...]

>>> +/// Refer to the examples in the [crate::clist] module documentation.
>>> +#[macro_export]
>>> +macro_rules! clist_create {
>>> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
>>> +        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
>>> +            { ::core::mem::offset_of!($c_type, $field) },
>>> +        >($head)
>>> +    };
>>> +}
>>
>> Unlike the corresponding C container_of() macro, this one here has no
>> compile-time verification that the field is actually a list_head.
>>
>> How about this, to add that check:
>>
>> --- a/rust/kernel/clist.rs
>> +++ b/rust/kernel/clist.rs
>>  macro_rules! clist_create {
>> -    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
>> -        $crate::clist::Clist::<$rust_type>::from_raw_and_offset::<
>> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {{
>> +        // Compile-time check: $field must be a list_head.
>> +        const _: () = {
>> +            let _check: fn(*const $c_type) -> *const $crate::bindings::list_head =
>> +                |p| unsafe { ::core::ptr::addr_of!((*p).$field) };
>> +        };
>> +        $crate::clist::Clist::<$rust_type, { ::core::mem::offset_of!($c_type, $field) }>::from_raw(
>>              $head,
>>          )
>> -    };
>> +    }};
>
> Sure I will play with your suggested snippet and add that, thanks.
I think it works also without the 'const _:' block:
        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
            |p| unsafe { ::core::ptr::addr_of!((*p).$field) };

I will roll that in, thanks John.

 - Joel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

On 12/1/2025 10:25 AM, Alice Ryhl wrote:
> On Sat, Nov 29, 2025 at 04:30:56PM -0500, Joel Fernandes wrote:
>> Add a new module `clist` for working with C's doubly circular linked
>> lists. Provide low-level iteration over list_head nodes and high-level
>> iteration over typed list items.
>>
>> Provide a `clist_create` macro to assist in creation of the `Clist` type.
>>
[...]
>> +//! // Rust wrapper for the C struct.
>> +//! // The list item struct in this example is defined in C code as:
>> +//! //   struct SampleItemC {
>> +//! //       int value;
>> +//! //       struct list_head link;
>> +//! //   };
>> +//! //
>> +//! #[repr(transparent)]
>> +//! pub(crate) struct Item(Opaque<SampleItemC>);
>> +//!
>> +//! impl Item {
>> +//!     pub(crate) fn value(&self) -> i32 {
>> +//!         // SAFETY: `Item` has same layout as `SampleItemC`.
>> +//!         unsafe { (*self.0.get()).value }
>> +//!     }
>> +//! }
>> +//!
>> +//! // Create typed Clist from sentinel head.
>> +//! // SAFETY: head is valid, items are `SampleItemC` with embedded `link` field.
>> +//! let list = unsafe { clist_create!(&mut head, Item, SampleItemC, link) };
>> +//!
>> +//! // Iterate directly over typed items.
>> +//! let mut found_0 = false;
>> +//! let mut found_10 = false;
>> +//! let mut found_20 = false;
>> +//!
>> +//! for item in list.iter() {
>> +//!     let val = item.value();
>> +//!     if val == 0 { found_0 = true; }
>> +//!     if val == 10 { found_10 = true; }
>> +//!     if val == 20 { found_20 = true; }
>> +//! }
>> +//!
>> +//! assert!(found_0 && found_10 && found_20);
>> +//! ```
>> +
>> +use core::{
>> +    iter::FusedIterator,
>> +    marker::PhantomData, //
>> +};
>> +
>> +use crate::{
>> +    bindings,
>> +    types::Opaque, //
>> +};
>> +
>> +/// Initialize a `list_head` object to point to itself.
>> +///
>> +/// # Safety
>> +///
>> +/// `list` must be a valid pointer to a `list_head` object.
>> +#[inline]
>> +pub unsafe fn init_list_head(list: *mut bindings::list_head) {
>> +    // SAFETY: Caller guarantees `list` is a valid pointer to a `list_head`.
>> +    unsafe {
>> +        (*list).next = list;
>> +        (*list).prev = list;
>> +    }
>> +}
> 
> It may make sense to move such manual reimplementations into the
> bindings crate so that other abstractions take advantage of them by
> default when they write bindings::init_list_head.
> 
> Of course you can still have a re-export here.
> 
Where would we make this change, do you mean to rust/helpers/ and have it come
into the bindings that (like i did in v2)? Or do you mean move this function as
it is to bindings/lib.rs?

I am Ok with either.

>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>> +///
>> +/// # Invariants
>> +///
>> +/// - `ClistHead` represents an allocated and valid `list_head` structure.
>> +///
>> +/// # Safety
>> +///
>> +/// - All `list_head` nodes must not be modified by C code for the lifetime of `ClistHead`.
> 
> So if I modify the list from Rust code, it's okay?
> 
> I think the actual requirement you want is just that nobody modifies it.

Yeah you're right, I will change the phrasing.

> 
>> +#[repr(transparent)]
>> +pub struct ClistHead(Opaque<bindings::list_head>);
>> +
>> +impl ClistHead {
>> +    /// Create a `&ClistHead` reference from a raw `list_head` pointer.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure.
>> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>> +    #[inline]
>> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
>> +        // SAFETY:
>> +        // - `ClistHead` has same layout as `list_head`.
>> +        // - `ptr` is valid and unmodified for 'a.
>> +        unsafe { &*ptr.cast() }
>> +    }
>> +
>> +    /// Get the raw `list_head` pointer.
>> +    #[inline]
>> +    pub fn as_raw(&self) -> *mut bindings::list_head {
>> +        self.0.get()
>> +    }
>> +
>> +    /// Get the next `ClistHead` in the list.
>> +    #[inline]
>> +    pub fn next(&self) -> &Self {
>> +        let raw = self.as_raw();
>> +        // SAFETY:
>> +        // - `self.as_raw()` is valid per type invariants.
>> +        // - The `next` pointer is guaranteed to be non-NULL.
>> +        unsafe { Self::from_raw((*raw).next) }
>> +    }
>> +
>> +    /// Get the previous `ClistHead` in the list.
>> +    #[inline]
>> +    pub fn prev(&self) -> &Self {
>> +        let raw = self.as_raw();
>> +        // SAFETY:
>> +        // - self.as_raw() is valid per type invariants.
>> +        // - The `prev` pointer is guaranteed to be non-NULL.
>> +        unsafe { Self::from_raw((*raw).prev) }
>> +    }
>> +
>> +    /// Check if this node is linked in a list (not isolated).
>> +    #[inline]
>> +    pub fn is_linked(&self) -> bool {
>> +        let raw = self.as_raw();
>> +        // SAFETY: self.as_raw() is valid per type invariants.
>> +        unsafe { (*raw).next != raw && (*raw).prev != raw }
>> +    }
>> +}
>> +
>> +// SAFETY: `ClistHead` can be sent to any thread.
>> +unsafe impl Send for ClistHead {}
>> +
>> +// SAFETY: `ClistHead` can be shared among threads as it is not modified by C per type invariants.
>> +unsafe impl Sync for ClistHead {}
>> +
>> +impl PartialEq for ClistHead {
>> +    fn eq(&self, other: &Self) -> bool {
>> +        self.as_raw() == other.as_raw()
>> +    }
>> +}
>> +

[...]

>> +impl<'a> FusedIterator for ClistHeadIter<'a> {}
>> +
>> +/// A typed C linked list with a sentinel head.
>> +///
>> +/// A sentinel head represents the entire linked list and can be used for
>> +/// iteration over items of type `T`, it is not associated with a specific item.
>> +///
>> +/// # Invariants
>> +///
>> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
>> +/// - `offset` is the byte offset of the `list_head` field within the C struct that `T` wraps.
>> +///
>> +/// # Safety
>> +///
>> +/// - All the list's `list_head` nodes must be allocated and have valid next/prev pointers.
>> +/// - The underlying `list_head` (and entire list) must not be modified by C for the
>> +///   lifetime 'a of `Clist`.
> 
> Here and elsewhere: We don't generally have Safety sections on structs.
> It looks like these should just be invariants.

I see, Ok I will move it, thanks.

> 
>> +pub struct Clist<'a, T> {
>> +    head: &'a ClistHead,
>> +    offset: usize,
>> +    _phantom: PhantomData<&'a T>,
>> +}
>> +
>> +impl<'a, T> Clist<'a, T> {
>> +    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
>> +    ///
>> +    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
>> +    /// the C struct that `T` wraps.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>> +    ///   representing a list sentinel.
>> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>> +    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>> +    /// - `T` must be `#[repr(transparent)]` over the C struct.
>> +    #[inline]
>> +    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {
> 
> I think OFFSET should probably be a constant on the struct rather than a
> field.

Yes John suggested this too. The type signature becomes complex/ugly (and you
guys know my hatred for const generic syntax :)). But ok, since this is mostly
hidden by a macro, I will make the change :).

> 
>> +        Self {
>> +            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>> +            head: unsafe { ClistHead::from_raw(ptr) },
>> +            offset: OFFSET,
>> +            _phantom: PhantomData,
>> +        }
>> +    }
>> +
>> +    /// Get the raw sentinel `list_head` pointer.
>> +    #[inline]
>> +    pub fn as_raw(&self) -> *mut bindings::list_head {
>> +        self.head.as_raw()
>> +    }
>> +
>> +    /// Check if the list is empty.
>> +    #[inline]
>> +    pub fn is_empty(&self) -> bool {
>> +        let raw = self.as_raw();
>> +        // SAFETY: self.as_raw() is valid per type invariants.
>> +        unsafe { (*raw).next == raw }
>> +    }
>> +
>> +    /// Create an iterator over typed items.
>> +    #[inline]
>> +    pub fn iter(&self) -> ClistIter<'a, T> {
>> +        ClistIter {
>> +            head_iter: ClistHeadIter {
>> +                current_head: self.head,
>> +                list_head: self.head,
>> +                exhausted: false,
>> +            },
>> +            offset: self.offset,
>> +            _phantom: PhantomData,
>> +        }
>> +    }
>> +}
>> +
>> +/// High-level iterator over typed list items.
>> +pub struct ClistIter<'a, T> {
>> +    head_iter: ClistHeadIter<'a>,
>> +    offset: usize,
>> +    _phantom: PhantomData<&'a T>,
>> +}
>> +
>> +impl<'a, T> Iterator for ClistIter<'a, T> {
>> +    type Item = &'a T;
>> +
>> +    fn next(&mut self) -> Option<Self::Item> {
>> +        let head = self.head_iter.next()?;
>> +
>> +        // Convert to item using offset.
>> +        // SAFETY:
>> +        // - `item_ptr` calculation from `offset` (calculated using offset_of!)
>> +        //    is valid per invariants.
>> +        Some(unsafe { &*head.as_raw().cast::<u8>().sub(self.offset).cast::<T>() })
> 
> Can be simplified to:
> head.as_raw().byte_sub(OFFSET).cast::<T>()
> 

Fixed, thanks.

 - Joel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

Hi Daniel,

On 12/1/2025 11:51 AM, Daniel Almeida wrote:
> 
[...]
>> +/// Create a C doubly-circular linked list interface `Clist` from a raw `list_head` pointer.
>> +///
>> +/// This macro creates a `Clist<T>` that can iterate over items of type `$rust_type` linked
>> +/// via the `$field` field in the underlying C struct `$c_type`.
>> +///
>> +/// # Arguments
>> +///
>> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
>> +/// - `$rust_type`: Each item's rust wrapper type.
>> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
>> +/// - `$field`: The name of the `list_head` field within the C struct.
>> +///
>> +/// # Safety
>> +///
>> +/// The caller must ensure:
>> +/// - `$head` is a valid, initialized sentinel `list_head` pointing to a list that remains
>> +///   unmodified for the lifetime of the rust `Clist`.
>> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
>> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
>> +/// - The macro is called from an unsafe block.
>> +///
>> +/// # Examples
>> +///
>> +/// Refer to the examples in the [crate::clist] module documentation.
> 
> Missing backticks?
> 

will fix, thanks.

>> +#[macro_export]
>> +macro_rules! clist_create {
>> +    ($head:expr, $rust_type:ty, $c_type:ty, $field:ident) => {
> 
> I think this needs a SAFETY comment, or otherwise the linter will complain.
> 
This is intentional, the SAFETY comes from the caller. This is exactly the
container_of! macro pattern too.

Instead, like container_of, we have a safety header above:

/// # Safety

thanks,

 - Joel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[John Hubbard]

On 12/1/25 12:32 PM, Joel Fernandes wrote:
> On 11/30/2025 7:34 PM, John Hubbard wrote:
>> On 11/29/25 1:30 PM, Joel Fernandes wrote:
...
>> This is sufficiently tricky that I think it needs some code to exercise it.
>>
>> Lately I'm not sure what to recommend, there are several choices, each with
>> trade-offs: kunit, samples/rust, or even new DRM Rust code. Maybe the last
>> one is especially nice, because it doesn't really have many downsides.
>>
>> Rather than wait for any of that, I wrote a quick samples/rust/rust_clist.rs
>> and used it to sanity check my review findings, which are below.
> 
> In v1, I had a samples/rust/ patch, but everyone's opinion almost unanimously
> was this does not belong in a sample, but rather in doctests. What in the sample
> is not supported by the current doctest? If something is missing, I think I can
> add it in. Plus yes, DRM_BUDDY is going to be a consumer shortly.

Well, I won't contest the choice of doctests, since wiser heads than mine
have already worked through the tradeoffs.

But for API developers, the problem with doctests is that no one has ever
actually *run* the code. It's just a build test. And so critical bugs, such
as the kernel crash/hang below, are missed.

I would humbly suggest that you build and *run* your own samples code, for
new code that has no users yet.

Because if you are skipping steps like this (posting the code before
there is an actual caller), then the documentation of how to use it
is not "just documentation" anymore--it really needs to run correctly.

And actually, after writing the above...I still think it would be better
to post this with its first caller (DRM_BUDDY, or BUDDY_DRM_ALUMNI, or
however it ends up), so that we can see how it looks and behaves in
practice.

What's the rush?

...
>> The fix requires two-step initialization, like this, for example:
> 
> It has nothing to do with 2-step initialization. The issue is only related to
> the HEAD (and not the items) right? The issue is `assume_init()` should not be
> used on self-referential structures, the fix just one line:
> 
> -//! # unsafe { init_list_head(head.as_mut_ptr()) };
> -//! # let mut head = unsafe { head.assume_init() };
> 
> +//! # let head = head.as_mut_ptr();
> +//! # unsafe { init_list_head(head) };
> 
> Does that fix the issue in your private sample test too?
> 
> Or did I miss what you're suggesting?
> 

Yes, you are correct: the main point is to avoid moving a struct
that contains self-referential fields. So your version is a more
accurate and better fix.

...
>>> +pub struct Clist<'a, T> {
>>> +    head: &'a ClistHead,
>>> +    offset: usize,
>>> +    _phantom: PhantomData<&'a T>,
>>> +}
>>
>> This discards build-time (const generic) information, and demotes it to 
>> runtime (.offset), without any real benefit. I believe it's better to keep
>> it as a const generic, like this:
>>
>> pub struct Clist<'a, T, const OFFSET: usize> {
>>     head: &'a ClistHead,
>>     _phantom: PhantomData<&'a T>,
>> }
>>
>>> +
>>> +impl<'a, T> Clist<'a, T> {
>>
>> And here, the above becomes:
>>
>> impl<'a, T, const OFFSET: usize> Clist<'a, T, OFFSET> {
>>
>> ...etc.
> 
> It is not ignored, the const-generic part only applies to the constructor method
> in my patch. I didn't want to add another argument to the diamond brackets, the
> type name looks really ugly then.
>

The macro hides it, though. Users never have to write the full type.

Increasing const-ness is worth something. The messy syntax is unfortunate,
but I don't really know what to say there. 

 
> The only advantage I think of doing this (inspite of the obvious aesthetic
> disadvantage) is that a mutable `Clist` cannot have its offset modified. Let me
> see if I can get Alice's suggestion to make it a const in the struct work to
> solve that.

Yes. I have it working locally, so I'm confident that you will prevail. :)


thanks,
-- 
John Hubbard

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

On 12/1/2025 5:17 PM, John Hubbard wrote:
> On 12/1/25 12:32 PM, Joel Fernandes wrote:
>> On 11/30/2025 7:34 PM, John Hubbard wrote:
>>> On 11/29/25 1:30 PM, Joel Fernandes wrote:
> ...
>>> This is sufficiently tricky that I think it needs some code to exercise it.
>>>
>>> Lately I'm not sure what to recommend, there are several choices, each with
>>> trade-offs: kunit, samples/rust, or even new DRM Rust code. Maybe the last
>>> one is especially nice, because it doesn't really have many downsides.
>>>
>>> Rather than wait for any of that, I wrote a quick samples/rust/rust_clist.rs
>>> and used it to sanity check my review findings, which are below.
>>
>> In v1, I had a samples/rust/ patch, but everyone's opinion almost unanimously
>> was this does not belong in a sample, but rather in doctests. What in the sample
>> is not supported by the current doctest? If something is missing, I think I can
>> add it in. Plus yes, DRM_BUDDY is going to be a consumer shortly.
> 
> Well, I won't contest the choice of doctests, since wiser heads than mine
> have already worked through the tradeoffs.
> 
> But for API developers, the problem with doctests is that no one has ever
> actually *run* the code. It's just a build test. And so critical bugs, such
> as the kernel crash/hang below, are missed.

You may want to read [1]. CONFIG_RUST_KERNEL_DOCTESTS are run at runtime. You
enable it and boot the kernel. The documentation clearly says "doctests get
compiled as Rust kernel objects, allowing them to run against a built kernel.".
And this is how I have run it as well.

[1] https://docs.kernel.org/rust/testing.html

This also explains why you think list_add_tail() is a noop in my patch, which it
is not.

> 
> I would humbly suggest that you build and *run* your own samples code, for
> new code that has no users yet.

Yes, I already have an internal tree running it. :) I am not sure why the
assume_init() triggered for you but not for me, I don't think has anything to do
with doctests since the doctests is in fact just rust code compiled as KUNIT tests.

> Because if you are skipping steps like this (posting the code before
> there is an actual caller), then the documentation of how to use it
> is not "just documentation" anymore--it really needs to run correctly.

No, that's the thing, these are run. You really are in the wrong here and appear
to not understand how doctests work.

> And actually, after writing the above...I still think it would be better
> to post this with its first caller (DRM_BUDDY, or BUDDY_DRM_ALUMNI, or
> however it ends up), so that we can see how it looks and behaves in
> practice.
> 
> What's the rush?

Who said anything about a rush? I am really confused by what you mean. It is
useful to post patches even if there are external dependencies to get feedback.
So this is also an invalid review comment unfortunately. There is no rush, this
is v3 now, did you miss that?

Thanks.

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Miguel Ojeda]

On Mon, Dec 1, 2025 at 11:18 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> But for API developers, the problem with doctests is that no one has ever
> actually *run* the code. It's just a build test. And so critical bugs, such
> as the kernel crash/hang below, are missed.

No, it is not just a build test. Doctests are actually transformed
into KUnit tests automatically, and `assert!`s inside them actually
report back to KUnit too.

It was a major hack to implement, but I am proud of that one :)

That also allows us to build them with Clippy, to e.g. get the safety
comments right, which is something normal Rust doesn't support yet.

We are actually getting a feature from upstream Rust to support it all
in a stable and nice way!

Cheers,
Miguel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[John Hubbard]

On 12/1/25 2:43 PM, Joel Fernandes wrote:
> On 12/1/2025 5:17 PM, John Hubbard wrote:
>> On 12/1/25 12:32 PM, Joel Fernandes wrote:
>>> On 11/30/2025 7:34 PM, John Hubbard wrote:
>>>> On 11/29/25 1:30 PM, Joel Fernandes wrote:
>> ...
> You may want to read [1]. CONFIG_RUST_KERNEL_DOCTESTS are run at runtime. You
> enable it and boot the kernel. The documentation clearly says "doctests get
> compiled as Rust kernel objects, allowing them to run against a built kernel.".
> And this is how I have run it as well.
> 
> [1] https://docs.kernel.org/rust/testing.html
> 
> This also explains why you think list_add_tail() is a noop in my patch, which it
> is not.

Yes, I forgot that they are actually run, you are right.

> 
>>
>> I would humbly suggest that you build and *run* your own samples code, for
>> new code that has no users yet.
> 
> Yes, I already have an internal tree running it. :) I am not sure why the
> assume_init() triggered for you but not for me, I don't think has anything to do
> with doctests since the doctests is in fact just rust code compiled as KUNIT tests.

I think it's because I wrote separate code that was not a doctest, and
that code is naturally different from however the doctest exercised it.
But it is a good question.

> 
>> Because if you are skipping steps like this (posting the code before
>> there is an actual caller), then the documentation of how to use it
>> is not "just documentation" anymore--it really needs to run correctly.
> 
> No, that's the thing, these are run. You really are in the wrong here and appear
> to not understand how doctests work.

That's a reasonable statement. :)

> 
>> And actually, after writing the above...I still think it would be better
>> to post this with its first caller (DRM_BUDDY, or BUDDY_DRM_ALUMNI, or
>> however it ends up), so that we can see how it looks and behaves in
>> practice.
>>
>> What's the rush?
> 
> Who said anything about a rush? I am really confused by what you mean. It is
> useful to post patches even if there are external dependencies to get feedback.
> So this is also an invalid review comment unfortunately. There is no rush, this
> is v3 now, did you miss that?
> 

I mean, doctests are far weaker than actual code that uses the new API.
It feels rushed to propose merging code without a caller. And I don't
think doctests are a "real enough" caller.

thanks,
-- 
John Hubbard

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[John Hubbard]

On 12/1/25 2:50 PM, Miguel Ojeda wrote:
> On Mon, Dec 1, 2025 at 11:18 PM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> But for API developers, the problem with doctests is that no one has ever
>> actually *run* the code. It's just a build test. And so critical bugs, such
>> as the kernel crash/hang below, are missed.
> 
> No, it is not just a build test. Doctests are actually transformed
> into KUnit tests automatically, and `assert!`s inside them actually
> report back to KUnit too.

I somehow failed to really internalize the connection to KUnit (or
to even remember it). Joel pointed it out too.

> 
> It was a major hack to implement, but I am proud of that one :)
> 
> That also allows us to build them with Clippy, to e.g. get the safety
> comments right, which is something normal Rust doesn't support yet.
> 
> We are actually getting a feature from upstream Rust to support it all
> in a stable and nice way!
> 



thanks,
-- 
John Hubbard

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Daniel Almeida]

> On 1 Dec 2025, at 16:35, John Hubbard <jhubbard@nvidia.com> wrote:
> 
> On 12/1/25 8:51 AM, Daniel Almeida wrote:
>>> On 29 Nov 2025, at 18:30, Joel Fernandes <joelagnelf@nvidia.com> wrote:
> ...
>>> +#[repr(transparent)]
>>> +pub struct ClistHead(Opaque<bindings::list_head>);
>> 
>> I still think we should call this CList. IMHO, it does not make sense to have a
> 
> I am guessing you meant to place this comment after Clist, rather than here
> (after ClistHead)? Otherwise I don't know what you are actually suggesting?
> 
>> Clist, and a ClistHead (notice the capitalization). CList and CListHead are
>> easier to read and reason about.
>> 
>> Did anyone push back on this?
> 
> If you are simply recommending renaming:
>    Clist     --> CList
>    ClistHead --> CListHead

Yes, this is what I meant.

> 
> ...then I'd say "+1" for that suggestion.
> 
> thanks,
> -- 
> John Hubbard
> 
> 

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Miguel Ojeda]

On Mon, Dec 1, 2025 at 11:43 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> The documentation clearly says "doctests get
> compiled as Rust kernel objects, allowing them to run against a built kernel.".

Yeah, there is also a Python script that KUnit provides that some
people like to use too, and quite convenient to pass certain configs etc.

Cheers,
Miguel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Miguel Ojeda]

On Mon, Dec 1, 2025 at 11:55 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> Yes, this is what I meant.

Sounds good to me too, but if enough people like otherwise, it is not
a big deal either way.

Cheers,
Miguel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Daniel Almeida]

> On 1 Dec 2025, at 17:06, Joel Fernandes <joelagnelf@nvidia.com> wrote:
> 
> 
> 
> On 12/1/2025 2:35 PM, John Hubbard wrote:
>> On 12/1/25 8:51 AM, Daniel Almeida wrote:
>>>> On 29 Nov 2025, at 18:30, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>> ...
>>>> +#[repr(transparent)]
>>>> +pub struct ClistHead(Opaque<bindings::list_head>);
>>> 
>>> I still think we should call this CList. IMHO, it does not make sense to have a
>> 
>> I am guessing you meant to place this comment after Clist, rather than here
>> (after ClistHead)? Otherwise I don't know what you are actually suggesting?
>> 
>>> Clist, and a ClistHead (notice the capitalization). CList and CListHead are
>>> easier to read and reason about.
>>> 
>>> Did anyone push back on this?
>> 
>> If you are simply recommending renaming:
>>    Clist     --> CList
>>    ClistHead --> CListHead
>> 
>> ...then I'd say "+1" for that suggestion.
> 
> I am not fond of the suggestion but I don't oppose it either. I honestly don't
> like the triple capitalization with CListHead though. Lets see where all of us
> stand and then take a call on it. Opinions?
> 
> Thanks.

Well, there are three things at play:

C, List, Head

So I think that the CListHead capitalization correctly describes it. IMHO
it’s hard to see “C” and “List” if you spell it
“Clist”, i.e.: it’s easier to read this as a single word and wonder
what’s that for a few seconds.

This is a bit of a nitpick though, so feel free to keep the old spelling if
there is no consensus here.

— Daniel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

On 12/1/2025 5:52 PM, John Hubbard wrote:
>>> And actually, after writing the above...I still think it would be better
>>> to post this with its first caller (DRM_BUDDY, or BUDDY_DRM_ALUMNI, or
>>> however it ends up), so that we can see how it looks and behaves in
>>> practice.
>>>
>>> What's the rush?
>> Who said anything about a rush? I am really confused by what you mean. It is
>> useful to post patches even if there are external dependencies to get feedback.
>> So this is also an invalid review comment unfortunately. There is no rush, this
>> is v3 now, did you miss that?
>>
> I mean, doctests are far weaker than actual code that uses the new API.
> It feels rushed to propose merging code without a caller. And I don't
> think doctests are a "real enough" caller.

Actually I was already rebasing my DRM buddy bindings patches today. So the next
version was already going to be with the actual DRM buddy bindings (inclusive of
the clist patches), now that clist has mostly settled. The point of posting the
clist series was to focus on just that part and get it right. If you notice, my
first version included the DRM buddy user as well but clist required a lot of
changes first.

I don't think one needs to include all users in a series if the series is
sufficiently complex (as long as you posted the user or share a tree using it -
which I already did in the v1). Then the maintainers can decide if it needs to
be pulled in advance or with the user. That's really up to a maintainer. I
certainly want clist to merged only once the drm buddy bindings go with it - why
else would we want to do that? There is absolutely no reason. I am unable to
find where you go the idea that I was proposing merging clist without the drm
buddy bindings - there is little reason to do that considering clist.rs is
mostly independent of other things and is really easy to rebase.

thanks,

 - Joel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[John Hubbard]

On 12/1/25 3:09 PM, Joel Fernandes wrote:
> On 12/1/2025 5:52 PM, John Hubbard wrote:
>>>> And actually, after writing the above...I still think it would be better
>>>> to post this with its first caller (DRM_BUDDY, or BUDDY_DRM_ALUMNI, or
>>>> however it ends up), so that we can see how it looks and behaves in
>>>> practice.
>>>>
>>>> What's the rush?
>>> Who said anything about a rush? I am really confused by what you mean. It is
>>> useful to post patches even if there are external dependencies to get feedback.
>>> So this is also an invalid review comment unfortunately. There is no rush, this
>>> is v3 now, did you miss that?
>>>
>> I mean, doctests are far weaker than actual code that uses the new API.
>> It feels rushed to propose merging code without a caller. And I don't
>> think doctests are a "real enough" caller.
> 
> Actually I was already rebasing my DRM buddy bindings patches today. So the next
> version was already going to be with the actual DRM buddy bindings (inclusive of
> the clist patches), now that clist has mostly settled. The point of posting the
> clist series was to focus on just that part and get it right. If you notice, my
> first version included the DRM buddy user as well but clist required a lot of
> changes first.

Excellent!

> 
> I don't think one needs to include all users in a series if the series is
> sufficiently complex (as long as you posted the user or share a tree using it -
> which I already did in the v1). Then the maintainers can decide if it needs to
> be pulled in advance or with the user. That's really up to a maintainer. I
> certainly want clist to merged only once the drm buddy bindings go with it - why

Probably good to say that directly, in a cover letter.

> else would we want to do that? There is absolutely no reason. I am unable to
> find where you go the idea that I was proposing merging clist without the drm
> buddy bindings - there is little reason to do that considering clist.rs is
> mostly independent of other things and is really easy to rebase.
>

Just the fact that these are non-RFC patches.

thanks,
John Hubbard

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

On 12/1/2025 6:15 PM, John Hubbard wrote:
> On 12/1/25 3:09 PM, Joel Fernandes wrote:

[..]

>> else would we want to do that? There is absolutely no reason. I am unable to
>> find where you go the idea that I was proposing merging clist without the drm
>> buddy bindings - there is little reason to do that considering clist.rs is
>> mostly independent of other things and is really easy to rebase.
>>
> 
> Just the fact that these are non-RFC patches.
On my part, I'll try to do better with marking things as RFC etc. Thanks!

 - Joel

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

On 12/1/2025 6:01 PM, Daniel Almeida wrote:
> 
> 
>> On 1 Dec 2025, at 17:06, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>>
>>
>> On 12/1/2025 2:35 PM, John Hubbard wrote:
>>> On 12/1/25 8:51 AM, Daniel Almeida wrote:
>>>>> On 29 Nov 2025, at 18:30, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>> ...
>>>>> +#[repr(transparent)]
>>>>> +pub struct ClistHead(Opaque<bindings::list_head>);
>>>>
>>>> I still think we should call this CList. IMHO, it does not make sense to have a
>>>
>>> I am guessing you meant to place this comment after Clist, rather than here
>>> (after ClistHead)? Otherwise I don't know what you are actually suggesting?
>>>
>>>> Clist, and a ClistHead (notice the capitalization). CList and CListHead are
>>>> easier to read and reason about.
>>>>
>>>> Did anyone push back on this?
>>>
>>> If you are simply recommending renaming:
>>>    Clist     --> CList
>>>    ClistHead --> CListHead
>>>
>>> ...then I'd say "+1" for that suggestion.
>>
>> I am not fond of the suggestion but I don't oppose it either. I honestly don't
>> like the triple capitalization with CListHead though. Lets see where all of us
>> stand and then take a call on it. Opinions?
>>
>> Thanks.
> 
> Well, there are three things at play:
> 
> C, List, Head
> 
> So I think that the CListHead capitalization correctly describes it. IMHO
> it’s hard to see “C” and “List” if you spell it
> “Clist”, i.e.: it’s easier to read this as a single word and wonder
> what’s that for a few seconds.
> 
> This is a bit of a nitpick though, so feel free to keep the old spelling if
> there is no consensus here.

No problem, I'll change it to CList/CListHead considering John and Miguel also
felt this is better. So I am the odd one out here :)

Thanks.

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Alexandre Courbot]

On Sun Nov 30, 2025 at 6:30 AM JST, Joel Fernandes wrote:
<snip>
> +/// Low-level iterator over `list_head` nodes.
> +///
> +/// An iterator used to iterate over a C intrusive linked list (`list_head`). Caller has to
> +/// perform conversion of returned `ClistHead` to an item (using `container_of` macro or similar).
> +///
> +/// # Invariants
> +///
> +/// `ClistHeadIter` is iterating over an allocated, initialized and valid list.
> +struct ClistHeadIter<'a> {
> +    current_head: &'a ClistHead,
> +    list_head: &'a ClistHead,
> +    exhausted: bool,

Mmm, I suspect `exhausted` is not needed at all - please see below.

> +}
> +
> +impl<'a> Iterator for ClistHeadIter<'a> {
> +    type Item = &'a ClistHead;
> +
> +    #[inline]
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if self.exhausted {
> +            return None;
> +        }
> +
> +        // Advance to next node.
> +        self.current_head = self.current_head.next();
> +
> +        // Check if we've circled back to the sentinel head.
> +        if self.current_head == self.list_head {
> +            self.exhausted = true;
> +            return None;
> +        }
> +
> +        Some(self.current_head)
> +    }

IIUC you could just rewrite this as

    let next = self.current_head.next();
    if next == self.list_head {
        None
    } else {
        self.current_head = next;
        Some(self.current_head)
    }

and drop `exhausted` altogether.

> +}
> +
> +impl<'a> FusedIterator for ClistHeadIter<'a> {}

Is there a reason for not implementing `DoubleEndedIterator` as well?

> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head represents the entire linked list and can be used for
> +/// iteration over items of type `T`, it is not associated with a specific item.
> +///
> +/// # Invariants
> +///
> +/// - `head` is an allocated and valid C `list_head` structure that is the list's sentinel.
> +/// - `offset` is the byte offset of the `list_head` field within the C struct that `T` wraps.
> +///
> +/// # Safety
> +///
> +/// - All the list's `list_head` nodes must be allocated and have valid next/prev pointers.
> +/// - The underlying `list_head` (and entire list) must not be modified by C for the
> +///   lifetime 'a of `Clist`.
> +pub struct Clist<'a, T> {
> +    head: &'a ClistHead,
> +    offset: usize,

Joining the chorus suggesting to move `offset` to a const generic. Not
only will it make the struct smaller, it is also better because now
every single C `list_head` becomes its own `Clist` type, allowing you to
discriminate between them in the type system! Not that we will ever make
use of that, but still! :)

In my review of v2 I also suggested to use a closure as the generic
argument, and that the offset case could be a specialization, but
looking at this version I realize this was overengineered - just using
the offset should be enough for all cases, and is much more elegant, so
that looks like the right call indeed.

> +    _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T> Clist<'a, T> {
> +    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
> +    ///
> +    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
> +    /// the C struct that `T` wraps.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
> +    ///   representing a list sentinel.
> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> +    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
> +    /// - `T` must be `#[repr(transparent)]` over the C struct.
> +    #[inline]
> +    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {
> +        Self {
> +            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
> +            head: unsafe { ClistHead::from_raw(ptr) },
> +            offset: OFFSET,
> +            _phantom: PhantomData,
> +        }
> +    }
> +
> +    /// Get the raw sentinel `list_head` pointer.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::list_head {
> +        self.head.as_raw()
> +    }
> +
> +    /// Check if the list is empty.
> +    #[inline]
> +    pub fn is_empty(&self) -> bool {
> +        let raw = self.as_raw();
> +        // SAFETY: self.as_raw() is valid per type invariants.
> +        unsafe { (*raw).next == raw }
> +    }
> +
> +    /// Create an iterator over typed items.
> +    #[inline]
> +    pub fn iter(&self) -> ClistIter<'a, T> {
> +        ClistIter {
> +            head_iter: ClistHeadIter {
> +                current_head: self.head,
> +                list_head: self.head,
> +                exhausted: false,
> +            },
> +            offset: self.offset,
> +            _phantom: PhantomData,
> +        }
> +    }
> +}
> +
> +/// High-level iterator over typed list items.
> +pub struct ClistIter<'a, T> {
> +    head_iter: ClistHeadIter<'a>,
> +    offset: usize,

Now that Clist's offset has moved to a const generic, let's do the same
here as well.

Overall I think this version looks pretty clean! A nice,
easy to understand wrapper over the C API.

Re: [PATCH v3] rust: clist: Add support to interface with C linked lists

[Joel Fernandes]

Hi Alex,

On 12/3/2025 8:06 AM, Alexandre Courbot wrote:
[..]
>> +}
>> +
>> +impl<'a> Iterator for ClistHeadIter<'a> {
>> +    type Item = &'a ClistHead;
>> +
>> +    #[inline]
>> +    fn next(&mut self) -> Option<Self::Item> {
>> +        if self.exhausted {
>> +            return None;
>> +        }
>> +
>> +        // Advance to next node.
>> +        self.current_head = self.current_head.next();
>> +
>> +        // Check if we've circled back to the sentinel head.
>> +        if self.current_head == self.list_head {
>> +            self.exhausted = true;
>> +            return None;
>> +        }
>> +
>> +        Some(self.current_head)
>> +    }
> 
> IIUC you could just rewrite this as
> 
>     let next = self.current_head.next();
>     if next == self.list_head {
>         None
>     } else {
>         self.current_head = next;
>         Some(self.current_head)
>     }
> 
> and drop `exhausted` altogether.
> 

Yes, this is better, changed, thanks!
>> +    _phantom: PhantomData<&'a T>,
>> +}
>> +
>> +impl<'a, T> Clist<'a, T> {
>> +    /// Create a typed `Clist` from a raw sentinel `list_head` pointer.
>> +    ///
>> +    /// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
>> +    /// the C struct that `T` wraps.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>> +    ///   representing a list sentinel.
>> +    /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>> +    /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>> +    /// - `T` must be `#[repr(transparent)]` over the C struct.
>> +    #[inline]
>> +    pub unsafe fn from_raw_and_offset<const OFFSET: usize>(ptr: *mut bindings::list_head) -> Self {
>> +        Self {
>> +            // SAFETY: Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>> +            head: unsafe { ClistHead::from_raw(ptr) },
>> +            offset: OFFSET,
>> +            _phantom: PhantomData,
>> +        }
>> +    }
>> +
>> +    /// Get the raw sentinel `list_head` pointer.
>> +    #[inline]
>> +    pub fn as_raw(&self) -> *mut bindings::list_head {
>> +        self.head.as_raw()
>> +    }
>> +
>> +    /// Check if the list is empty.
>> +    #[inline]
>> +    pub fn is_empty(&self) -> bool {
>> +        let raw = self.as_raw();
>> +        // SAFETY: self.as_raw() is valid per type invariants.
>> +        unsafe { (*raw).next == raw }
>> +    }
>> +
>> +    /// Create an iterator over typed items.
>> +    #[inline]
>> +    pub fn iter(&self) -> ClistIter<'a, T> {
>> +        ClistIter {
>> +            head_iter: ClistHeadIter {
>> +                current_head: self.head,
>> +                list_head: self.head,
>> +                exhausted: false,
>> +            },
>> +            offset: self.offset,
>> +            _phantom: PhantomData,
>> +        }
>> +    }
>> +}
>> +
>> +/// High-level iterator over typed list items.
>> +pub struct ClistIter<'a, T> {
>> +    head_iter: ClistHeadIter<'a>,
>> +    offset: usize,
> 
> Now that Clist's offset has moved to a const generic, let's do the same
> here as well.

Yes, done.

> Overall I think this version looks pretty clean! A nice,
> easy to understand wrapper over the C API.

Indeed, it has come out to be very nice. Simple design and clean code. Thanks
for all the reviews! I will shoot for posting this patch with the DRM buddy
series together today. There is also a pin initialization patch for `Clist` that
I will separately add to the series on top of this patch.

thanks,

 - Joel