On 2022/01/26 16:18, Ming Lei wrote:
>>> Isn't that the reason of
>>>
>>> } else if (lo->lo_state == Lo_bound) {
>>> /*
>>> * Otherwise keep thread (if running) and config,
>>> * but flush possible ongoing bios in thread.
>>> */
>>> blk_mq_freeze_queue(lo->lo_queue);
>>> blk_mq_unfreeze_queue(lo->lo_queue);
>>> }
>>>
>>> path in lo_release() being there?
>>
>> This looks completely spurious to me. Adding Ming who added it.
>
> It was added when converting to blk-mq.
Well, commit b5dd2f6047ca1080 ("block: loop: improve performance via blk-mq") in v4.0-rc1.
That's where "thread" in the block comment above became outdated, and a global WQ_MEM_RECLAIM
workqueue was used. We need to update both.
>
> I remember it was to replace original loop_flush() which uses
> wait_for_completion() for draining all inflight bios, but seems
> the flush isn't needed in lo_release().
Even if we can remove blk_mq_freeze_queue()/blk_mq_unfreeze_queue() from lo_release(), we
cannot remove blk_mq_freeze_queue()/blk_mq_unfreeze_queue() from e.g. loop_set_status(), right?
Then, lo_release() which is called with disk->open_mutex held can be still blocked at
mutex_lock(&lo->lo_mutex) waiting for e.g. loop_set_status() to call mutex_unlock(&lo->lo_mutex).
That is, lo_open() from e.g. /sys/power/resume can still wait for I/O completion with disk->open_mutex held.
How to kill this dependency? Don't we need to make sure that lo_open()/lo_release() are not
blocked on lo->lo_mutex (like [PATCH v3 3/5] and [PATCH v3 4/5] does) ?
