Skip to content

Commit c77f85b

Browse files
tamirdojeda
tamird
authored and
ojeda
committed
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>
1 parent 5d840b4 commit c77f85b

2 files changed

Lines changed: 81 additions & 87 deletions

File tree

‎rust/kernel/list.rs‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,11 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField};
5757
/// }
5858
/// }
5959
///
60-
/// impl_has_list_links! {
61-
/// impl HasListLinks<0> for BasicItem { self.links }
62-
/// }
6360
/// impl_list_arc_safe! {
6461
/// impl ListArcSafe<0> for BasicItem { untracked; }
6562
/// }
6663
/// impl_list_item! {
67-
/// impl ListItem<0> for BasicItem { using ListLinks; }
64+
/// impl ListItem<0> for BasicItem { using ListLinks { self.links }; }
6865
/// }
6966
///
7067
/// // Create a new empty list.
@@ -320,9 +317,6 @@ unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
320317
unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
321318

322319
impl<T: ?Sized, const ID: u64> ListLinksSelfPtr<T, ID> {
323-
/// The offset from the [`ListLinks`] to the self pointer field.
324-
pub const LIST_LINKS_SELF_PTR_OFFSET: usize = core::mem::offset_of!(Self, self_ptr);
325-
326320
/// Creates a new initializer for this type.
327321
pub fn new() -> impl PinInit<Self> {
328322
// INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
@@ -337,6 +331,16 @@ impl<T: ?Sized, const ID: u64> ListLinksSelfPtr<T, ID> {
337331
self_ptr: Opaque::uninit(),
338332
}
339333
}
334+
335+
/// Returns a pointer to the self pointer.
336+
///
337+
/// # Safety
338+
///
339+
/// The provided pointer must point at a valid struct of type `Self`.
340+
pub unsafe fn raw_get_self_ptr(me: *const Self) -> *const Opaque<*const T> {
341+
// SAFETY: The caller promises that the pointer is valid.
342+
unsafe { ptr::addr_of!((*me).self_ptr) }
343+
}
340344
}
341345

