[greybus-dev] Re: [PATCH] rust: serdev: Synchronize receive callback before calling unbind

On Thu, 2026-09-03 at 22:03 +0000, Markus Probst wrote:

The receive callback and unbind callback now have exclusive access to the drivers private data. Provide mutable references in callbacks to avoid the need for locks in the private data. Remove the Sync requirement.

Signed-off-by: Markus Probst markus.probst@posteo.de

This patch avoids the need for a SpinLock in the patch series https://lore.kernel.org/rust-for-linux/20260827-gb-uart-transport-v2-7-a03bb... .

---

rust/kernel/serdev.rs | 51 ++++++++++++++++++++++---------------- samples/rust/rust_driver_serdev.rs | 2 +- 2 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs index 17ca504b7f8d..44f029ed93fd 100644 --- a/rust/kernel/serdev.rs +++ b/rust/kernel/serdev.rs @@ -106,7 +106,7 @@ pub struct PrivateData<'bound, T: Driver> { /// Whether `receive_buf_callback` is allowed to call `Driver::receive`. /// /// If locked, the receive_buf_callback will be blocked on data reception.

• /// This is the case while the driver is being probed or while [`PrivateData`] is being dropped.

• /// This is the case while the driver is being probed or removed. /// This is necessary, because we need to open the serdev device before the driver has been /// probed in order to allow it to be configured, which allows `receive_buf_callback` to be /// called. Thus we need to block data until probe completes and the driver data becomes

@@ -127,16 +127,6 @@ pub struct PrivateData<'bound, T: Driver> { #[pinned_drop] impl<T: Driver> PinnedDrop for PrivateData<'_, T> { fn drop(self: Pin<&mut Self>) {

•    let mut active = self.active.lock();
  

•    if *active {
  

•        // SAFETY:
  

•        // - We have exclusive access to `self.driver`.
  

•        // - `self.driver` is guaranteed to be initialized.
  

•        unsafe { (*self.driver.get()).assume_init_drop() };
  

•        *active = false;
  

•    }
  

•    drop(active);
  

•    // SAFETY: We have exclusive access to `self.open`.
     if unsafe { *self.open.get() } {
         // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
  

@@ -176,7 +166,20 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi: let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() }; let private_data = ScopeGuard::new_with_data(private_data, |_| { // SAFETY: We just set drvdata to `PrivateData<'_, T>`.

•            drop(unsafe { sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() });
  

•            let private_data = unsafe {
  

•                sdev.as_ref()
  

•                    .drvdata_obtain::<PrivateData<'_, T>>()
  

•                    .unwrap_unchecked()
  

•            };
  

•            let mut active = private_data.active.lock();
  

•            if *active {
  

•                // SAFETY:
  

•                // - We have exclusive access to `private_data.driver`.
  

•                // - `private_data.driver` is guaranteed to be initialized.
  

•                unsafe { (*private_data.driver.get()).assume_init_drop() };
  

•                *active = false;
  

•            }
  

Ignore this hunk.

As Sashiko correctly noticed, this introduces dead code.

Thanks - Markus Probst

         });
         let mut active = private_data.active.lock();

@@ -222,15 +225,21 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) { // and stored a `Pin<KBox<PrivateData<'_, T>>>`. let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };

•    // SAFETY: No one has exclusive access to `private_data.driver`.
  

•    let data = unsafe { &*private_data.driver.get() };
  

•    let mut active = private_data.active.lock();
  

•    // SAFETY: We have exclusive access to `private_data.driver`.
  

•    let data = unsafe { &mut *private_data.driver.get() };
     // SAFETY:
     // - `private_data.driver` is pinned.
     // - `remove_callback` is only ever called after a successful call to `probe_callback`,
     //   hence it's guaranteed that `private_data.driver` was initialized.
  

•    let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
  

•    let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
  

T::unbind(sdev, data_pinned);

•    // SAFETY: We already established that `data` is guaranteed to be initialized.
  

•    unsafe { data.assume_init_drop() };
  

•    *active = false;
  

  }

extern "C" fn receive_buf_callback( @@ -254,13 +263,13 @@ extern "C" fn receive_buf_callback( return length; }

•    // SAFETY: No one has exclusive access to `private_data.driver`.
  

•    let data = unsafe { &*private_data.driver.get() };
  

•    // SAFETY: We have exclusive access to `private_data.driver`.
  

•    let data = unsafe { &mut *private_data.driver.get() };
     // SAFETY:
     // - `private_data.driver` is pinned.
     // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
     //   hence it's guaranteed that `private_data.driver` was initialized.
  

•    let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
  

•    let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
  

// SAFETY: `buf` is guaranteed to be non-null and has the size of `length`. let buf = unsafe { core::slice::from_raw_parts(buf, length) }; @@ -365,7 +374,7 @@ pub trait Driver { type IdInfo: 'static; /// The type of the driver's bus device private data.

• type Data<'bound>: Send + Sync + 'bound;

• type Data<'bound>: Send + 'bound;

/// The table of OF device ids supported by the driver. const OF_ID_TABLE: Option<of::IdTableSelf::IdInfo> = None; @@ -391,7 +400,7 @@ fn probe<'bound>( /// `&Device<Core>` or `&Device<Bound>` reference. For instance. /// /// Otherwise, release operations for driver resources should be performed in `Drop`.

• fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {

• fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&mut Self::Data<'bound>>) { let _ = (sdev, this); }

@@ -402,7 +411,7 @@ fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data< /// Returns the number of bytes accepted. fn receive<'bound>( sdev: &'bound Devicedevice::Bound,

•    this: Pin<&Self::Data<'bound>>,
  

•    this: Pin<&mut Self::Data<'bound>>,
     data: &[u8],
  

  ) -> usize { let _ = (sdev, this, data);

diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs index 51b4898cd855..d00d547234c8 100644 --- a/samples/rust/rust_driver_serdev.rs +++ b/samples/rust/rust_driver_serdev.rs @@ -63,7 +63,7 @@ fn probe<'bound>( fn receive<'bound>( sdev: &'bound serdev::Device<Bound>,

•    _this: Pin<&Self>,
  

•    _this: Pin<&mut Self>,
     data: &[u8],
  

  ) -> usize { sdev.write(data).unwrap_or_default() as usize

---

base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b change-id: 20260903-rust_serdev_ref_mut-4d2285776ae1