Re: [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver

["Gary Guo" <gary@garyguo.net>]

On Wed Jun 3, 2026 at 11:24 PM BST, Danilo Krummrich wrote:
> On Wed Jun 3, 2026 at 1:51 PM CEST, Gary Guo wrote:
>>> +    /// Safe variant of [`Registration::new_with_lt()`] for registration data that does not contain
>>> +    /// borrowed references.
>>> +    pub fn new<E>(
>>
>> This is currently unsound, as leaking the unbind guard also gives out
>> `&Device<Bound>` in addition to the registration data.
>
> For this to be unsound someone would need to be able to move the
> drm::Registration into a context where its Drop runs independently of the driver
> unbind, because otherwise leaking the UnbindGuard would also block driver unbind
> forever and the now unconstraint &Device<Bound> remains valid.
>
> So, I assume you refer to the case where someone calls forget() on the
> drm::Registration that was created without the promise not to do so, i.e. new().

Right, I was indeed thinking about this.

>
>> I think we should just remove the not pass `&Device<Bound>` to ioctl callbacks.
>> Giving back registration data is sufficient; if a device driver needs
>> `&Device<Bound>` it can just store a reference in its registration data; more
>> commonly I suspect it will just store whatever device resource is needed and
>> doesn't need `&Device<Bound>` (with the introduction of lifetime, we have much
>> fewer cases that we actually need `&Device<Bound>` and cannot be replaced with a
>> direct reference to the device resource).
>>
>> Not passing this bound device allows us to make this safe, and also remove the
>> need of patch 1 and patch 5.
>
> I follow your reasoning, but not passing T::ParentDevice<Bound> in the ioctl
> makes Registration::new() rather pointless on its own; given that it takes
> T: 'static you can't store &'a T::ParentDevice<Bound> in the first place.

I suppose you could create a DRM device without it backed by any real hardware
resoruces :)

Joke aside, I think my actual point is that with the current approach, and the
fact that `UnbindGuard` serializes with drop of `drm::Registration`,
`&Device<Bound>` is not really inherent to the design anymore. This can follow
the same pattern as other registration data and there's no need to bring
`&Device<Bound>` into the mix.

You might be right the `new` might not be very useful without storing device
resources; in that case I think you could just remove `new` and have the
`new_with_lt` be the only API.

Best,
Gary