linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET
@ 2025-04-09 14:51 Tamir Duberstein
  2025-04-09 14:51 ` [PATCH v2 1/4] rust: list: simplify macro capture Tamir Duberstein
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-04-09 14:51 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci, Tamir Duberstein

The bulk of this change occurs in the last commit, please its commit
messages for details. The first commit exists in 2 other series but was
picked into this series to allow using `container_of!` without the need
to cast from `*const Self` to `*mut Self`.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
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/20250324-list-no-offset-v1-0-afd2b7fc442a@gmail.com

---
Tamir Duberstein (4):
      rust: list: simplify macro capture
      rust: list: use consistent type parameter style
      rust: list: use consistent self parameter name
      rust: list: remove OFFSET constants

 rust/kernel/list.rs                    |  18 +++--
 rust/kernel/list/impl_list_item_mod.rs | 128 +++++++++++++++------------------
 2 files changed, 67 insertions(+), 79 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,
-- 
Tamir Duberstein <tamird@gmail.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/4] rust: list: simplify macro capture
  2025-04-09 14:51 [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
@ 2025-04-09 14:51 ` Tamir Duberstein
  2025-04-16 15:17   ` Christian Schrefl
  2025-04-09 14:51 ` [PATCH v2 2/4] rust: list: use consistent type parameter style Tamir Duberstein
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Tamir Duberstein @ 2025-04-09 14:51 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci, Tamir Duberstein

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

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 rust/kernel/list/impl_list_item_mod.rs | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index a0438537cee1..5ed66fdce953 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -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]

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/4] rust: list: use consistent type parameter style
  2025-04-09 14:51 [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
  2025-04-09 14:51 ` [PATCH v2 1/4] rust: list: simplify macro capture Tamir Duberstein
@ 2025-04-09 14:51 ` Tamir Duberstein
  2025-04-16 15:18   ` Christian Schrefl
  2025-04-09 14:51 ` [PATCH v2 3/4] rust: list: use consistent self parameter name Tamir Duberstein
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Tamir Duberstein @ 2025-04-09 14:51 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci, 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 <tamird@gmail.com>
---
 rust/kernel/list/impl_list_item_mod.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 5ed66fdce953..49f5af12e796 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -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]

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/4] rust: list: use consistent self parameter name
  2025-04-09 14:51 [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
  2025-04-09 14:51 ` [PATCH v2 1/4] rust: list: simplify macro capture Tamir Duberstein
  2025-04-09 14:51 ` [PATCH v2 2/4] rust: list: use consistent type parameter style Tamir Duberstein
@ 2025-04-09 14:51 ` Tamir Duberstein
  2025-04-16 15:20   ` Christian Schrefl
  2025-04-09 14:51 ` [PATCH v2 4/4] rust: list: remove OFFSET constants Tamir Duberstein
  2025-04-09 14:53 ` [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
  4 siblings, 1 reply; 10+ messages in thread
From: Tamir Duberstein @ 2025-04-09 14:51 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci, 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 <tamird@gmail.com>
---
 rust/kernel/list/impl_list_item_mod.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 49f5af12e796..d79bebb363ce 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -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`

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/4] rust: list: remove OFFSET constants
  2025-04-09 14:51 [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
                   ` (2 preceding siblings ...)
  2025-04-09 14:51 ` [PATCH v2 3/4] rust: list: use consistent self parameter name Tamir Duberstein
@ 2025-04-09 14:51 ` Tamir Duberstein
  2025-04-09 14:53 ` [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
  4 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-04-09 14:51 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci, 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/20250224-hrtimer-v3-v6-12-rc2-v9-1-5bd3bf0ce6cc@kernel.org/ [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 rust/kernel/list.rs                    |  18 +++---
 rust/kernel/list/impl_list_item_mod.rs | 100 +++++++++++++++------------------
 2 files changed, 56 insertions(+), 62 deletions(-)

diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index a335c3b1ff5e..f370a8c1df98 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -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: *mut 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.
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index d79bebb363ce..24489996d11b 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -6,21 +6,18 @@
 
 use crate::list::ListLinks;
 
-/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
+/// Declares that this type has a [`ListLinks<ID>`] field.
 ///
-/// This trait is only used to help implement `ListItem` safely. If `ListItem` is implemented
+/// This trait is only used to help implement [`ListItem`] safely. If [`ListItem`] is implemented
 /// manually, then this trait is not needed. Use the [`impl_has_list_links!`] macro to implement
 /// this trait.
 ///
 /// # Safety
 ///
-/// All values of this type must have a `ListLinks<ID>` field at the given offset.
+/// The methods on this trait must have exactly the behavior that the definitions given below have.
 ///
-/// The behavior of `raw_get_list_links` must not be changed.
+/// [`ListItem`]: crate::list::ListItem
 pub unsafe trait HasListLinks<const ID: u64 = 0> {
-    /// The offset of the `ListLinks` field.
-    const OFFSET: usize;
-
     /// Returns a pointer to the [`ListLinks<T, ID>`] field.
     ///
     /// # Safety
@@ -28,14 +25,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
     /// The provided pointer must point at a valid struct of type `Self`.
     ///
     /// [`ListLinks<T, ID>`]: 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> {
-        // 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 fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID>;
 }
 
 /// Implements the [`HasListLinks`] trait for the given type.
@@ -48,14 +38,11 @@ macro_rules! impl_has_list_links {
     )*) => {$(
         // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
         // right type.
-        //
-        // 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$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
-            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>)? {
+                const _: usize = ::core::mem::offset_of!($self, $($field).*);
+
                 // SAFETY: The caller promises that the pointer is not dangling. We know that this
                 // expression doesn't follow any pointers, as the `offset_of!` invocation above
                 // would otherwise not compile.
@@ -66,12 +53,15 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
 }
 pub use impl_has_list_links;
 
-/// Declares that the `ListLinks<ID>` field in this struct is inside a `ListLinksSelfPtr<T, ID>`.
+/// Declares that the [`ListLinks<ID>`] field in this struct is inside a
+/// [`ListLinksSelfPtr<T, ID>`].
 ///
 /// # Safety
 ///
-/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be
-/// inside a `ListLinksSelfPtr<T, ID>`.
+/// The [`ListLinks<ID>`] field of this struct at [`HasListLinks<ID>::raw_get_list_links`] must be
+/// inside a [`ListLinksSelfPtr<T, ID>`].
+///
+/// [`ListLinksSelfPtr<T, ID>`]: crate::list::ListLinksSelfPtr
 pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
 where
     Self: HasListLinks<ID>,
@@ -91,8 +81,6 @@ macro_rules! impl_has_list_links_self_ptr {
         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;
-
             #[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.
@@ -115,9 +103,13 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
 macro_rules! impl_list_item {
     (
         $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
-            using ListLinks;
+            using ListLinks { self$(.$field:ident)* };
         })*
     ) => {$(
+        $crate::list::impl_has_list_links! {
+            impl$({$($generics:tt)*})? HasListLinks<$num> for $self { self$(.$field)* }
+        }
+
         // SAFETY: See GUARANTEES comment on each method.
         unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
@@ -133,20 +125,19 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
             }
 
             // GUARANTEES:
-            // * `me` originates from the most recent call to `prepare_to_insert`, which just added
-            //   `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
-            //   `offset` from `me` so it returns the pointer originally passed to
-            //   `prepare_to_insert`.
+            // * `me` originates from the most recent call to `prepare_to_insert`, which calls
+            //   `raw_get_list_link`, which is implemented using `addr_of_mut!((*self).$field)`.
+            //   This method uses `container_of` to perform the inverse operation, so it returns the
+            //   pointer originally passed to `prepare_to_insert`.
             // * The pointer remains valid until the next call to `post_remove` because the caller
             //   of the most recent call to `prepare_to_insert` promised to retain ownership of the
             //   `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
             //   be destroyed while a `ListArc` reference exists.
             unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
-                let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
                 // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
-                // points at the field at offset `offset` in a value of type `Self`. Thus,
-                // subtracting `offset` from `me` is still in-bounds of the allocation.
-                unsafe { (me as *const u8).sub(offset) as *const Self }
+                // points at the field `$field` in a value of type `Self`. Thus, reversing that
+                // operation is still in-bounds of the allocation.
+                $crate::container_of!(me, Self, $($field)*)
             }
 
             // GUARANTEES:
@@ -163,25 +154,28 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
             }
 
             // GUARANTEES:
