On Tue Dec 16, 2025 at 9:14 PM JST, Danilo Krummrich wrote: > On Tue Dec 16, 2025 at 6:13 AM CET, Alexandre Courbot wrote: >> When unbinding a PCI driver, the `T::unbind` callback is invoked by the >> driver framework, passing the driver data as a `Pin<&T>`. >> >> This artificially restricts what the driver can do, as it cannot mutate >> any state on the data. This becomes a problem in e.g. Nova, which needs >> to invoke mutable methods when unbinding. >> >> `remove_callback` retrieves the driver data by value, and drops it right >> after the call to `T::unbind`, meaning it is the only reference to the >> driver data by the time `T::unbind` is called. >> >> There is thus no reason for not granting full ownership of the data to >> `T::unbind`, so do it. > > There are multiple reasons I did avoid this for: > > (1) Race conditions > > A driver can call Device::drvdata() and obtain a reference to the driver's > device private data as long as it has a &Device<Bound> and asserts the correct > type of the driver's device private data [1]. > > Assume you have an IRQ registration, for instance, that lives within this > device > private data. Within the IRQ handler, nothing prevents us from calling > Device::drvdata() given that the IRQ handler has a Device<Bound> reference. > > Consequently, with passing the device private data by value to unbind() it can > happen that we have both a mutable and immutable reference at of the device > private data at the same time. > > The same is true for a lot of other cases, such as work items or workqueues > that > are scoped to the Device being bound living within the device private data. > > More generally, you can't take full ownership of the device private data as > long > as the device is not yet fully unbound (which is not yet the case in > unbind()).
Ah, I completely ignored the fact that we can indeed have other references to the private data! The fact that `unbind` works with `Device<Bounded>` should have given me a hint, but somehow I blissfully ignored that. ^_^; I will implement some basic locking on the command queue so we can work with a non-mutable reference.
