Re: [PATCH 1/9] workqueue: devres: Add device-managed allocate workqueue

[Krzysztof Kozlowski]

On 23/02/2026 09:56, Andy Shevchenko wrote:
> On Mon, Feb 23, 2026 at 08:27:29AM +0100, Krzysztof Kozlowski wrote:
>> 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().
> 
>>  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?

No problem, I can move it there.

> 
> ...
> 
>> +	ptr = devres_alloc(devm_destroy_workqueue, sizeof(*ptr), GFP_KERNEL);
>> +	if (!ptr)
>> +		return NULL;
>> +
>> +	va_start(args, max_active);
>> +	wq = alloc_workqueue(fmt, flags, max_active, args);
>> +	va_end(args);
>> +	if (wq) {
>> +		*ptr = wq;
>> +		devres_add(dev, ptr);
>> +	} else {
>> +		devres_free(ptr);
>> +	}
> 
> Why not using devm_add_action_or_reset()?

Where? Here? How the code would be simpler, exactly?

> 
> ...
> 
>> +void devm_destroy_workqueue(struct device *dev, void *res)
>> +{
>> +	destroy_workqueue(*(struct workqueue_struct **)res);
>> +}
>> +EXPORT_SYMBOL_GPL(devm_destroy_workqueue);
> 
> Is this going to be used?

It is not used in this patchset, but most of devm-allocators have the
cleanup.


Best regards,
Krzysztof