-            // * `me` originates from the most recent call to `prepare_to_insert`, which just added
-            //   `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
-            //   `offset` from `me` so it returns the pointer originally passed to
-            //   `prepare_to_insert`.
+            // * `me` originates from the most recent call to `prepare_to_insert`, which calls
+            //   `raw_get_list_link`, which is implemented using `addr_of_mut!((*self).$field)`.
+            //   This method uses `container_of` to perform the inverse operation, so it returns the
+            //   pointer originally passed to `prepare_to_insert`.
             unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
-                let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
                 // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
-                // points at the field at offset `offset` in a value of type `Self`. Thus,
-                // subtracting `offset` from `me` is still in-bounds of the allocation.
-                unsafe { (me as *const u8).sub(offset) as *const Self }
+                // points at the field `$field` in a value of type `Self`. Thus, reversing that
+                // operation is still in-bounds of the allocation.
+                $crate::container_of!(me, Self, $($field)*)
             }
         }
     )*};
 
     (
         $(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
-            using ListLinksSelfPtr;
+            using ListLinksSelfPtr { self$(.$field:ident)* };
         })*
     ) => {$(
+        $crate::list::impl_has_list_links_self_ptr! {
+            impl$({$($generics:tt)*})? HasListLinks<$num> for $self { self$(.$field)* }
+        }
+
         // SAFETY: See GUARANTEES comment on each method.
         unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
             // GUARANTEES:
@@ -196,13 +190,10 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
                 // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
                 let links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
 
-                let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
-                // Goes via the offset as the field is private.
-                //
-                // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
-                // the pointer stays in bounds of the allocation.
-                let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
-                    as *const $crate::types::Opaque<*const Self>;
+                // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
+                let self_ptr = unsafe {
+                    $crate::list::ListLinksSelfPtr::<Self, $num>::raw_get_self_ptr(links_field)
+                };
                 let cell_inner = $crate::types::Opaque::raw_get(self_ptr);
 
                 // SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
@@ -241,11 +232,10 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
             //   `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
             //   be destroyed while a `ListArc` reference exists.
             unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self {
-                let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
-                // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
-                // the pointer stays in bounds of the allocation.
-                let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
-                    as *const ::core::cell::UnsafeCell<*const Self>;
+                // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
+                let self_ptr = unsafe {
+                    $crate::list::ListLinksSelfPtr::<Self, $num>::raw_get_self_ptr(links_field)
+                };
                 let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
                 // SAFETY: This is not a data race, because the only function that writes to this
                 // value is `prepare_to_insert`, but by the safety requirements the

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET
  2025-04-09 14:51 [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
                   ` (3 preceding siblings ...)
  2025-04-09 14:51 ` [PATCH v2 4/4] rust: list: remove OFFSET constants Tamir Duberstein
@ 2025-04-09 14:53 ` Tamir Duberstein
  2025-04-14 17:53   ` Tamir Duberstein
  4 siblings, 1 reply; 10+ messages in thread
From: Tamir Duberstein @ 2025-04-09 14:53 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci

On Wed, Apr 9, 2025 at 10:51 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> The bulk of this change occurs in the last commit, please its commit
> messages for details. The first commit exists in 2 other series but was
> picked into this series to allow using `container_of!` without the need
> to cast from `*const Self` to `*mut Self`.

That first commit is now in the prerequisite patch, not here.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET
  2025-04-09 14:53 ` [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
@ 2025-04-14 17:53   ` Tamir Duberstein
  0 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-04-14 17:53 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci

On Wed, Apr 9, 2025 at 10:53 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Wed, Apr 9, 2025 at 10:51 AM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > The bulk of this change occurs in the last commit, please its commit
> > messages for details. The first commit exists in 2 other series but was
> > picked into this series to allow using `container_of!` without the need
> > to cast from `*const Self` to `*mut Self`.
>
> That first commit is now in the prerequisite patch, not here.

Gentle ping; this still needs a review. The first two patches are
trivial, and the last one is similar in spirit to
https://lore.kernel.org/all/20250411-no-offset-v3-1-c0b174640ec3@gmail.com/
which was reviewed by Benno Lossin, Alice Ryhl, and Andreas Hindborg.
Could you folks have a look please?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/4] rust: list: simplify macro capture
  2025-04-09 14:51 ` [PATCH v2 1/4] rust: list: simplify macro capture Tamir Duberstein
@ 2025-04-16 15:17   ` Christian Schrefl
  0 siblings, 0 replies; 10+ messages in thread
From: Christian Schrefl @ 2025-04-16 15:17 UTC (permalink / raw)
  To: Tamir Duberstein, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci

On 09.04.25 4:51 PM, Tamir Duberstein wrote:
> Avoid manually capturing generics; use `ty` to capture the whole type
> instead.
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/4] rust: list: use consistent type parameter style
  2025-04-09 14:51 ` [PATCH v2 2/4] rust: list: use consistent type parameter style Tamir Duberstein
@ 2025-04-16 15:18   ` Christian Schrefl
  0 siblings, 0 replies; 10+ messages in thread
From: Christian Schrefl @ 2025-04-16 15:18 UTC (permalink / raw)
  To: Tamir Duberstein, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci

On 09.04.25 4:51 PM, Tamir Duberstein wrote:
> 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 <tamird@gmail.com>
> ---

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/4] rust: list: use consistent self parameter name
  2025-04-09 14:51 ` [PATCH v2 3/4] rust: list: use consistent self parameter name Tamir Duberstein
@ 2025-04-16 15:20   ` Christian Schrefl
  0 siblings, 0 replies; 10+ messages in thread
From: Christian Schrefl @ 2025-04-16 15:20 UTC (permalink / raw)
  To: Tamir Duberstein, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bjorn Helgaas, Greg Kroah-Hartman,
	Rafael J. Wysocki
  Cc: rust-for-linux, linux-kernel, linux-pci

On 09.04.25 4:51 PM, Tamir Duberstein wrote:
> 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 <tamird@gmail.com>
> ---
Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-04-16 15:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-09 14:51 [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
2025-04-09 14:51 ` [PATCH v2 1/4] rust: list: simplify macro capture Tamir Duberstein
2025-04-16 15:17   ` Christian Schrefl
2025-04-09 14:51 ` [PATCH v2 2/4] rust: list: use consistent type parameter style Tamir Duberstein
2025-04-16 15:18   ` Christian Schrefl
2025-04-09 14:51 ` [PATCH v2 3/4] rust: list: use consistent self parameter name Tamir Duberstein
2025-04-16 15:20   ` Christian Schrefl
2025-04-09 14:51 ` [PATCH v2 4/4] rust: list: remove OFFSET constants Tamir Duberstein
2025-04-09 14:53 ` [PATCH v2 0/4] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
2025-04-14 17:53   ` Tamir Duberstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).