Re: [PATCH] loop: defer the queue limits clear to a workqueue

[Tao Cui]

Hi Bart,

在 2026/9/26 00:22, Bart Van Assche 写道:
> On 9/24/26 6:29 PM, Tao Cui wrote:
>> 在 2026/9/25 02:01, Bart Van Assche 写道:
>>> On 9/24/26 3:20 AM, Tao Cui wrote:
>>>>        queue_limits_commit_update(lo->lo_queue, &lim);
>>>> +    mutex_unlock(&lo->clear_limits_lock);
>>>
>>> Since the mutex_lock(&lo->clear_limits_lock) call occurs after
>>> queue_limits_start_update(), please move the
>>> mutex_unlock(&lo->clear_limits_lock) call above the
>>> queue_limits_commit_update() call. This has no impact on
>>> correctness nor on the lock nesting order but makes the code
>>> easier to follow.
>>>
>>
>> On moving mutex_unlock() above queue_limits_commit_update(): with
>> the generation counters removed, the mutex is what keeps consuming
>> clear_limits_mode and committing the resulting queue limits atomic
>> with respect to a rebind. blk_mq_freeze_queue() is
>> reference-counted, so the workfn and loop_change_fd() can both hold
>> a freeze concurrently. If the workfn drops the mutex after consuming
>> a non-zero mode but before queue_limits_commit_update(),
>> loop_change_fd() could install a new backing file in that window,
>> and the pending clear would then be committed to the new file. Since
>> loop_change_fd() does not recompute the queue limits afterward, the
>> incorrectly cleared discard limits would persist until the next
>> reconfiguration.
>>
>> For that reason, I'd prefer to keep the mutex held until after
>> queue_limits_commit_update(). I can add a comment explaining this if
>> that would help.
> 
> This seems fragile to me. Can this race be solved by calling
> cancel_work_sync(&lo->clear_limits_work) before the backing file is
> changed?
> 

I looked into this, and that works. The freeze in loop_change_fd()
first drains all in-flight requests, so once it returns no further
clear_limits_work can be queued from the old backing file. Calling
cancel_work_sync() immediately afterwards ensures that any already
queued or running work has finished before the backing file
changes. I'll do the same in __loop_clr_fd().

With that, the mutex only protects clear_limits_mode, and
mutex_unlock() can move back above queue_limits_commit_update().
I'll send v6 with the generation counters removed and
mutex_unlock() moved above queue_limits_commit_update().

Thanks,
Tao.

> Thanks,
> 
> Bart.