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

["Gary Guo" <gary@garyguo.net>]

On Thu Mar 19, 2026 at 12:05 PM GMT, Danilo Krummrich wrote:
> 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 {}.

The former.

"The caller of this macro is responsible for ensuring safety" justification is
not really useful here IMO.

If there're cases where we do want to justify unsafe code that's not immediately
deferring to the user inside the macro, we could use the SAFETY* trick proposed
in the thread, without writing an actual `unsafe {}` block.

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

impl_device_context_deref!() expands to an item, so the user couldn't put an
`unsafe {}` on the outside. This macro expands to an expression, so users can
add `unsafe` themselves.

Best,
Gary

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