Skip to contentSkip to conversation

rust: list: remove HasListLinks::OFFSET

PATCH Superseded Tamir Duberstein 3 participants 11 messages Apr 23, 2025 – Jul 9, 2025

Subsystems (2)
PCI Rust

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Tamir Duberstein

author
#0 lore raw

v3 cover rust: list: remove HasListLinks::OFFSET

The bulk of this change occurs in the last commit, please its commit messages for details.

Signed-off-by Tamir Duberstein <[email protected]>

---
Changes in v3:
- Add a patch to improve macro hygiene.
- Add a patch to include examples for all macros.
- Make it build properly!
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Change type parameter delimiter to {} for consistency. (Boqun Feng)
- Rebase on v6.15-rc1.
- Extract first commit to its own series as it is shared with other
series.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Tamir Duberstein (6):
rust: list: simplify macro capture
rust: list: use consistent type parameter style
rust: list: use consistent self parameter name
rust: list: use fully qualified path
rust: list: add impl_list_item! examples
rust: list: remove OFFSET constants

 rust/kernel/list.rs                    |  18 ++-
 rust/kernel/list/impl_list_item_mod.rs | 233 ++++++++++++++++++++++-----------
 2 files changed, 165 insertions(+), 86 deletions(-)

---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250324-list-no-offset-96ef65cb2a95
prerequisite-change-id: 20250409-container-of-mutness-b153dab4388d:v1
prerequisite-patch-id: 53d5889db599267f87642bb0ae3063c29bc24863

Best regards,

Signature · 1 line
Tamir Duberstein <[email protected]>
author
#1 lore raw

PATCH v3 1/6 rust: list: simplify macro capture

Avoid manually capturing generics; use ty to capture the whole type instead.

Signed-off-by Tamir Duberstein <[email protected]>
---
 rust/kernel/list/impl_list_item_mod.rs | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)
