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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 May 2021 08:33:58 +0300
From:   Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
To:     Rob Herring <robh@...nel.org>
Cc:     hanwoo Choi <cw00.choi@...sung.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/5] Add devm helper for work-queue initialization


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