On Sun, 25 Oct 2026 13:05:09 +0100 Alice Ryhl <[email protected]> wrote:
> On Sun, Oct 25, 2026 at 12:23 PM Onur Özkan <[email protected]> wrote: > > > > On Wed, 21 Oct 2026 09:38:07 +0000 > > Alice Ryhl <[email protected]> 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/[email protected] > > > > > > 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
