I saw that v10 has been sent, but since the code is mostly identical this review should still apply (I was in the middle of writing it before going to sleep).
On Wed Feb 11, 2026 at 8:32 AM JST, Joel Fernandes wrote: > Add a new module `clist` for working with C's doubly circular linked > lists. Provide low-level iteration over list nodes. > > Typed iteration over actual items is provided with a `clist_create` > macro to assist in creation of the `CList` type. > > Reviewed-by: Daniel Almeida <[email protected]> > Acked-by: Gary Guo <[email protected]> > Signed-off-by: Joel Fernandes <[email protected]> > --- > MAINTAINERS | 7 + > rust/helpers/helpers.c | 1 + > rust/helpers/list.c | 17 +++ > rust/kernel/clist.rs | 320 +++++++++++++++++++++++++++++++++++++++++ > rust/kernel/lib.rs | 1 + > 5 files changed, 346 insertions(+) > create mode 100644 rust/helpers/list.c > create mode 100644 rust/kernel/clist.rs > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7cfb766112cd..b0050b478dc9 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -23205,6 +23205,13 @@ S: Maintained > T: git https://github.com/Rust-for-Linux/linux.git rust-analyzer-next > F: scripts/generate_rust_analyzer.py > > +RUST TO C LIST INTERFACES > +M: Joel Fernandes <[email protected]> > +M: Alexandre Courbot <[email protected]> > +L: [email protected] > +S: Maintained > +F: rust/kernel/clist.rs > + > RXRPC SOCKETS (AF_RXRPC) > M: David Howells <[email protected]> > M: Marc Dionne <[email protected]> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > index a3c42e51f00a..724fcb8240ac 100644 > --- a/rust/helpers/helpers.c > +++ b/rust/helpers/helpers.c > @@ -35,6 +35,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..4c1f9c111ec8 > --- /dev/null > +++ b/rust/helpers/list.c > @@ -0,0 +1,17 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * Helpers for C Circular doubly linked list implementation. nit: "Circular" doesn't need a capital "C". > + */ > + > +#include <linux/list.h> > + > +__rust_helper void rust_helper_INIT_LIST_HEAD(struct list_head *list) > +{ > + INIT_LIST_HEAD(list); > +} > + > +__rust_helper 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..8aa72b5d54be > --- /dev/null > +++ b/rust/kernel/clist.rs > @@ -0,0 +1,320 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! A C doubly circular intrusive linked list interface for rust code. > +//! > +//! # Examples > +//! > +//! ``` > +//! use kernel::{ > +//! bindings, > +//! 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 struct SampleItemC { > +//! # pub value: i32, > +//! # pub link: bindings::list_head, > +//! # } > +//! # > +//! # let mut head = MaybeUninit::<bindings::list_head>::uninit(); > +//! # > +//! # let head = head.as_mut_ptr(); > +//! # // SAFETY: head and all the items are test objects allocated in this > scope. > +//! # unsafe { bindings::INIT_LIST_HEAD(head) }; > +//! # > +//! # 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; > +//! # // &raw mut computes address of link directly as link is > uninitialized. > +//! # bindings::INIT_LIST_HEAD(&raw mut (*ptr).link); > +//! # bindings::list_add_tail(&mut (*ptr).link, head); Is the `INIT_LIST_HEAD` line needed? `list_add_tail` will immediately overwrite the initial values of `ptr.link`. > +//! # } > +//! # } > +//! > +//! // 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 struct Item(Opaque<SampleItemC>); > +//! > +//! impl Item { > +//! pub 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!(head, Item, SampleItemC, link) }; `head` appears in the documentation for the first time here - it would help to make at least its declaration visible, or add a comment specifying its type. > +//! > +//! // 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, // > +}; > + > +use pin_init::{ > + pin_data, > + pin_init, > + PinInit // > +}; > + > +/// Wraps a `list_head` object for use in intrusive linked lists. > +/// > +/// # Invariants > +/// > +/// - [`CListHead`] represents an allocated and valid `list_head` structure. > +#[pin_data] > +#[repr(transparent)] > +pub struct CListHead { > + #[pin] > + inner: 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`. > + /// - The list and all linked `list_head` nodes must not be modified by > non-Rust code > + /// 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 per caller guarantees. > + unsafe { &*ptr.cast() } > + } > + > + /// Get the raw `list_head` pointer. > + #[inline] > + pub fn as_raw(&self) -> *mut bindings::list_head { > + self.inner.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. Safety nit: the SAFETY comment should mention what guarantees that `next` is non-NULL. Also the safety section of `from_raw` has 3 items, but this only justifies 2. > + unsafe { Self::from_raw((*raw).next) } > + } > + > + /// 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 } > + } This will return true on a non-empty sentinel head - is that intended? Since this is only used in `CList::is_empty`, and in the GPU buddy's `is_empty` method, is this the right name? The code also mirrors C's `list_empty`... > + > + /// Pin-initializer that initializes the list head. > + pub fn new() -> impl PinInit<Self> { > + pin_init!(Self { > + // SAFETY: `INIT_LIST_HEAD` initializes `slot` to a valid empty > list. > + inner <- Opaque::ffi_init(|slot| unsafe { > bindings::INIT_LIST_HEAD(slot) }), > + }) > + } > +} > + > +// SAFETY: [`CListHead`] can be sent to any thread. > +unsafe impl Send for CListHead {} That safety comment is circular. I guess you mean to say that `list_head` can be sent to any thread? > + > +// SAFETY: [`CListHead`] can be shared among threads as it is not modified > +// by non-Rust code per safety requirements of [`CListHead::from_raw`]. > +unsafe impl Sync for CListHead {} > + > +impl PartialEq for CListHead { > + #[inline] > + fn eq(&self, other: &Self) -> bool { > + core::ptr::eq(self, other) > + } > +} > + > +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 position in the list. > + current: &'a CListHead, > + /// The sentinel head (used to detect end of iteration). > + sentinel: &'a CListHead, > +} > + > +impl<'a> Iterator for CListHeadIter<'a> { > + type Item = &'a CListHead; > + > + #[inline] > + fn next(&mut self) -> Option<Self::Item> { > + // Check if we've reached the sentinel (end of list). > + if self.current == self.sentinel { > + return None; > + } > + > + let item = self.current; > + self.current = item.next(); > + Some(item) > + } > +} > + > +impl<'a> FusedIterator for CListHeadIter<'a> {} I asked this a couple of times ([1], [2]) but got no reply, so let me try again. :) Given that `list_head` is doubly-linked, can we also implement `DoubleEndedIterator`? This can be done in a follow-up patch but should be there eventually as C lists are often parsed in both directions. [1] https://lore.kernel.org/all/[email protected]/ [2] https://lore.kernel.org/all/[email protected]/ > + > +/// 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. > +/// > +/// The const generic `OFFSET` specifies the byte offset of the `list_head` > field within > +/// the struct that `T` wraps. > +/// > +/// # Invariants > +/// > +/// - The [`CListHead`] is an allocated and valid sentinel C `list_head` > structure. > +/// - `OFFSET` is the byte offset of the `list_head` field within the struct > that `T` wraps. > +/// - All the list's `list_head` nodes are allocated and have valid > next/prev pointers. > +#[repr(transparent)] > +pub struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>); > + > +impl<T, const OFFSET: usize> CList<T, OFFSET> { > + /// Create a typed [`CList`] reference from a raw sentinel `list_head` > pointer. > + /// > + /// # 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<'a>(ptr: *mut bindings::list_head) -> &'a Self { > + // SAFETY: > + // - [`CList`] has same layout as [`CListHead`] due to > repr(transparent). > + // - Caller guarantees `ptr` is a valid, sentinel `list_head` object. > + unsafe { &*ptr.cast() } > + } IIUC you can call `CListHead::from_raw` here instead of repeating its code. > + > + /// Check if the list is empty. > + #[inline] > + pub fn is_empty(&self) -> bool { > + !self.0.is_linked() > + } > + > + /// Create an iterator over typed items. > + #[inline] > + pub fn iter(&self) -> CListIter<'_, T, OFFSET> { > + let head = &self.0; > + CListIter { > + head_iter: CListHeadIter { > + current: head.next(), > + sentinel: head, > + }, > + _phantom: PhantomData, > + } > + } > +} > + > +/// High-level iterator over typed list items. > +pub struct CListIter<'a, T, const OFFSET: usize> { > + head_iter: CListHeadIter<'a>, > + _phantom: PhantomData<&'a T>, > +} > + > +impl<'a, T, const OFFSET: usize> Iterator for CListIter<'a, T, OFFSET> { > + type Item = &'a T; > + > + fn next(&mut self) -> Option<Self::Item> { This method is the only one not marked `#[inline]`. > + 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().byte_sub(OFFSET).cast::<T>() }) > + } > +} > + > +impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> > {} > + > +/// Create a C doubly-circular linked list interface `CList` from a raw > `list_head` pointer. > +/// > +/// This macro creates a `CList<T, OFFSET>` 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 > +/// > +/// This is an unsafe macro. 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. > +/// > +/// # Examples > +/// > +/// Refer to the examples in this module's documentation. > +#[macro_export] > +macro_rules! clist_create { > + ($head:expr, $rust_type:ty, $c_type:ty, $($field:tt).+) => {{ > + // Compile-time check that field path is a list_head. > + let _: fn(*const $c_type) -> *const $crate::bindings::list_head = > + |p| &raw const (*p).$($field).+; > + > + // Calculate offset and create `CList`. > + const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+); > + $crate::clist::CList::<$rust_type, OFFSET>::from_raw($head) > + }}; > +} I find it difficult to remember which argument fits where in the macro's syntax (I have the same problem every time I need to use `list_for_each_entry_safe` for instance). Since Rust macros let us be more creative, how about something like this: let list = unsafe { clist_create!(head => Item, SampleItemC.link) }; It makes it more obvious that we use `head` to iterate through a list of `Item`s, using the `link` member of `SampleItemC`. There are probably better syntaxes; the above is just an example.
