Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Mar 19, 2026 at 12:39 PM CET, Gary Guo wrote:
> On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
>> Add a new module `kernel::interop::list` 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.
>>
>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Gary Guo <gary@garyguo.net>
>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>  MAINTAINERS                 |   8 +
>>  rust/helpers/helpers.c      |   1 +
>>  rust/helpers/list.c         |  17 ++
>>  rust/kernel/interop.rs      |   9 +
>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs          |   2 +
>>  6 files changed, 379 insertions(+)
>>  create mode 100644 rust/helpers/list.c
>>  create mode 100644 rust/kernel/interop.rs
>>  create mode 100644 rust/kernel/interop/list.rs
>>
>> +/// 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
>> +///
>> +/// The caller must ensure:
>> +///
>> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
>> +///   pointing to a list that is not concurrently modified for the lifetime of the [`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 the [`crate::interop::list`] module documentation.
>> +#[macro_export]
>> +macro_rules! clist_create {
>> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
>> +        // Compile-time check that field path is a `list_head`.
>> +        // SAFETY: `p` is a valid pointer to `$c_type`.
>> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
>> +            |p| unsafe { &raw const (*p).$($field).+ };
>
> Actually, this check is insufficient, you should create a reference instead
> (just in case people put this inside `repr(packed)`.
>
> This could be something like
>
>     let _ = |p: &$c_type| { _ = &p.$($field).+ }
>
> ?
>
>> +
>> +        // Calculate offset and create `CList`.
>> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
>> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
>
> Given that this is unsafe, I am not sure why the macro should have unsafe
> keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
> user write unsafe.

Either you are proposing to not wrap unsafe code within unsafe {} within the
macro, such that the user is forced to write an unsafe {} around the macro, but
then they calls within the macro are not justified individually, or you propose
to let the user write an unsafe {} around the macro regardless of the inner
unsafe {} blocks, but then then the compiler warns about an unnecessary unsafe
and nothing forces the user to actually wrap it in unsafe {}.

Is there a third option I'm not aware of? I.e. for the above reason
impl_device_context_deref!() was designed the same way.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n650