rust: list: remove OFFSET constants

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>
Link: https://lore.kernel.org/r/20250709-list-no-offset-v4-6-a429e75840a9@gmail.com
[ Fixed broken intra-doc links. Used the renamed
  `Opaque::cast_into`. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
This commit is contained in:
Tamir Duberstein 2025-07-09 15:31:16 -04:00 committed by Miguel Ojeda
parent 5d840b4c49
commit c77f85b347
2 changed files with 81 additions and 87 deletions

View File

@ -57,14 +57,11 @@
/// } /// }
/// } /// }
/// ///
/// impl_has_list_links! {
/// impl HasListLinks<0> for BasicItem { self.links }
/// }
/// impl_list_arc_safe! { /// impl_list_arc_safe! {
/// impl ListArcSafe<0> for BasicItem { untracked; } /// impl ListArcSafe<0> for BasicItem { untracked; }
/// } /// }
/// impl_list_item! { /// impl_list_item! {
/// impl ListItem<0> for BasicItem { using ListLinks; } /// impl ListItem<0> for BasicItem { using ListLinks { self.links }; }
/// } /// }
/// ///
/// // Create a new empty list. /// // 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> {} unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
impl<T: ?Sized, const ID: u64> 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. /// Creates a new initializer for this type.
pub fn new() -> impl PinInit<Self> { pub fn new() -> impl PinInit<Self> {
// INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will // 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(), 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> { 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! { /// kernel::list::impl_list_arc_safe! {
/// impl ListArcSafe<0> for ListItem { untracked; } /// impl ListArcSafe<0> for ListItem { untracked; }
/// } /// }
/// kernel::list::impl_list_item! { /// 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. /// // Use a cursor to remove the first element with the given value.

View File

@ -4,21 +4,19 @@
//! Helpers for implementing list traits safely. //! 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 /// manually, then this trait is not needed. Use the [`impl_has_list_links!`] macro to implement
/// this trait. /// this trait.
/// ///
/// # Safety /// # 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. /// [`ListLinks<ID>`]: crate::list::ListLinks
/// [`ListItem`]: crate::list::ListItem
pub unsafe trait HasListLinks<const ID: u64 = 0> { 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. /// Returns a pointer to the [`ListLinks<T, ID>`] field.
/// ///
/// # Safety /// # Safety
@ -26,14 +24,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
/// The provided pointer must point at a valid struct of type `Self`. /// The provided pointer must point at a valid struct of type `Self`.
/// ///
/// [`ListLinks<T, ID>`]: crate::list::ListLinks /// [`ListLinks<T, ID>`]: crate::list::ListLinks
// We don't really need this method, but it's necessary for the implementation of unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut crate::list::ListLinks<ID>;
// `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() }
}
} }
/// Implements the [`HasListLinks`] trait for the given type. /// Implements the [`HasListLinks`] trait for the given type.
@ -46,14 +37,15 @@ macro_rules! impl_has_list_links {
)*) => {$( )*) => {$(
// SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
// right type. // 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 { unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
#[inline] #[inline]
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? { 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 // 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 // expression doesn't follow any pointers, as the `offset_of!` invocation above
// would otherwise not compile. // would otherwise not compile.
@ -64,12 +56,16 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
} }
pub use impl_has_list_links; 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 /// # Safety
/// ///
/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be /// The [`ListLinks<ID>`] field of this struct at [`HasListLinks<ID>::raw_get_list_links`] must be
/// inside a `ListLinksSelfPtr<T, ID>`. /// inside a [`ListLinksSelfPtr<T, ID>`].
///
/// [`ListLinks<ID>`]: crate::list::ListLinks
/// [`ListLinksSelfPtr<T, ID>`]: crate::list::ListLinksSelfPtr
pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
where where
Self: HasListLinks<ID>, Self: HasListLinks<ID>,
@ -89,8 +85,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::HasSelfPtr<$item_type $(, $id)?> for $self {}
unsafe impl$(<$($generics)*>)? $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] #[inline]
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? { unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
// SAFETY: The caller promises that the pointer is not dangling. // SAFETY: The caller promises that the pointer is not dangling.
@ -120,16 +114,12 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// links: kernel::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! { /// kernel::list::impl_list_arc_safe! {
/// impl ListArcSafe<0> for SimpleListItem { untracked; } /// impl ListArcSafe<0> for SimpleListItem { untracked; }
/// } /// }
/// ///
/// kernel::list::impl_list_item! { /// kernel::list::impl_list_item! {
/// impl ListItem<0> for SimpleListItem { using ListLinks; } /// impl ListItem<0> for SimpleListItem { using ListLinks { self.links }; }
/// } /// }
/// ///
/// struct ListLinksHolder { /// struct ListLinksHolder {
@ -143,16 +133,12 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// links: ListLinksHolder, /// 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! { /// kernel::list::impl_list_arc_safe! {
/// impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; } /// impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; }
/// } /// }
/// ///
/// kernel::list::impl_list_item! { /// 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 +154,8 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// impl ListArcSafe<0> for SimpleListItem { untracked; } /// 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! { /// 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> { /// struct ListLinksSelfPtrHolder<T, U> {
@ -191,21 +173,23 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
/// impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; } /// 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! { /// 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_export]
macro_rules! impl_list_item { macro_rules! impl_list_item {
( (
$(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty { $(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. // SAFETY: See GUARANTEES comment on each method.
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self { unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
// GUARANTEES: // GUARANTEES:
@ -221,20 +205,19 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
} }
// GUARANTEES: // GUARANTEES:
// * `me` originates from the most recent call to `prepare_to_insert`, which just added // * `me` originates from the most recent call to `prepare_to_insert`, which calls
// `offset` to the pointer passed to `prepare_to_insert`. This method subtracts // `raw_get_list_link`, which is implemented using `addr_of_mut!((*self)$(.$field)*)`.
// `offset` from `me` so it returns the pointer originally passed to // This method uses `container_of` to perform the inverse operation, so it returns the
// `prepare_to_insert`. // pointer originally passed to `prepare_to_insert`.
// * The pointer remains valid until the next call to `post_remove` because the caller // * 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 // 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 // `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
// be destroyed while a `ListArc` reference exists. // be destroyed while a `ListArc` reference exists.
unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self { 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 // 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, // points at the field `$field` in a value of type `Self`. Thus, reversing that
// subtracting `offset` from `me` is still in-bounds of the allocation. // operation is still in-bounds of the allocation.
unsafe { (me as *const u8).sub(offset) as *const Self } $crate::container_of!(me, Self, $($field).*)
} }
// GUARANTEES: // GUARANTEES:
@ -251,25 +234,28 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
} }
// GUARANTEES: // GUARANTEES:
// * `me` originates from the most recent call to `prepare_to_insert`, which just added // * `me` originates from the most recent call to `prepare_to_insert`, which calls
// `offset` to the pointer passed to `prepare_to_insert`. This method subtracts // `raw_get_list_link`, which is implemented using `addr_of_mut!((*self)$(.$field)*)`.
// `offset` from `me` so it returns the pointer originally passed to // This method uses `container_of` to perform the inverse operation, so it returns the
// `prepare_to_insert`. // pointer originally passed to `prepare_to_insert`.
unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self { 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 // 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, // points at the field `$field` in a value of type `Self`. Thus, reversing that
// subtracting `offset` from `me` is still in-bounds of the allocation. // operation is still in-bounds of the allocation.
unsafe { (me as *const u8).sub(offset) as *const Self } $crate::container_of!(me, Self, $($field).*)
} }
} }
)*}; )*};
( (
$(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty { $(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. // SAFETY: See GUARANTEES comment on each method.
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self { unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
// GUARANTEES: // GUARANTEES:
@ -284,13 +270,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`. // 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 links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET; let container = $crate::container_of!(
// Goes via the offset as the field is private. links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
// );
// SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
// the pointer stays in bounds of the allocation. // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
let self_ptr = unsafe { (links_field as *const u8).add(spoff) } let self_ptr = unsafe {
as *const $crate::types::Opaque<*const Self>; $crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
};
let cell_inner = $crate::types::Opaque::cast_into(self_ptr); let cell_inner = $crate::types::Opaque::cast_into(self_ptr);
// SAFETY: This value is not accessed in any other places than `prepare_to_insert`, // SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
@ -331,12 +319,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 // `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
// be destroyed while a `ListArc` reference exists. // be destroyed while a `ListArc` reference exists.
unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self { 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; let container = $crate::container_of!(
// SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
// 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 cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr); let self_ptr = unsafe {
$crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
};
let cell_inner = $crate::types::Opaque::cast_into(self_ptr);
// SAFETY: This is not a data race, because the only function that writes to this // 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 // 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 // `prepare_to_insert` method may not be called in parallel with `view_value` or