Message-ID: <20260129145208.GF3327197@killaraus>
Date: Thu, 29 Jan 2026 16:52:08 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>,
	Johan Hovold <johan@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Danilo Krummrich <dakr@...nel.org>,
	"Rafael J . Wysocki" <rafael@...nel.org>,
	Tzung-Bi Shih <tzungbi@...nel.org>,
	Linus Walleij <linusw@...nel.org>, Jonathan Corbet <corbet@....net>,
	Shuah Khan <shuah@...nel.org>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Simona Vetter <simona.vetter@...ll.ch>,
	Dan Williams <dan.j.williams@...el.com>, linux-doc@...r.kernel.org,
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
	Bartosz Golaszewski <brgl@...nel.org>
Subject: Re: [PATCH 0/3] Revert "revocable: Revocable resource management"

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

Powered by blists - more mailing lists