Re: [PATCH v2 0/5] Add devm helper for work-queue initialization

[Matti Vaittinen]

On Tue, 2021-05-18 at 19:39 -0500, Rob Herring wrote:
> On Tue, May 18, 2021 at 03:31:37PM +0300, Matti Vaittinen wrote:
> > This series adds new devm_work_autocancel() helper.
> > 
> > Many drivers which use work-queues must ensure the work is not
> > queued when
> > driver is detached. Often this is done by ensuring new work is not
> > added and
> > then calling cancel_work_sync() at remove(). In many cases this
> > also requires
> > cleanup at probe error path - which is easy to forget (or get
> > wrong).
> > 
> > Also the "by ensuring new work is not added" has a gotcha.
> > 
> > It is not strange to see devm managed IRQs scheduling work.
> > Mixing this with manual wq clean-up is hard to do correctly because
> > the
> > devm is likely to free the IRQ only after the remove() is ran. So
> > manual
> > wq cancellation and devm-based IRQ management do not mix well -
> > there is
> > a short(?) time-window after the wq clean-up when IRQs are still
> > not
> > freed and may schedule new work.
> > 
> > When both WQs and IRQs are managed by devm things are likely to
> > just
> > work. WQs should be initialized before IRQs (when IRQs need to
> > schedule
> > work) and devm unwinds things in "FILO" order.
> 
> Wouldn't it be better to convert drivers to use threaded IRQ
> handlers?

Morning Rob,

I think for example the extcon drivers were using threaded IRQs. In
general, there may be legitimate use-cases for having threaded IRQs but
still offloading some work to WQ. I guess that for example the
IRQF_ONESHOT usage could be such, right?

Best Regards
	Matti Vaittinen