Re: [PATCH 0/3] Revert "revocable: Revocable resource management"

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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