On Thu, Jan 29, 2026 at 09:34:40AM -0400, Jason Gunthorpe wrote: > On Thu, Jan 29, 2026 at 12:38:50PM +0200, Laurent Pinchart wrote: > > On Wed, Jan 28, 2026 at 09:23:22PM -0400, Jason Gunthorpe wrote: > > > On Thu, Jan 29, 2026 at 03:08:22AM +0200, Laurent Pinchart wrote: > > > > > The latter already have robust schemes to help the driver shutdown and > > > > > end the concurrent operations. ie cancel_work_sync(), > > > > > del_timer_sync(), free_irq(), and *notifier_unregister(). > > > > > > > > One a side note, devm_request_irq() is another of the devm_* helpers > > > > that cause race conditions, as interrupt handlers can run right after > > > > .remove() returns, which drivers will most likely not handle correctly. > > > > > > Yes! You *cannot* intermix devm and non-devm approaches without > > > creating very subtle bugs exactly like this. If your subsystem does > > > not provide a "devm register" helper its drivers shouldn't use devm. > > > > I'd relax that rule a bit. There are resources that drivers must never, > > ever access after .remove(), such as MMIO. Using devm_ioremap* should > > therefore be safe in all cases. > > Yeah, probably, but I've seen driver using devm before & after > non-devm and it is just too hard to tell if things are going to > even work right. > > To be fair the IRQ issue is always more involved. The subsystem should > provide a state after unregistration where the memory is still around > and the IRQ path into the subsystem becomes a NOP. The driver then > frees the IRQ, fences work and releases the driver memory. > > It is hard to do this sequence with devm.. > > I think a lot of places manage without this because seeing interrupts > after unregister is probably a rare race condition in their HW. > > > > But sure, it is all easy once you figure out how to give the fops shim > > > some place to store all this state since people would not agree to > > > make this a universal cost to all fops. > > > > I didn't see any push back against Dan's proposal to store that > > information in struct cdev, did I miss something ? > > I also don't see an issue with that, especially if we can stack misc > on top of cdev to share the same logic. > > I think if you take that idea and the other proposal to shim the fops > with ones that use the cdev data then we can see some > cdev_unregister_sync() primitive.
I think we'll need to split that primitive in two (or add a second primitive), as drivers need to wake up thread sleeping in fops between flagging the cdev as being unregistered and completing the unregistration with cdev_unregister_sync(). How to wake those threads up is highly driver-specific (or at least subsystem-specific), so we need two functions. Other than that, I think we're on the same page. -- Regards, Laurent Pinchart
