Re: [PATCH 01/13] block: freeze queue when updating zone resources

Damien Le Moal
2025-11-03T07:30:38Z

On 11/3/25 16:18, Daniel Vacek wrote:
> On Mon, 3 Nov 2025 at 06:55, Damien Le Moal <dlemoal@kernel.org> wrote:
>>
>> On 11/1/25 02:48, Bart Van Assche wrote:
>>> Hi Damien,
>>>
>>> disk_update_zone_resources() only has a single caller and just below the
>>> only call of this function the following code is present:
>>>
>>> if (ret) {
>>> unsigned int memflags = blk_mq_freeze_queue(q);
>>>
>>> disk_free_zone_resources(disk);
>>> blk_mq_unfreeze_queue(q, memflags);
>>> }
>>>
>>> Shouldn't this code be moved into disk_update_zone_resources() such that
>>> error handling happens without unfreezing and refreezing the request
>>> queue?
>>
>> Check the code again. disk_free_zone_resources() if the report zones callbacks
>> return an error, and in that case disk_update_zone_resources() is not called.
>> So having this call as it is cover all cases.
>
> I understand Bart's idea was more like below:
>
>> @@ -1568,7 +1572,12 @@ static int disk_update_zone_resources(str
> uct gendisk *disk,
>> }
>>
>> commit:
>> - return queue_limits_commit_update_frozen(q, &lim);
>> + ret = queue_limits_commit_update(q, &lim);
>> +
>> +unfreeze:
>
> + if (ret)
> + disk_free_zone_resources(disk);
>
>> + blk_mq_unfreeze_queue(q, memflags);
>> +
>> + return ret;
>> }
>>
>> static int blk_revalidate_conv_zone(struct blk_zone *zone, unsigned int idx,
>
> And then in blk_revalidate_disk_zones() do this:
>
> if (ret > 0) {
> ret = disk_update_zone_resources(disk, &args);
> } else if (ret) {
> unsigned int memflags;
>
> pr_warn("%s: failed to revalidate zones\n", disk->disk_name);
>
> memflags = blk_mq_freeze_queue(q);
> disk_free_zone_resources(disk);
> blk_mq_unfreeze_queue(q, memflags);
> }
>
> The question remains if this looks better?

Rereading everything, I think that Bart has a good point: moving the call to
disk_free_zone_resources() in the error path of disk_update_zone_resources()
allows doing the error handling without unfreezing and re-freezing the queue.
That's better.

--
Damien Le Moal
Western Digital Research