Re: [PATCH 1/2] loop: use a global workqueue

[Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>]

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.