342346
impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {
@@ -711,14 +715,11 @@ impl<'a, T: ?Sized + ListItem<ID>, const ID: u64> Iterator for Iter<'a, T, ID> {
711715
/// }
712716
/// }
713717
///
714-
/// kernel::list::impl_has_list_links! {
715-
/// impl HasListLinks<0> for ListItem { self.links }
716-
/// }
717718
/// kernel::list::impl_list_arc_safe! {
718719
/// impl ListArcSafe<0> for ListItem { untracked; }
719720
/// }
720721
/// kernel::list::impl_list_item! {
721-
/// impl ListItem<0> for ListItem { using ListLinks; }
722+
/// impl ListItem<0> for ListItem { using ListLinks { self.links }; }
722723
/// }
723724
///
724725
/// // Use a cursor to remove the first element with the given value.

‎rust/kernel/list/impl_list_item_mod.rs‎

Lines changed: 69 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -4,36 +4,27 @@
44

55
//! Helpers for implementing list traits safely.
66
7-
/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
7+
/// Declares that this type has a [`ListLinks<ID>`] field.
88
///
9-
/// This trait is only used to help implement `ListItem` safely. If `ListItem` is implemented
9+
/// This trait is only used to help implement [`ListItem`] safely. If [`ListItem`] is implemented
1010
/// manually, then this trait is not needed. Use the [`impl_has_list_links!`] macro to implement
1111
/// this trait.
1212
///
1313
/// # Safety
1414
///
15-
/// All values of this type must have a `ListLinks<ID>` field at the given offset.
15+
/// The methods on this trait must have exactly the behavior that the definitions given below have.
1616
///
17-
/// The behavior of `raw_get_list_links` must not be changed.
17+
/// [`ListLinks<ID>`]: crate::list::ListLinks
18+
/// [`ListItem`]: crate::list::ListItem
1819
pub unsafe trait HasListLinks<const ID: u64 = 0> {
19-
/// The offset of the `ListLinks` field.
20-
const OFFSET: usize;
21-
2220
/// Returns a pointer to the [`ListLinks<T, ID>`] field.
2321
///
2422
/// # Safety
2523
///
2624
/// The provided pointer must point at a valid struct of type `Self`.
2725
///
2826
/// [`ListLinks<T, ID>`]: crate::list::ListLinks
29-
// We don't really need this method, but it's necessary for the implementation of
30-
// `impl_has_list_links!` to be correct.
31-
#[inline]
32-
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut crate::list::ListLinks<ID> {
33-
// SAFETY: The caller promises that the pointer is valid. The implementer promises that the
34-
// `OFFSET` constant is correct.
35-
unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast() }
36-
}
27+
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut crate::list::ListLinks<ID>;
3728
}
3829

3930
/// Implements the [`HasListLinks`] trait for the given type.
@@ -46,14 +37,15 @@ macro_rules! impl_has_list_links {
4637
)*) => {$(
4738
// SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
4839
// right type.
49-
//
50-
// The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is
51-
// equivalent to the pointer offset operation in the trait definition.
5240
unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
53-
const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
54-
5541
#[inline]
5642
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
43+
// Statically ensure that `$(.field)*` doesn't follow any pointers.
44+
//
45+
// Cannot be `const` because `$self` may contain generics and E0401 says constants
46+
// "can't use {`Self`,generic parameters} from outer item".
47+
if false { let _: usize = ::core::mem::offset_of!(Self, $($field).*); }
48+
5749
// SAFETY: The caller promises that the pointer is not dangling. We know that this
5850
// expression doesn't follow any pointers, as the `offset_of!` invocation above
5951
// would otherwise not compile.
@@ -64,12 +56,16 @@ macro_rules! impl_has_list_links {
6456
}
6557
pub use impl_has_list_links;
6658

67-
/// Declares that the `ListLinks<ID>` field in this struct is inside a `ListLinksSelfPtr<T, ID>`.
59+
/// Declares that the [`ListLinks<ID>`] field in this struct is inside a
60+
/// [`ListLinksSelfPtr<T, ID>`].
6861
///
6962
/// # Safety
7063
///
71-
/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be
72-
/// inside a `ListLinksSelfPtr<T, ID>`.
64+
/// The [`ListLinks<ID>`] field of this struct at [`HasListLinks<ID>::raw_get_list_links`] must be
65+
/// inside a [`ListLinksSelfPtr<T, ID>`].
66+
///
67+
/// [`ListLinks<ID>`]: crate::list::ListLinks
68+
/// [`ListLinksSelfPtr<T, ID>`]: crate::list::ListLinksSelfPtr
7369
pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
7470
where
7571
Self: HasListLinks<ID>,
@@ -89,8 +85,6 @@ macro_rules! impl_has_list_links_self_ptr {
8985
unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
9086

9187
unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
92-
const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
93-
9488
#[inline]
9589
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
9690
// SAFETY: The caller promises that the pointer is not dangling.
@@ -120,16 +114,12 @@ pub use impl_has_list_links_self_ptr;
120114
/// links: kernel::list::ListLinks,
121115
/// }
122116
///
123-
/// kernel::list::impl_has_list_links! {
124-
/// impl HasListLinks<0> for SimpleListItem { self.links }
125-
/// }
126-
///
127117
/// kernel::list::impl_list_arc_safe! {
128118
/// impl ListArcSafe<0> for SimpleListItem { untracked; }
129119
/// }
130120
///
131121
/// kernel::list::impl_list_item! {
132-
/// impl ListItem<0> for SimpleListItem { using ListLinks; }
122+
/// impl ListItem<0> for SimpleListItem { using ListLinks { self.links }; }
133123
/// }
134124
///
135125
/// struct ListLinksHolder {
@@ -143,16 +133,12 @@ pub use impl_has_list_links_self_ptr;
143133
/// links: ListLinksHolder,
144134
/// }
145135
///
146-
/// kernel::list::impl_has_list_links! {
147-
/// impl{T, U} HasListLinks<0> for ComplexListItem<T, U> { self.links.inner }
148-
/// }
149-
///
150136
/// kernel::list::impl_list_arc_safe! {
151137
/// impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; }
152138
/// }
153139
///
154140
/// kernel::list::impl_list_item! {
155-
/// impl{T, U} ListItem<0> for ComplexListItem<T, U> { using ListLinks; }
141+
/// impl{T, U} ListItem<0> for ComplexListItem<T, U> { using ListLinks { self.links.inner }; }
156142
/// }
157143
/// ```
158144
///
@@ -168,12 +154,8 @@ pub use impl_has_list_links_self_ptr;
168154
/// impl ListArcSafe<0> for SimpleListItem { untracked; }
169155
/// }
170156
///
171-
/// kernel::list::impl_has_list_links_self_ptr! {
172-
/// impl HasSelfPtr<SimpleListItem> for SimpleListItem { self.links }
173-
/// }
174-
///
175157
/// kernel::list::impl_list_item! {
176-
/// impl ListItem<0> for SimpleListItem { using ListLinksSelfPtr; }
158+
/// impl ListItem<0> for SimpleListItem { using ListLinksSelfPtr { self.links }; }
177159
/// }
178160
///
179161
/// struct ListLinksSelfPtrHolder<T, U> {
@@ -191,21 +173,23 @@ pub use impl_has_list_links_self_ptr;
191173
/// impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; }
192174
/// }
193175
///
194-
/// kernel::list::impl_has_list_links_self_ptr! {
195-
/// impl{T, U} HasSelfPtr<ComplexListItem<T, U>> for ComplexListItem<T, U> { self.links.inner }
196-
/// }
197-
///
198176
/// kernel::list::impl_list_item! {
199-
/// impl{T, U} ListItem<0> for ComplexListItem<T, U> { using ListLinksSelfPtr; }
177+
/// impl{T, U} ListItem<0> for ComplexListItem<T, U> {
178+
/// using ListLinksSelfPtr { self.links.inner };
179+
/// }
200180
/// }
201181
/// ```
202182
#[macro_export]
203183
macro_rules! impl_list_item {
204184
(
205185
$(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
206-
using ListLinks;
186+
using ListLinks { self$(.$field:ident)* };
207187
})*
208188
) => {$(
189+
$crate::list::impl_has_list_links! {
190+
impl$({$($generics)*})? HasListLinks<$num> for $self { self$(.$field)* }
191+
}
192+
209193
// SAFETY: See GUARANTEES comment on each method.
210194
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
211195
// GUARANTEES:
@@ -221,20 +205,19 @@ macro_rules! impl_list_item {
221205
}
222206

223207
// GUARANTEES:
224-
// * `me` originates from the most recent call to `prepare_to_insert`, which just added
225-
// `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
226-
// `offset` from `me` so it returns the pointer originally passed to
227-
// `prepare_to_insert`.
208+
// * `me` originates from the most recent call to `prepare_to_insert`, which calls
209+
// `raw_get_list_link`, which is implemented using `addr_of_mut!((*self)$(.$field)*)`.
210+
// This method uses `container_of` to perform the inverse operation, so it returns the
211+
// pointer originally passed to `prepare_to_insert`.
228212
// * The pointer remains valid until the next call to `post_remove` because the caller
229213
// of the most recent call to `prepare_to_insert` promised to retain ownership of the
230214
// `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
231215
// be destroyed while a `ListArc` reference exists.
232216
unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
233-
let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
234217
// SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
235-
// points at the field at offset `offset` in a value of type `Self`. Thus,
236-
// subtracting `offset` from `me` is still in-bounds of the allocation.
237-
unsafe { (me as *const u8).sub(offset) as *const Self }
218+
// points at the field `$field` in a value of type `Self`. Thus, reversing that
219+
// operation is still in-bounds of the allocation.
220+
$crate::container_of!(me, Self, $($field).*)
238221
}
239222

240223
// GUARANTEES:
@@ -251,25 +234,28 @@ macro_rules! impl_list_item {
251234
}
252235

253236
// GUARANTEES:
254-
// * `me` originates from the most recent call to `prepare_to_insert`, which just added
255-
// `offset` to the pointer passed to `prepare_to_insert`. This method subtracts
256-
// `offset` from `me` so it returns the pointer originally passed to
257-
// `prepare_to_insert`.
237+
// * `me` originates from the most recent call to `prepare_to_insert`, which calls
238+
// `raw_get_list_link`, which is implemented using `addr_of_mut!((*self)$(.$field)*)`.
239+
// This method uses `container_of` to perform the inverse operation, so it returns the
240+
// pointer originally passed to `prepare_to_insert`.
258241
unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
259-
let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
260242
// SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
261-
// points at the field at offset `offset` in a value of type `Self`. Thus,
262-
// subtracting `offset` from `me` is still in-bounds of the allocation.
263-
unsafe { (me as *const u8).sub(offset) as *const Self }
243+
// points at the field `$field` in a value of type `Self`. Thus, reversing that
244+
// operation is still in-bounds of the allocation.
245+
$crate::container_of!(me, Self, $($field).*)
264246
}
265247
}
266248
)*};
267249

