Re: [PATCH 5/5] rust: serdev: Pause receive callback before calling unbind

[Markus Probst <markus.probst@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 2619 bytes --]

On Sun, 2026-09-06 at 21:13 +0100, Gary Guo wrote:
> On Sun Sep 6, 2026 at 5:20 PM BST, Danilo Krummrich wrote:
> > On Sun Sep 6, 2026 at 5:55 PM CEST, Markus Probst wrote:
> > > @@ -200,10 +203,14 @@ extern "C" fn receive_buf_callback(
> > >          // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > >          let sdev = unsafe { &*sdev.cast::<Device<device::BoundInternal>>() };
> > >  
> > > -        // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > > -        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > > -        // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > > -        let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > > +        // SAFETY:
> > > +        // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> > > +        //   hence it's guaranteed that `Device::set_drvdata()` has been called and stored a
> > > +        //   `Pin<KBox<PrivateData<'_, T>>>`.
> > > +        // - `unbind_callback` calls `serdev_device_pause_rx` before accessing the driver data,
> > > +        //   which guarantees that this function will not overlap with it. Thus we have exclusive
> > > +        //   access.
> > > +        let private_data = unsafe { sdev.as_ref().drvdata_borrow_mut::<PrivateData<'_, T>>() };
> > 
> > This would break the driver core's lifetime design. Any kind of registration
> > (such as class device, auxiliary, IRQ, etc.) may borrow fields from the bus
> > device private data. The whole design is based on the guarantee that we never
> > construct a mutable reference of the bus device private data.
> 
> Mutable references should be fine (of course, provided that the bus actually
> serialize callbacks).
The calls do not overlap.

> 
> It's only problematic now because in absence of pin-init self-reference, the
> immutable borrow is the only mechanism that prevent user from having multiple
> mutable borrow of the data fields.
Why is having multiple mutable borrows, assuming they are from
different fields, problematic?

> 
> Say this code:
> 
>     struct MyDeviceData<'a> {
>         foo: Resource<'a>,
>         bar: Resource<'foo>,
>         baz: Resource<'bar>,
>     }
> 
> pin-init would make `foo` and `bar` be only visible immutably in the projection,
> even from `Pin<&mut MyDeviceData<'_>>`, so the design is still sound.
So its only problematic, if the driver data is not pinned?

Thanks
- Markus Probst

> 
> Best,
> Gary

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]