rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add Opaque::cast_from
@ 2025-06-24 15:27 ` Alice Ryhl
  2025-06-24 15:27   ` [PATCH v2 1/2] rust: types: add Opaque::cast_from Alice Ryhl
                     ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Alice Ryhl @ 2025-06-24 15:27 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Danilo Krummrich
  Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, dri-devel, rust-for-linux,
	linux-kernel, Alice Ryhl

Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
`container_of!`") we have enforced that the field pointer passed to
container_of! must match the declared field. This caused mismatches when
using a pointer to bindings::x for fields of type Opaque<bindings::x>.

This situation encourages the user to simply pass field.cast() to the
container_of! macro, but this is not great because you might
accidentally pass a *mut bindings::y when the field type is
Opaque<bindings::x>, which would be wrong.

To help catch this kind of mistake, add a new Opaque::cast_from that
wraps a raw pointer in Opaque without changing the inner type. Also
rename raw_get() to cast_into() for naming consistency.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v2:
- Use new naming, and rename raw_get().
- Link to v1: https://lore.kernel.org/r/20250617-opaque-from-raw-v1-1-a2e99efa3ba2@google.com

---
Alice Ryhl (2):
      rust: types: add Opaque::cast_from
      rust: types: rename Opaque::raw_get to cast_into

 rust/kernel/configfs.rs                |  2 +-
 rust/kernel/drm/device.rs              |  4 +---
 rust/kernel/drm/gem/mod.rs             |  4 +---
 rust/kernel/init.rs                    |  6 +++---
 rust/kernel/lib.rs                     |  7 +++++++
 rust/kernel/list.rs                    |  2 +-
 rust/kernel/list/impl_list_item_mod.rs |  4 ++--
 rust/kernel/time/hrtimer.rs            |  4 ++--
 rust/kernel/types.rs                   | 11 ++++++++---
 rust/kernel/workqueue.rs               |  2 +-
 10 files changed, 27 insertions(+), 19 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250617-opaque-from-raw-ac5b8ef6faa2

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>


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

* [PATCH v2 1/2] rust: types: add Opaque::cast_from
  2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
@ 2025-06-24 15:27   ` Alice Ryhl
  2025-06-24 15:27   ` [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into Alice Ryhl
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Alice Ryhl @ 2025-06-24 15:27 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Danilo Krummrich
  Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, dri-devel, rust-for-linux,
	linux-kernel, Alice Ryhl

Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
`container_of!`") we have enforced that the field pointer passed to
container_of! must match the declared field. This caused mismatches when
using a pointer to bindings::x for fields of type Opaque<bindings::x>.

This situation encourages the user to simply pass field.cast() to the
container_of! macro, but this is not great because you might
accidentally pass a *mut bindings::y when the field type is
Opaque<bindings::x>, which would be wrong.

To help catch this kind of mistake, add a new Opaque::cast_from that
wraps a raw pointer in Opaque without changing the inner type. Also
update the docs to reflect this as well as some existing users.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/drm/device.rs  | 4 +---
 rust/kernel/drm/gem/mod.rs | 4 +---
 rust/kernel/lib.rs         | 7 +++++++
 rust/kernel/types.rs       | 5 +++++
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 624d7a4c83ead64b93325189f481d9b37c3c6eae..5731746eaea63296f418e34892e05c9960c45a90 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -135,11 +135,9 @@ pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
     ///
     /// `ptr` must be a valid pointer to a `struct device` embedded in `Self`.
     unsafe fn from_drm_device(ptr: *const bindings::drm_device) -> *mut Self {
-        let ptr: *const Opaque<bindings::drm_device> = ptr.cast();
-
         // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a
         // `struct drm_device` embedded in `Self`.
-        unsafe { crate::container_of!(ptr, Self, dev) }.cast_mut()
+        unsafe { crate::container_of!(Opaque::cast_from(ptr), Self, dev) }.cast_mut()
     }
 
     /// Not intended to be called externally, except via declare_drm_ioctls!()
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 4cd69fa84318c3ff2cec57949e9bab05559a3c2f..6f914ae0a5aade12a50eb15a029362d4d636bf76 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -125,11 +125,9 @@ fn as_raw(&self) -> *mut bindings::drm_gem_object {
     }
 
     unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self {
-        let self_ptr: *mut Opaque<bindings::drm_gem_object> = self_ptr.cast();
-
         // SAFETY: `obj` is guaranteed to be in an `Object<T>` via the safety contract of this
         // function
-        unsafe { &*crate::container_of!(self_ptr, Object<T>, obj) }
+        unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object<T>, obj) }
     }
 }
 
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c37f4da1866e993be6230bc6715841..529ce907499679997415cbaa6a97ac3939587a11 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -204,6 +204,13 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
 
 /// Produces a pointer to an object from a pointer to one of its fields.
 ///
