On 2021/12/23 20:25, Christoph Hellwig wrote:
> Using a per-device unbound workqueue is a bit of an anti-pattern and
> in this case also creates lock ordering problems. Just use a global
> concurrency managed workqueue instead.
Use of a global workqueue for the loop driver itself is fine. But
> @@ -1115,7 +1107,6 @@ static void __loop_clr_fd(struct loop_device *lo)
> /* freeze request queue during the transition */
> blk_mq_freeze_queue(lo->lo_queue);
>
> - destroy_workqueue(lo->workqueue);
is it safe to remove destroy_workqueue() call here?
> spin_lock_irq(&lo->lo_work_lock);
> list_for_each_entry_safe(worker, pos, &lo->idle_worker_list,
> idle_list) {
destroy_workqueue() implies flush_workqueue() which is creating the lock
ordering problem. And I think that flush_workqueue() is required for making
sure that there is no more work to process (i.e. loop_process_work() is
no longer running) before start deleting idle workers.
My understanding is that the problem is not the use of a per-device workqueue
but the need to call flush_workqueue() in order to make sure that all pending
works are completed.