rust/kernel/list/impl_list_item_mod.rs +5 -11
index a0438537cee1..5ed66fdce953 100644
@@ -43,7 +43,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
 macro_rules! impl_has_list_links {
     ($(impl$(<$($implarg:ident),*>)?
        HasListLinks$(<$id:tt>)?
-       for $self:ident $(<$($selfarg:ty),*>)?
+       for $self:ty
        { self$(.$field:ident)* }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the

@@ -51,9 +51,7 @@ macro_rules! impl_has_list_links {
         //
         // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
         // equivalent to the pointer offset operation in the trait definition.
-        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for
-            $self $(<$($selfarg),*>)?
-        {
+        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
 
             #[inline]

@@ -85,18 +83,14 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
 macro_rules! impl_has_list_links_self_ptr {
     ($(impl$({$($implarg:tt)*})?
        HasSelfPtr<$item_type:ty $(, $id:tt)?>
-       for $self:ident $(<$($selfarg:ty),*>)?
+       for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
         // right type.
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for
-            $self $(<$($selfarg),*>)?
-        {}
+        unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
 
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for
-            $self $(<$($selfarg),*>)?
-        {
+        unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]
Signature · 1 line
2.49.0
author
#2 lore raw

PATCH v3 2/6 rust: list: use consistent type parameter style

In reply to Tamir Duberstein

Refer to the type parameters of impl_has_list_links{,_self_ptr}! by the same name used in impl_list_item!. Capture type parameters of impl_list_item! as tt using {} to match the style of all other macros that work with generics.

Signed-off-by Tamir Duberstein <[email protected]>
---
 rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
rust/kernel/list/impl_list_item_mod.rs +5 -5
index 5ed66fdce953..49f5af12e796 100644
@@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
 /// Implements the [`HasListLinks`] trait for the given type.
 #[macro_export]
 macro_rules! impl_has_list_links {
-    ($(impl$(<$($implarg:ident),*>)?
+    ($(impl$({$($generics:tt)*})?
        HasListLinks$(<$id:tt>)?
        for $self:ty
        { self$(.$field:ident)* }

@@ -51,7 +51,7 @@ macro_rules! impl_has_list_links {
         //
         // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
         // equivalent to the pointer offset operation in the trait definition.
-        unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self {
+        unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
 
             #[inline]

@@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
 /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
 #[macro_export]
 macro_rules! impl_has_list_links_self_ptr {
-    ($(impl$({$($implarg:tt)*})?
+    ($(impl$({$($generics:tt)*})?
        HasSelfPtr<$item_type:ty $(, $id:tt)?>
        for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
         // right type.
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
+        unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
 
-        unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
+        unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]
Signature · 1 line
2.49.0
author
#3 lore raw

PATCH v3 3/6 rust: list: use consistent self parameter name

In reply to Tamir Duberstein

Refer to the self parameter of impl_list_item! by the same name used in impl_has_list_links{,_self_ptr}!.

Signed-off-by Tamir Duberstein <[email protected]>
---
 rust/kernel/list/impl_list_item_mod.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
rust/kernel/list/impl_list_item_mod.rs +4 -4
index 49f5af12e796..d79bebb363ce 100644
@@ -114,12 +114,12 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
 #[macro_export]
 macro_rules! impl_list_item {
     (
-        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
+        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
             using ListLinks;
         })*
     ) => {$(
         // SAFETY: See GUARANTEES comment on each method.
-        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
+        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
             // * This returns the same pointer as `prepare_to_insert` because `prepare_to_insert`
             //   is implemented in terms of `view_links`.

@@ -178,12 +178,12 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
     )*};
 
     (
-        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
+        $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
             using ListLinksSelfPtr;
         })*
     ) => {$(
         // SAFETY: See GUARANTEES comment on each method.
-        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
+        unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
             // This implementation of `ListItem` will not give out exclusive access to the same
             // `ListLinks` several times because calls to `prepare_to_insert` and `post_remove`
Signature · 1 line
2.49.0
author
#4 lore raw

PATCH v3 4/6 rust: list: use fully qualified path

In reply to Tamir Duberstein

Use a fully qualified path rooted at $crate rather than relying on imports in the invoking scope.

Signed-off-by Tamir Duberstein <[email protected]>
---
 rust/kernel/list/impl_list_item_mod.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
rust/kernel/list/impl_list_item_mod.rs +6 -6
index d79bebb363ce..3f6cd1b090a7 100644
@@ -4,8 +4,6 @@
 
 //! Helpers for implementing list traits safely.
 
-use crate::list::ListLinks;
-
 /// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
 ///
 /// This trait is only used to help implement `ListItem` safely. If `ListItem` is implemented

@@ -27,14 +25,14 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
     ///
     /// The provided pointer must point at a valid struct of type `Self`.
     ///
-    /// [`ListLinks<T, ID>`]: ListLinks
+    /// [`ListLinks<T, ID>`]: crate::list::ListLinks
     // We don't really need this method, but it's necessary for the implementation of
     // `impl_has_list_links!` to be correct.
     #[inline]
-    unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
+    unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut crate::list::ListLinks<ID> {
         // SAFETY: The caller promises that the pointer is valid. The implementer promises that the
         // `OFFSET` constant is correct.
-        unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
+        unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut crate::list::ListLinks<ID> }
     }
 }
 

@@ -222,7 +220,9 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
             //   this value is not in a list.
             unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
                 // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
-                unsafe { <Self as HasListLinks<$num>>::raw_get_list_links(me.cast_mut()) }
+                unsafe {
+                    <Self as $crate::list::HasListLinks<$num>>::raw_get_list_links(me.cast_mut())
+                }
             }
 
             // This function is also used as the implementation of `post_remove`, so the caller
Signature · 1 line
2.49.0
author
#5 lore raw

PATCH v3 5/6 rust: list: add `impl_list_item!` examples

In reply to Tamir Duberstein

There's a comprehensive example in rust/kernel/list.rs but it doesn't exercise the using ListLinksSelfPtr variant nor the generic cases. Add that here. Generalize impl_has_list_links_self_ptr to handle nested fields in the same manner as impl_has_list_links.

Signed-off-by Tamir Duberstein <[email protected]>
---
 rust/kernel/list/impl_list_item_mod.rs | 96 ++++++++++++++++++++++++++++++++--
 1 file changed, 93 insertions(+), 3 deletions(-)
rust/kernel/list/impl_list_item_mod.rs +93 -3
index 3f6cd1b090a7..df21070e1cf6 100644
@@ -82,20 +82,20 @@ macro_rules! impl_has_list_links_self_ptr {
     ($(impl$({$($generics:tt)*})?
        HasSelfPtr<$item_type:ty $(, $id:tt)?>
        for $self:ty
-       { self.$field:ident }
+       { self$(.$field:ident)* }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
         // right type.
         unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
 
         unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
-            const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
+            const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
 
             #[inline]
             unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
                 // SAFETY: The caller promises that the pointer is not dangling.
                 let ptr: *mut $crate::list::ListLinksSelfPtr<$item_type $(, $id)?> =
-                    unsafe { ::core::ptr::addr_of_mut!((*ptr).$field) };
+                    unsafe { ::core::ptr::addr_of_mut!((*ptr)$(.$field)*) };
                 ptr.cast()
             }
         }

@@ -109,6 +109,96 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
 /// implement that trait.
 ///
 /// [`ListItem`]: crate::list::ListItem
+///
+/// # Examples
+///
+/// ```
+/// #[pin_data]
+/// struct SimpleListItem {
+///     value: u32,
+///     #[pin]
+///     links: kernel::list::ListLinks,
+/// }
+///
+/// kernel::list::impl_has_list_links! {
+///     impl HasListLinks<0> for SimpleListItem { self.links }
+/// }
+///
+/// kernel::list::impl_list_arc_safe! {
+///     impl ListArcSafe<0> for SimpleListItem { untracked; }
+/// }
+///
+/// kernel::list::impl_list_item! {
+///     impl ListItem<0> for SimpleListItem { using ListLinks; }
+/// }
+///
+/// struct ListLinksHolder {
+///     inner: kernel::list::ListLinks,
+/// }
+///
+/// #[pin_data]
+/// struct ComplexListItem<T, U> {
+///     value: Result<T, U>,
+///     #[pin]
+///     links: ListLinksHolder,
+/// }
+///
+/// kernel::list::impl_has_list_links! {
+///     impl{T, U} HasListLinks<0> for ComplexListItem<T, U> { self.links.inner }
+/// }
+///
+/// kernel::list::impl_list_arc_safe! {
+///     impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; }
+/// }
+///
+/// kernel::list::impl_list_item! {
+///     impl{T, U} ListItem<0> for ComplexListItem<T, U> { using ListLinks; }
+/// }
+/// ```
+///
+/// ```
+/// #[pin_data]
+/// struct SimpleListItem {
+///     value: u32,
+///     #[pin]
+///     links: kernel::list::ListLinksSelfPtr<SimpleListItem>,
+/// }
+///
+/// kernel::list::impl_list_arc_safe! {
+///     impl ListArcSafe<0> for SimpleListItem { untracked; }
+/// }
+///
+/// kernel::list::impl_has_list_links_self_ptr! {
+///     impl HasSelfPtr<SimpleListItem> for SimpleListItem { self.links }
+/// }
+///
+/// kernel::list::impl_list_item! {
+///     impl ListItem<0> for SimpleListItem { using ListLinksSelfPtr; }
+/// }
+///
+/// struct ListLinksSelfPtrHolder<T, U> {
+///     inner: kernel::list::ListLinksSelfPtr<ComplexListItem<T, U>>,
+/// }
+///
+/// #[pin_data]
+/// struct ComplexListItem<T, U> {
+///     value: Result<T, U>,
+///     #[pin]
+///     links: ListLinksSelfPtrHolder<T, U>,
+/// }
+///
+/// kernel::list::impl_list_arc_safe! {
+///     impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; }
+/// }
+///
+/// kernel::list::impl_has_list_links_self_ptr! {
+///     impl{T, U} HasSelfPtr<ComplexListItem<T, U>> for ComplexListItem<T, U> { self.links.inner }
+/// }
+///
+/// kernel::list::impl_list_item! {
+///     impl{T, U} ListItem<0> for ComplexListItem<T, U> { using ListLinksSelfPtr; }
+/// }
+/// ```
 #[macro_export]
 macro_rules! impl_list_item {
     (
Signature · 1 line
2.49.0
author
#6 lore raw

PATCH v3 6/6 rust: list: remove OFFSET constants

In reply to Tamir Duberstein

Replace ListLinksSelfPtr::LIST_LINKS_SELF_PTR_OFFSET with unsafe fn raw_get_self_ptr which returns a pointer to the field rather than requiring the caller to do pointer arithmetic.

Implement HasListLinks::raw_get_list_links in impl_has_list_links!, narrowing the interface of HasListLinks and replacing pointer arithmetic with container_of!.

Modify impl_list_item to also invoke impl_has_list_links! or impl_has_list_links_self_ptr!. This is necessary to allow impl_list_item to see more of the tokens used by impl_has_list_links{,_self_ptr}!.

A similar API change was discussed on the hrtimer series[1].

Link https://lore.kernel.org/all/[email protected]/ [1] open here
Signed-off-by Tamir Duberstein <[email protected]>
---
 rust/kernel/list.rs                    |  18 +++--
 rust/kernel/list/impl_list_item_mod.rs | 143 +++++++++++++++------------------
 2 files changed, 78 insertions(+), 83 deletions(-)
rust/kernel/list.rs +11 -7
index a335c3b1ff5e..9017164c4556 100644
@@ -212,9 +212,6 @@ unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
 unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
 
 impl<T: ?Sized, const ID: u64> ListLinksSelfPtr<T, ID> {
-    /// The offset from the [`ListLinks`] to the self pointer field.
-    pub const LIST_LINKS_SELF_PTR_OFFSET: usize = core::mem::offset_of!(Self, self_ptr);
-
     /// Creates a new initializer for this type.
     pub fn new() -> impl PinInit<Self> {
         // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will

@@ -229,6 +226,16 @@ pub fn new() -> impl PinInit<Self> {
             self_ptr: Opaque::uninit(),
         }
     }
+
+    /// Returns a pointer to the self pointer.
+    ///
+    /// # Safety
+    ///
+    /// The provided pointer must point at a valid struct of type `Self`.
+    pub unsafe fn raw_get_self_ptr(me: *const Self) -> *const Opaque<*const T> {
+        // SAFETY: The caller promises that the pointer is valid.
+        unsafe { ptr::addr_of!((*me).self_ptr) }
+    }
 }
 
 impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {

@@ -603,14 +610,11 @@ fn next(&mut self) -> Option<ArcBorrow<'a, T>> {
 ///     }
 /// }
 ///
-/// kernel::list::impl_has_list_links! {
-///     impl HasListLinks<0> for ListItem { self.links }
-/// }
 /// kernel::list::impl_list_arc_safe! {
 ///     impl ListArcSafe<0> for ListItem { untracked; }
 /// }
 /// kernel::list::impl_list_item! {
-///     impl ListItem<0> for ListItem { using ListLinks; }
+///     impl ListItem<0> for ListItem { using ListLinks { self.links }; }
 /// }
 ///
 /// // Use a cursor to remove the first element with the given value.
rust/kernel/list/impl_list_item_mod.rs +67 -76

271 lines not on this page · raw message

Signature · 1 line
2.49.0
author
#8 lore raw
Quoted 10 lines · Alice Ryhl
Quoted 4 lines · Tamir Duberstein

The bulk of this change occurs in the last commit, please its commit messages for details.

Signed-off-by Tamir Duberstein <[email protected]>

This works with Rust Binder and Ashmem.

Tested-by Alice Ryhl <[email protected]>

Thanks Alice. Could you also review? I guess this still needs some RBs. @Benno Lossin could you perhaps have a look as well? You both reviewed my other series[0] which was quite similar.

[0] https://lore.kernel.org/all/[email protected]/

reviewer
#9 lore raw
Quoted 10 lines · Tamir Duberstein
Quoted 9 lines · Alice Ryhl
Quoted 4 lines · Tamir Duberstein

The bulk of this change occurs in the last commit, please its commit messages for details.

Signed-off-by Tamir Duberstein <[email protected]>

This works with Rust Binder and Ashmem.

Quoted 7 lines
Quoted 1 line
Tested-by Alice Ryhl <[email protected]>

Thanks Alice. Could you also review? I guess this still needs some RBs. @Benno Lossin could you perhaps have a look as well? You both reviewed my other series[0] which was quite similar.

[0] https://lore.kernel.org/all/[email protected]/

I probably won't have time for this one.

---
Cheers,
Benno