Re: [PATCH 2/2] drm/tyr: unplug the GPU when reset fails

[Onur Özkan]

On Sun, 25 Oct 2026 13:05:09 +0100
Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:

> On Sun, Oct 25, 2026 at 12:23 PM Onur Özkan <work@xxxxxxxxxxxxx> wrote:
> >
> > On Wed, 21 Oct 2026 09:38:07 +0000
> > Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> >
> > > On Sun, Aug 23, 2026 at 01:50:08PM +0300, Onur Özkan wrote:
> > > > @@ -56,7 +57,7 @@ pub(crate) struct TyrPlatformDriverData<'bound> {
> > > >      // ensure the correct drop order.
> > > >      #[pin]
> > > >      _reset: reset::ResetHandle<'bound>,
> > > > -    _reg: drm::Registration<'bound, TyrDrmDriver>,
> > > > +    _reg: Arc<Revocable<drm::Registration<'bound, TyrDrmDriver>>>,
> > > >      _device: ARef<TyrDrmDevice>,
> > > >  }
> > > >
> > > > @@ -161,17 +162,18 @@ fn probe<'bound>(
> > > >                      gpu_info,
> > > >              });
> > > >
> > > > -            // SAFETY: `reg` is stored in the platform driver data and is not leaked or
> > > > -            // forgotten, so it is dropped before the `'bound` registration data can become
> > > > -            // invalid.
> > > > +            // SAFETY: `reg` is wrapped in a `Revocable` owned by the platform driver
> > > > +            // data and its reset handle. Neither is leaked or forgotten, so `reg` is
> > > > +            // dropped before the `'bound` registration data can become invalid.
> > > >              let reg =
> > > >                  unsafe { drm::Registration::new_with_lt(pdev.as_ref(), unreg_dev, reg_data, 0)? };
> > > >              let device = reg.device().into();
> > > > +            let reg = Arc::pin_init(Revocable::new(reg), GFP_KERNEL)?;
> > >
> > > `drm::Registration::new` requires that the `Registration` is dropped
> > > before `'bound` expires so that `drm_dev_unplug()` runs and
> > > `TyrDrmRegistrationData<'bound>` is dropped while `'bound` is still
> > > valid. Wrapping it in an `Arc` makes this invariant fragile: if any
> > > `Arc` clone outlives `TyrPlatformDriverData` or is leaked,
> > > `drm::Registration` will not be dropped on platform unbind and
> > > `TyrDrmRegistrationData<'bound>` will outlive `'bound`.
> > >
> > > With `pin-init`'s new safe self-referential struct support in `rust-next`
> > > (see e.g. [1]), you can avoid both `Arc` and raw pointers altogether by
> > > storing `_reg: Revocable<drm::Registration<'bound, TyrDrmDriver>>`
> > > directly as a `#[pin]` field of `TyrPlatformDriverData<'bound>` and
> > > having `_reset` borrow it via field + existential lifetimes:
> > >
> > >     #[pin_data]
> > >     pub(crate) struct TyrPlatformDriverData<'bound> {
> > >         #[pin]
> > >         #[uses('_reg: invariant)]
> > >         _reset: reset::ResetHandle<'_reg>,
> > >         #[pin]
> > >         _reg: Revocable<drm::Registration<'bound, TyrDrmDriver>>,
> > >     }
> > >
> > > and in `reset.rs`:
> > >
> > >     #[pin_data]
> > >     struct Controller<'ctrl>
> > >     where
> > >         exists<'bound>: 'ctrl,
> > >     {
> > >         pdev: &'ctrl platform::Device<Bound>,
> > >         state: Atomic<ResetState>,
> > >         hw: Arc<HwGate<'ctrl>>,
> > >         #[uses('bound: invariant)]
> > >         registration:
> > >             Pin<&'ctrl Revocable<drm::Registration<'bound, TyrDrmDriver>>>,
> > >     }
> > >
> > > Then `probe()` simply initializes `_reg` first and passes `_reg` to
> > > `ResetHandle::new(pdev, hw, _reg)?`, `reset_work()` calls
> > > `self.with_registration(|reg| reg.revoke())`, and `#[pin_data]`
> > > statically checks that `_reset` drops before `_reg`.
> > >
> > > Also, as a heads-up for rebasing onto `drm-rust-next`:
> > > `_device: ARef<TyrDrmDevice>` has been removed from
> > > `TyrPlatformDriverData` (along with the empty `PinnedDrop` impl), and
> > > `drm::Registration::new()` now returns `impl PinInit<Self, Error>`
> > > directly without the `flags` argument, so `_reg` can be initialized in
> > > place via `Revocable::new(unsafe { drm::Registration::new(...) })`.
> > >
> > > [1] https://lore.kernel.org/r/20261019133647.1279779-1-gary@xxxxxxxxxx
> > >
> > > Alice
> >
> > Any idea when will we push drm-rust-next changes into rust-next?
> 
> We are currently in the merge window, so drm-rust-next and rust-next
> are both being pushed into Linus's tree. Once the merge window is
> over, drm-rust-next will have everything that was pushed to Linus's
> tree available.
> 
> Alice

Cool, I will wait for that and send new versions for this and the reset series.

- Onur