268250
(
269251
$(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
270-
using ListLinksSelfPtr;
252+
using ListLinksSelfPtr { self$(.$field:ident)* };
271253
})*
272254
) => {$(
255+
$crate::list::impl_has_list_links_self_ptr! {
256+
impl$({$($generics)*})? HasSelfPtr<$self> for $self { self$(.$field)* }
257+
}
258+
273259
// SAFETY: See GUARANTEES comment on each method.
274260
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
275261
// GUARANTEES:
@@ -284,13 +270,15 @@ macro_rules! impl_list_item {
284270
// SAFETY: The caller promises that `me` points at a valid value of type `Self`.
285271
let links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
286272

287-
let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
288-
// Goes via the offset as the field is private.
289-
//
290-
// SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
291-
// the pointer stays in bounds of the allocation.
292-
let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
293-
as *const $crate::types::Opaque<*const Self>;
273+
let container = $crate::container_of!(
274+
links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
275+
);
276+
277+
// SAFETY: By the same reasoning above, `links_field` is a valid pointer.
278+
let self_ptr = unsafe {
279+
$crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
280+
};
281+
294282
let cell_inner = $crate::types::Opaque::cast_into(self_ptr);
295283

296284
// SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
@@ -331,12 +319,17 @@ macro_rules! impl_list_item {
331319
// `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
332320
// be destroyed while a `ListArc` reference exists.
333321
unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self {
334-
let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
335-
// SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
336-
// the pointer stays in bounds of the allocation.
337-
let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
338-
as *const ::core::cell::UnsafeCell<*const Self>;
339-
let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
322+
let container = $crate::container_of!(
323+
links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
324+
);
325+
326+
// SAFETY: By the same reasoning above, `links_field` is a valid pointer.
327+
let self_ptr = unsafe {
328+
$crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
329+
};
330+
331+
let cell_inner = $crate::types::Opaque::cast_into(self_ptr);
332+
340333
// SAFETY: This is not a data race, because the only function that writes to this
341334
// value is `prepare_to_insert`, but by the safety requirements the
342335
// `prepare_to_insert` method may not be called in parallel with `view_value` or

0 commit comments

Comments
 (0)