ma 23.2.2026 klo 14.13 Andy Shevchenko
(andriy.shevchenko@linux.intel.com) kirjoitti:

+Cc: devm-helpers maintainers/reviewers

On Mon, Feb 23, 2026 at 12:52:14PM +0100, Krzysztof Kozlowski wrote:

quoted

On 23/02/2026 09:56, Andy Shevchenko wrote:

quoted

On Mon, Feb 23, 2026 at 08:27:29AM +0100, Krzysztof Kozlowski wrote:

quoted

Add a Resource-managed version of alloc_workqueue() to fix common
problem of drivers mixing devm() calls with destroy_workqueue.  Such
naive and discouraged driver approach leads to difficult to debug bugs
when the driver:

1. Allocates workqueue in standard way and destroys it in driver
   remove() callback,
2. Sets work struct with devm_work_autocancel(),
3. Registers interrupt handler with devm_request_threaded_irq().

Which leads to following unbind/removal path:

1. destroy_workqueue() via driver remove(),
   Any interrupt coming now would still execute the interrupt handler,
   which queues work on destroyed workqueue.
2. devm_irq_release(),
3. devm_work_drop() -> cancel_work_sync() on destroyed workqueue.

devm_alloc_workqueue() has two benefits:
1. Solves above problem of mix-and-match devres and non-devres code in
   driver,
2. Simplify any sane drivers which were correctly using
   alloc_workqueue() + devm_add_action_or_reset().

quoted

 include/linux/workqueue.h                        | 32 ++++++++++++++++++++++++
 kernel/workqueue.c                               | 32 ++++++++++++++++++++++++

Hmm... We have devm-helpers.h. Why the new one is in workqueue.h?
Can we have some consistency here?

Answering with update:
I don't think this should go to devm-helpers.h. The definition is in
workqueue.c, thus the declaration should be in corresponding header.
It's logical and consistent.

Otherwise, I could move it entirely - definition and declaration - to
devm-helpers.h, but then the release (devm_destroy_workqueue()) will be
essentially exported to everyone through the header.

So kind of conflicting choices.

Hmm... An alternative I see is more intrusive but should make it less
inconsistent: Treat the devm-helpers as devres like header for workqueue
and collect there all devm_*wq* related stuff with maybe something putting
back to / holding in the c-file.

OTOH we may leave devm_destroy_workqueue() visible for now with a comment
saying do not use, it's internal or something like that.

Hans, what would be your opinion as you IIRC is the author of devm-helpers.h?

Matti, I also Cc'ed to you, you have usually non-standard thinkig and
insightful solutions (besides being reviewer of devm-helpers).

Thanks Andy for Cc'ing (and flattering!) me ;)

My personal opinion is that the devm-helpers.h as a "collect 'em all
here" -place might not have been the best idea. Perhaps the original
author didn't know what he was doing :) Problem lurking there is that
such a file collecting misc devm -interfaces in one place, will end up
slowing down the compilation while some people put effort on splitting
the headers.

Currently the devm-helpers.h contains only workqueue related
devm-helpers. I'd consider renaming it to something like "devm-wq.h" -
and then just stuffing the devm-wq stuff in it. Optionally, I'd
considered adding all the wq-devm stiff from the devm-helpers.h to the
regular workqueue headers - but I think this was not liked by
everyone.

So sorry, I have no helpful or strong opinion on this. Besides, You,
Krzysztof and Hans all probably have better insight on this than I do.

Yours,
    -- Matti
-- 

Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));