+/// If you encounter a type mismatch due to the [`Opaque`] type, then use [`Opaque::raw_get`] or
+/// [`Opaque::cast_from`] to resolve the mismatch.
+///
+/// [`Opaque`]: crate::types::Opaque
+/// [`Opaque::raw_get`]: crate::types::Opaque::raw_get
+/// [`Opaque::cast_from`]: crate::types::Opaque::cast_from
+///
 /// # Safety
 ///
 /// The pointer passed to this macro, and the pointer returned by this macro, must both be in
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 22985b6f69820d6df8ff3aae0bf815fad36a9d92..82e0c6fa5646dfd2aea52c8fe9ada769ad427e50 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -413,6 +413,11 @@ pub const fn get(&self) -> *mut T {
     pub const fn raw_get(this: *const Self) -> *mut T {
         UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()
     }
+
+    /// The opposite operation of [`Opaque::raw_get`].
+    pub const fn cast_from(this: *const T) -> *const Self {
+        this.cast()
+    }
 }
 
 /// Types that are _always_ reference counted.

-- 
2.50.0.714.g196bf9f422-goog


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

* [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into
  2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
  2025-06-24 15:27   ` [PATCH v2 1/2] rust: types: add Opaque::cast_from Alice Ryhl
@ 2025-06-24 15:27   ` Alice Ryhl
  2025-06-25 12:45     ` kernel test robot
                       ` (2 more replies)
  2025-06-24 15:35   ` [PATCH v2 0/2] Add Opaque::cast_from Danilo Krummrich
                     ` (4 subsequent siblings)
  6 siblings, 3 replies; 11+ messages in thread
From: Alice Ryhl @ 2025-06-24 15:27 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Danilo Krummrich
  Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, dri-devel, rust-for-linux,
	linux-kernel, Alice Ryhl

In the previous patch we added Opaque::cast_from() that performs the
opposite operation to Opaque::raw_get(). For consistency with this
naming, rename raw_get() to cast_from().

There are a few other options such as calling cast_from() something
closer to raw_get() rather than renaming this method. However, I could
not find a great naming scheme that works with raw_get(). The previous
version of this patch used from_raw(), but functions of that name
typically have a different signature, so that's not a great option.

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/configfs.rs                | 2 +-
 rust/kernel/init.rs                    | 6 +++---
 rust/kernel/lib.rs                     | 4 ++--
 rust/kernel/list.rs                    | 2 +-
 rust/kernel/list/impl_list_item_mod.rs | 4 ++--
 rust/kernel/time/hrtimer.rs            | 4 ++--
 rust/kernel/types.rs                   | 8 ++++----
 rust/kernel/workqueue.rs               | 2 +-
 8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 34d0bea4f9a517e890e5e0cdaa86698d397cd3b8..05d8c8ecfbb743ec60b13659f76c1d39e356d51a 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -279,7 +279,7 @@ pub fn new(
 // within the `group` field.
 unsafe impl<Data> HasGroup<Data> for Group<Data> {
     unsafe fn group(this: *const Self) -> *const bindings::config_group {
-        Opaque::raw_get(
+        Opaque::cast_into(
             // SAFETY: By impl and function safety requirements this field
             // projection is within bounds of the allocation.
             unsafe { &raw const (*this).group },
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 8d228c23795445e379b40f662e1c355a934cbd13..cf5eaad8c730deb3ae400bced1f226c8a93e76fb 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -108,13 +108,13 @@
 //!                 let foo = addr_of_mut!((*slot).foo);
 //!
 //!                 // Initialize the `foo`
-//!                 bindings::init_foo(Opaque::raw_get(foo));
+//!                 bindings::init_foo(Opaque::cast_into(foo));
 //!
 //!                 // Try to enable it.
-//!                 let err = bindings::enable_foo(Opaque::raw_get(foo), flags);
+//!                 let err = bindings::enable_foo(Opaque::cast_into(foo), flags);
 //!                 if err != 0 {
 //!                     // Enabling has failed, first clean up the foo and then return the error.
-//!                     bindings::destroy_foo(Opaque::raw_get(foo));
+//!                     bindings::destroy_foo(Opaque::cast_into(foo));
 //!                     return Err(Error::from_errno(err));
 //!                 }
 //!
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 529ce907499679997415cbaa6a97ac3939587a11..f61ac6f81f5d7cfd97045438bae25884aa1a141b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -204,11 +204,11 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
 
 /// Produces a pointer to an object from a pointer to one of its fields.
 ///
-/// If you encounter a type mismatch due to the [`Opaque`] type, then use [`Opaque::raw_get`] or
+/// If you encounter a type mismatch due to the [`Opaque`] type, then use [`Opaque::cast_into`] or
 /// [`Opaque::cast_from`] to resolve the mismatch.
 ///
 /// [`Opaque`]: crate::types::Opaque
-/// [`Opaque::raw_get`]: crate::types::Opaque::raw_get
+/// [`Opaque::cast_into`]: crate::types::Opaque::cast_into
 /// [`Opaque::cast_from`]: crate::types::Opaque::cast_from
 ///
 /// # Safety
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index c391c30b80f890273a0c2ce5ae1a6a66ecedce90..dd9d4d8f63d0953432e1e9c033ff6185692b6a87 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -284,7 +284,7 @@ pub fn new() -> impl PinInit<Self> {
     #[inline]
     unsafe fn fields(me: *mut Self) -> *mut ListLinksFields {
         // SAFETY: The caller promises that the pointer is valid.
-        unsafe { Opaque::raw_get(ptr::addr_of!((*me).inner)) }
+        unsafe { Opaque::cast_into(ptr::addr_of!((*me).inner)) }
     }
 
     /// # Safety
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index a0438537cee12d4a1772a9f12dd6eb6b772060db..9df7c52604947342cc0172a620e7014a7fea7062 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -209,7 +209,7 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
                 // 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 cell_inner = $crate::types::Opaque::raw_get(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`,
                 // `post_remove`, or `view_value`. By the safety requirements of those methods,
@@ -252,7 +252,7 @@ unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const
                 // 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 cell_inner = ::core::cell::UnsafeCell::cast_into(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
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 9df3dcd2fa3942f75ee8471459734621f8dfb16a..3d39bb331d7f10a9dd6cf694b0040d1e9c438685 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -148,7 +148,7 @@ unsafe fn raw_get(this: *const Self) -> *mut bindings::hrtimer {
         // SAFETY: The field projection to `timer` does not go out of bounds,
         // because the caller of this function promises that `this` points to an
         // allocation of at least the size of `Self`.
-        unsafe { Opaque::raw_get(core::ptr::addr_of!((*this).timer)) }
+        unsafe { Opaque::cast_into(core::ptr::addr_of!((*this).timer)) }
     }
 
     /// Cancel an initialized and potentially running timer.
@@ -172,7 +172,7 @@ unsafe fn raw_get(this: *const Self) -> *mut bindings::hrtimer {
     /// `this` must point to a valid `Self`.
     pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
         // SAFETY: `this` points to an allocation of at least `HrTimer` size.
-        let c_timer_ptr = unsafe { HrTimer::raw_get(this) };
+        let c_timer_ptr = unsafe { HrTimer::cast_into(this) };
 
         // If the handler is running, this will wait for the handler to return
         // before returning.
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 82e0c6fa5646dfd2aea52c8fe9ada769ad427e50..382e9f704c3eb55e2c16ae666c06c165e07b3397 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -377,7 +377,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> {
         // initialize the `T`.
         unsafe {
             pin_init::pin_init_from_closure::<_, ::core::convert::Infallible>(move |slot| {
-                init_func(Self::raw_get(slot));
+                init_func(Self::cast_into(slot));
                 Ok(())
             })
         }
@@ -397,7 +397,7 @@ pub fn try_ffi_init<E>(
         // SAFETY: We contain a `MaybeUninit`, so it is OK for the `init_func` to not fully
         // initialize the `T`.
         unsafe {
-            pin_init::pin_init_from_closure::<_, E>(move |slot| init_func(Self::raw_get(slot)))
+            pin_init::pin_init_from_closure::<_, E>(move |slot| init_func(Self::cast_into(slot)))
         }
     }
 
@@ -410,11 +410,11 @@ pub const fn get(&self) -> *mut T {
     ///
     /// This function is useful to get access to the value without creating intermediate
     /// references.
-    pub const fn raw_get(this: *const Self) -> *mut T {
+    pub const fn cast_into(this: *const Self) -> *mut T {
         UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()
     }
 
-    /// The opposite operation of [`Opaque::raw_get`].
+    /// The opposite operation of [`Opaque::cast_into`].
     pub const fn cast_from(this: *const T) -> *const Self {
         this.cast()
     }
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index d092112d843f3225ea582e013581b39e36652a84..a3085a56419db9a7d4c4b757ddd0964b59e453f9 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -403,7 +403,7 @@ pub unsafe fn raw_get(ptr: *const Self) -> *mut bindings::work_struct {
         //
         // A pointer cast would also be ok due to `#[repr(transparent)]`. We use `addr_of!` so that
         // the compiler does not complain that the `work` field is unused.
-        unsafe { Opaque::raw_get(core::ptr::addr_of!((*ptr).work)) }
+        unsafe { Opaque::cast_into(core::ptr::addr_of!((*ptr).work)) }
     }
 }
 

-- 
2.50.0.714.g196bf9f422-goog


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

* Re: [PATCH v2 0/2] Add Opaque::cast_from
  2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
  2025-06-24 15:27   ` [PATCH v2 1/2] rust: types: add Opaque::cast_from Alice Ryhl
  2025-06-24 15:27   ` [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into Alice Ryhl
@ 2025-06-24 15:35   ` Danilo Krummrich
  2025-06-24 15:50   ` Boqun Feng
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-06-24 15:35 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, dri-devel, rust-for-linux, linux-kernel

On 6/24/25 5:27 PM, Alice Ryhl wrote:
> Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
> `container_of!`") we have enforced that the field pointer passed to
> container_of! must match the declared field. This caused mismatches when
> using a pointer to bindings::x for fields of type Opaque<bindings::x>.
> 
> This situation encourages the user to simply pass field.cast() to the
> container_of! macro, but this is not great because you might
> accidentally pass a *mut bindings::y when the field type is
> Opaque<bindings::x>, which would be wrong.
> 
> To help catch this kind of mistake, add a new Opaque::cast_from that
> wraps a raw pointer in Opaque without changing the inner type. Also
> rename raw_get() to cast_into() for naming consistency.

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

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

* Re: [PATCH v2 0/2] Add Opaque::cast_from
  2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
                     ` (2 preceding siblings ...)
  2025-06-24 15:35   ` [PATCH v2 0/2] Add Opaque::cast_from Danilo Krummrich
@ 2025-06-24 15:50   ` Boqun Feng
  2025-07-15  7:34   ` Andreas Hindborg
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Boqun Feng @ 2025-06-24 15:50 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Danilo Krummrich, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, dri-devel, rust-for-linux, linux-kernel

On Tue, Jun 24, 2025 at 03:27:54PM +0000, Alice Ryhl wrote:
> Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
> `container_of!`") we have enforced that the field pointer passed to
> container_of! must match the declared field. This caused mismatches when
> using a pointer to bindings::x for fields of type Opaque<bindings::x>.
> 
> This situation encourages the user to simply pass field.cast() to the
> container_of! macro, but this is not great because you might
> accidentally pass a *mut bindings::y when the field type is
> Opaque<bindings::x>, which would be wrong.
> 
> To help catch this kind of mistake, add a new Opaque::cast_from that
> wraps a raw pointer in Opaque without changing the inner type. Also
> rename raw_get() to cast_into() for naming consistency.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Acked-by: Boqun Feng <boqun.feng@gmail.com>

Thanks!

Regards,
Boqun

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

* Re: [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into
  2025-06-24 15:27   ` [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into Alice Ryhl
@ 2025-06-25 12:45     ` kernel test robot
  2025-06-25 22:18     ` Benno Lossin
  2025-07-17 13:38     ` Miguel Ojeda
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2025-06-25 12:45 UTC (permalink / raw)
  To: Alice Ryhl, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Miguel Ojeda, Danilo Krummrich
  Cc: llvm, oe-kbuild-all, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, dri-devel,
	rust-for-linux, linux-kernel, Alice Ryhl

Hi Alice,

kernel test robot noticed the following build errors:

[auto build test ERROR on 19272b37aa4f83ca52bdf9c16d5d81bdd1354494]

url:    https://github.com/intel-lab-lkp/linux/commits/Alice-Ryhl/rust-types-add-Opaque-cast_from/20250624-232939
base:   19272b37aa4f83ca52bdf9c16d5d81bdd1354494
patch link:    https://lore.kernel.org/r/20250624-opaque-from-raw-v2-2-e4da40bdc59c%40google.com
patch subject: [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250625/202506252207.uYOBf8SU-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250625/202506252207.uYOBf8SU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506252207.uYOBf8SU-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0599]: no function or associated item named `cast_into` found for struct `HrTimer` in the current scope
   --> rust/kernel/time/hrtimer.rs:175:45
   |
   96  | #[pin_data]
   | ----------- function or associated item `cast_into` not found for this struct
   ...
   175 |         let c_timer_ptr = unsafe { HrTimer::cast_into(this) };
   |                                             ^^^^^^^^^ function or associated item not found in `HrTimer<_>`

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into
  2025-06-24 15:27   ` [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into Alice Ryhl
  2025-06-25 12:45     ` kernel test robot
@ 2025-06-25 22:18     ` Benno Lossin
  2025-07-17 13:38     ` Miguel Ojeda
  2 siblings, 0 replies; 11+ messages in thread
From: Benno Lossin @ 2025-06-25 22:18 UTC (permalink / raw)
  To: Alice Ryhl, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Miguel Ojeda, Danilo Krummrich
  Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
	Trevor Gross, dri-devel, rust-for-linux, linux-kernel

On Tue Jun 24, 2025 at 5:27 PM CEST, Alice Ryhl wrote:
> In the previous patch we added Opaque::cast_from() that performs the
> opposite operation to Opaque::raw_get(). For consistency with this
> naming, rename raw_get() to cast_from().
>
> There are a few other options such as calling cast_from() something
> closer to raw_get() rather than renaming this method. However, I could
> not find a great naming scheme that works with raw_get(). The previous
> version of this patch used from_raw(), but functions of that name
> typically have a different signature, so that's not a great option.
>
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/configfs.rs                | 2 +-
>  rust/kernel/init.rs                    | 6 +++---
>  rust/kernel/lib.rs                     | 4 ++--
>  rust/kernel/list.rs                    | 2 +-
>  rust/kernel/list/impl_list_item_mod.rs | 4 ++--
>  rust/kernel/time/hrtimer.rs            | 4 ++--
>  rust/kernel/types.rs                   | 8 ++++----
>  rust/kernel/workqueue.rs               | 2 +-
>  8 files changed, 16 insertions(+), 16 deletions(-)

For init.rs:

Acked-by: Benno Lossin <lossin@kernel.org>

---
Cheers,
Benno

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

* Re: [PATCH v2 0/2] Add Opaque::cast_from
  2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
                     ` (3 preceding siblings ...)
  2025-06-24 15:50   ` Boqun Feng
@ 2025-07-15  7:34   ` Andreas Hindborg
  2025-07-15  8:27   ` Danilo Krummrich
  2025-07-16 22:49   ` Miguel Ojeda
  6 siblings, 0 replies; 11+ messages in thread
From: Andreas Hindborg @ 2025-07-15  7:34 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Danilo Krummrich, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Trevor Gross,
	dri-devel, rust-for-linux, linux-kernel

"Alice Ryhl" <aliceryhl@google.com> writes:

> Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
> `container_of!`") we have enforced that the field pointer passed to
> container_of! must match the declared field. This caused mismatches when
> using a pointer to bindings::x for fields of type Opaque<bindings::x>.
>
> This situation encourages the user to simply pass field.cast() to the
> container_of! macro, but this is not great because you might
> accidentally pass a *mut bindings::y when the field type is
> Opaque<bindings::x>, which would be wrong.
>
> To help catch this kind of mistake, add a new Opaque::cast_from that
> wraps a raw pointer in Opaque without changing the inner type. Also
> rename raw_get() to cast_into() for naming consistency.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

I think your search/replace was a bit too general in hrtimer.rs. When
you fix that:

Acked-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg




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

* Re: [PATCH v2 0/2] Add Opaque::cast_from
  2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
                     ` (4 preceding siblings ...)
  2025-07-15  7:34   ` Andreas Hindborg
@ 2025-07-15  8:27   ` Danilo Krummrich
  2025-07-16 22:49   ` Miguel Ojeda
  6 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-07-15  8:27 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, dri-devel, rust-for-linux, linux-kernel

On Tue Jun 24, 2025 at 5:27 PM CEST, Alice Ryhl wrote:
> Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
> `container_of!`") we have enforced that the field pointer passed to
> container_of! must match the declared field. This caused mismatches when
> using a pointer to bindings::x for fields of type Opaque<bindings::x>.
>
> This situation encourages the user to simply pass field.cast() to the
> container_of! macro, but this is not great because you might
> accidentally pass a *mut bindings::y when the field type is
> Opaque<bindings::x>, which would be wrong.
>
> To help catch this kind of mistake, add a new Opaque::cast_from that
> wraps a raw pointer in Opaque without changing the inner type. Also
> rename raw_get() to cast_into() for naming consistency.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

For the series,

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

For the DRM parts,

Acked-by: Danilo Krummrich <dakr@kernel.org>

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

* Re: [PATCH v2 0/2] Add Opaque::cast_from
  2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
                     ` (5 preceding siblings ...)
  2025-07-15  8:27   ` Danilo Krummrich
@ 2025-07-16 22:49   ` Miguel Ojeda
  6 siblings, 0 replies; 11+ messages in thread
From: Miguel Ojeda @ 2025-07-16 22:49 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Danilo Krummrich, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, dri-devel, rust-for-linux, linux-kernel

On Tue, Jun 24, 2025 at 5:28 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Since commit b20fbbc08a36 ("rust: check type of `$ptr` in
> `container_of!`") we have enforced that the field pointer passed to
> container_of! must match the declared field. This caused mismatches when
> using a pointer to bindings::x for fields of type Opaque<bindings::x>.
>
> This situation encourages the user to simply pass field.cast() to the
> container_of! macro, but this is not great because you might
> accidentally pass a *mut bindings::y when the field type is
> Opaque<bindings::x>, which would be wrong.
>
> To help catch this kind of mistake, add a new Opaque::cast_from that
> wraps a raw pointer in Opaque without changing the inner type. Also
> rename raw_get() to cast_into() for naming consistency.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Applied (yesterday) to `rust-next` -- thanks everyone!

    [ Removed `HrTimer::raw_get` change. - Miguel ]

Cheers,
Miguel

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

* Re: [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into
  2025-06-24 15:27   ` [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into Alice Ryhl
  2025-06-25 12:45     ` kernel test robot
  2025-06-25 22:18     ` Benno Lossin
@ 2025-07-17 13:38     ` Miguel Ojeda
  2 siblings, 0 replies; 11+ messages in thread
From: Miguel Ojeda @ 2025-07-17 13:38 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Danilo Krummrich, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, dri-devel, rust-for-linux, linux-kernel

On Tue, Jun 24, 2025 at 5:28 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> -                let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
> +                let cell_inner = ::core::cell::UnsafeCell::cast_into(self_ptr);

Bah, we also missed this one -- I will rebase / send a patch.

Cheers,
Miguel

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

end of thread, other threads:[~2025-07-17 13:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <OeCi_sbX1kfHFfD5rJSghX6nJ6nZA25Ak9sBH6PvR8BqWfBDlxdB0ejBjYgqbzD_m18QNDU4BdjsKdtdSq4lrA==@protonmail.internalid>
2025-06-24 15:27 ` [PATCH v2 0/2] Add Opaque::cast_from Alice Ryhl
2025-06-24 15:27   ` [PATCH v2 1/2] rust: types: add Opaque::cast_from Alice Ryhl
2025-06-24 15:27   ` [PATCH v2 2/2] rust: types: rename Opaque::raw_get to cast_into Alice Ryhl
2025-06-25 12:45     ` kernel test robot
2025-06-25 22:18     ` Benno Lossin
2025-07-17 13:38     ` Miguel Ojeda
2025-06-24 15:35   ` [PATCH v2 0/2] Add Opaque::cast_from Danilo Krummrich
2025-06-24 15:50   ` Boqun Feng
2025-07-15  7:34   ` Andreas Hindborg
2025-07-15  8:27   ` Danilo Krummrich
2025-07-16 22:49   ` Miguel Ojeda

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).