* [PATCH v4 1/6] rust: list: simplify macro capture
2025-07-09 19:31 [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
@ 2025-07-09 19:31 ` Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 2/6] rust: list: use consistent type parameter style Tamir Duberstein
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-07-09 19:31 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,
Christian Schrefl
Avoid manually capturing generics; use `ty` to capture the whole type
instead.
Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Tested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
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 1f9498c1458f..6bfa59fde17b 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.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 2/6] rust: list: use consistent type parameter style
2025-07-09 19:31 [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 1/6] rust: list: simplify macro capture Tamir Duberstein
@ 2025-07-09 19:31 ` Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 3/6] rust: list: use consistent self parameter name Tamir Duberstein
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-07-09 19:31 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,
Christian Schrefl
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.
Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Tested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
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 6bfa59fde17b..27feaf849c48 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.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 3/6] rust: list: use consistent self parameter name
2025-07-09 19:31 [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 1/6] rust: list: simplify macro capture Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 2/6] rust: list: use consistent type parameter style Tamir Duberstein
@ 2025-07-09 19:31 ` Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 4/6] rust: list: use fully qualified path Tamir Duberstein
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-07-09 19:31 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,
Christian Schrefl
Refer to the self parameter of `impl_list_item!` by the same name used
in `impl_has_list_links{,_self_ptr}!`.
Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Tested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
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 27feaf849c48..050299380330 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.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 4/6] rust: list: use fully qualified path
2025-07-09 19:31 [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
` (2 preceding siblings ...)
2025-07-09 19:31 ` [PATCH v4 3/6] rust: list: use consistent self parameter name Tamir Duberstein
@ 2025-07-09 19:31 ` Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 5/6] rust: list: add `impl_list_item!` examples Tamir Duberstein
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-07-09 19:31 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
Use a fully qualified path rooted at `$crate` rather than relying on
imports in the invoking scope.
Tested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
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 050299380330..32087aee4c87 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -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,11 +25,11 @@ 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.cast::<u8>().add(Self::OFFSET).cast() }
@@ -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
--
2.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 5/6] rust: list: add `impl_list_item!` examples
2025-07-09 19:31 [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
` (3 preceding siblings ...)
2025-07-09 19:31 ` [PATCH v4 4/6] rust: list: use fully qualified path Tamir Duberstein
@ 2025-07-09 19:31 ` Tamir Duberstein
2025-07-09 19:31 ` [PATCH v4 6/6] rust: list: remove OFFSET constants Tamir Duberstein
2025-07-19 22:16 ` [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Miguel Ojeda
6 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-07-09 19:31 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
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`.
Tested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/list/impl_list_item_mod.rs | 96 ++++++++++++++++++++++++++++++++--
1 file changed, 93 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 32087aee4c87..6b67c0ecc57b 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -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 {
(
--
2.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 6/6] rust: list: remove OFFSET constants
2025-07-09 19:31 [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
` (4 preceding siblings ...)
2025-07-09 19:31 ` [PATCH v4 5/6] rust: list: add `impl_list_item!` examples Tamir Duberstein
@ 2025-07-09 19:31 ` Tamir Duberstein
2025-07-19 21:09 ` Miguel Ojeda
2025-07-19 22:16 ` [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Miguel Ojeda
6 siblings, 1 reply; 10+ messages in thread
From: Tamir Duberstein @ 2025-07-09 19:31 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]
Tested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/list.rs | 23 +++---
rust/kernel/list/impl_list_item_mod.rs | 143 +++++++++++++++------------------
2 files changed, 79 insertions(+), 87 deletions(-)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index fe58a3920e70..d976b31d1639 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -57,14 +57,11 @@
/// }
/// }
///
-/// impl_has_list_links! {
-/// impl HasListLinks<0> for BasicItem { self.links }
-/// }
/// impl_list_arc_safe! {
/// impl ListArcSafe<0> for BasicItem { untracked; }
/// }
/// impl_list_item! {
-/// impl ListItem<0> for BasicItem { using ListLinks; }
+/// impl ListItem<0> for BasicItem { using ListLinks { self.links }; }
/// }
///
/// // Create a new empty list.
@@ -320,9 +317,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
@@ -337,6 +331,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> {
@@ -711,14 +715,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 6b67c0ecc57b..289101e0ab5e 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -4,21 +4,18 @@
//! Helpers for implementing list traits safely.
-/// 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
@@ -26,14 +23,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
/// The provided pointer must point at a valid struct of type `Self`.
///
/// [`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 crate::list::ListLinks<ID> {
- // SAFETY: The caller promises that the pointer is valid. The implementer promises that the
- // `OFFSET` constant is correct.
- unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast() }
- }
+ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut crate::list::ListLinks<ID>;
}
/// Implements the [`HasListLinks`] trait for the given type.
@@ -46,14 +36,15 @@ 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>)? {
+ // Statically ensure that `$(.field)*` doesn't follow any pointers.
+ //
+ // Cannot be `const` because `$self` may contain generics and E0401 says constants
+ // "can't use {`Self`,generic parameters} from outer item".
+ if false { let _: 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.
@@ -64,12 +55,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>,
@@ -89,8 +83,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.
@@ -120,16 +112,12 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// 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; }
+/// impl ListItem<0> for SimpleListItem { using ListLinks { self.links }; }
/// }
///
/// struct ListLinksHolder {
@@ -143,16 +131,12 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// 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; }
+/// impl{T, U} ListItem<0> for ComplexListItem<T, U> { using ListLinks { self.links.inner }; }
/// }
/// ```
///
@@ -168,12 +152,8 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// 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; }
+/// impl ListItem<0> for SimpleListItem { using ListLinksSelfPtr { self.links }; }
/// }
///
/// struct ListLinksSelfPtrHolder<T, U> {
@@ -191,21 +171,23 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// 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; }
+/// impl{T, U} ListItem<0> for ComplexListItem<T, U> {
+/// using ListLinksSelfPtr { self.links.inner };
+/// }
/// }
/// ```
#[macro_export]
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)*})? HasListLinks<$num> for $self { self$(.$field)* }
+ }
+
// SAFETY: See GUARANTEES comment on each method.
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
// GUARANTEES:
@@ -221,20 +203,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:
@@ -251,25 +232,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)*})? HasSelfPtr<$self> for $self { self$(.$field)* }
+ }
+
// SAFETY: See GUARANTEES comment on each method.
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
// GUARANTEES:
@@ -284,13 +268,15 @@ 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>;
+ let container = $crate::container_of!(
+ links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
+ );
+
+ // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
+ let self_ptr = unsafe {
+ $crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
+ };
+
let cell_inner = $crate::types::Opaque::raw_get(self_ptr);
// SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
@@ -331,12 +317,17 @@ 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>;
- let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
+ let container = $crate::container_of!(
+ links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
+ );
+
+ // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
+ let self_ptr = unsafe {
+ $crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
+ };
+
+ let cell_inner = $crate::types::Opaque::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
// `prepare_to_insert` method may not be called in parallel with `view_value` or
--
2.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 6/6] rust: list: remove OFFSET constants
2025-07-09 19:31 ` [PATCH v4 6/6] rust: list: remove OFFSET constants Tamir Duberstein
@ 2025-07-19 21:09 ` Miguel Ojeda
2025-07-19 21:17 ` Tamir Duberstein
0 siblings, 1 reply; 10+ messages in thread
From: Miguel Ojeda @ 2025-07-19 21:09 UTC (permalink / raw)
To: Tamir Duberstein
Cc: 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, rust-for-linux, linux-kernel, linux-pci
On Wed, Jul 9, 2025 at 9:31 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> -/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
> +/// Declares that this type has a [`ListLinks<ID>`] field.
I was applying this series the other day, and I noticed these
doc-related changes in the patch, which are appreciated (I think you
did it to make it consistent with the other lines you were adding with
intra-doc links), but I think in general it is better to clean those
separately in a patch first.
I am mentioning it because the docs do not build due to those --
please check the `rustdoc` target for patches, especially if it is a
non-trivial change.
I also did another change to make the examples (in the other patch)
build with the minimum Rust version. It is good to test that too,
since sometimes that can slip, especially as the window of versions
grow.
Anyway, the examples/series here caught another issue with a previous
patch, so that is good news :)
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 6/6] rust: list: remove OFFSET constants
2025-07-19 21:09 ` Miguel Ojeda
@ 2025-07-19 21:17 ` Tamir Duberstein
0 siblings, 0 replies; 10+ messages in thread
From: Tamir Duberstein @ 2025-07-19 21:17 UTC (permalink / raw)
To: Miguel Ojeda
Cc: 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, rust-for-linux, linux-kernel, linux-pci
On Sat, Jul 19, 2025 at 5:09 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Wed, Jul 9, 2025 at 9:31 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > -/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
> > +/// Declares that this type has a [`ListLinks<ID>`] field.
>
> I was applying this series the other day, and I noticed these
> doc-related changes in the patch, which are appreciated (I think you
> did it to make it consistent with the other lines you were adding with
> intra-doc links), but I think in general it is better to clean those
> separately in a patch first.
>
> I am mentioning it because the docs do not build due to those --
> please check the `rustdoc` target for patches, especially if it is a
> non-trivial change.
>
> I also did another change to make the examples (in the other patch)
> build with the minimum Rust version. It is good to test that too,
> since sometimes that can slip, especially as the window of versions
> grow.
>
> Anyway, the examples/series here caught another issue with a previous
> patch, so that is good news :)
>
> Thanks!
>
> Cheers,
> Miguel
Thanks Miguel, will do in the future!
Tamir
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET
2025-07-09 19:31 [PATCH v4 0/6] rust: list: remove HasListLinks::OFFSET Tamir Duberstein
` (5 preceding siblings ...)
2025-07-09 19:31 ` [PATCH v4 6/6] rust: list: remove OFFSET constants Tamir Duberstein
@ 2025-07-19 22:16 ` Miguel Ojeda
6 siblings, 0 replies; 10+ messages in thread
From: Miguel Ojeda @ 2025-07-19 22:16 UTC (permalink / raw)
To: Tamir Duberstein
Cc: 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, rust-for-linux, linux-kernel, linux-pci,
Christian Schrefl
On Wed, Jul 9, 2025 at 9:31 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> The bulk of this change occurs in the last commit, please see its commit
> messages for details.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Applied to `rust-next` -- thanks everyone!
[ Fixed Rust < 1.82 build by enabling the `offset_of_nested`
feature. - Miguel ]
[ Fixed broken intra-doc links. Used the renamed
`Opaque::cast_into`. - Miguel ]
Cheers,
Miguel
^ permalink raw reply [flat|nested] 10+ messages in thread