Re: [PATCH 02/11] block: add a queue_limits_commit_update_frozen helper

[John Garry <john.g.garry@oracle.com>]

On 09/01/2025 05:57, Christoph Hellwig wrote:
> Add a helper that freezes the queue, updates the queue limits and
> unfreezes the queue and convert all open coded versions of that to the
> new helper.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Apart from some nitpicking on the code comments, FWIW:
Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   block/blk-integrity.c      |  4 +---
>   block/blk-settings.c       | 24 ++++++++++++++++++++++++
>   block/blk-zoned.c          |  7 +------
>   drivers/block/virtio_blk.c |  4 +---
>   drivers/scsi/sd.c          | 17 +++++------------
>   drivers/scsi/sr.c          |  5 +----
>   include/linux/blkdev.h     |  2 ++
>   7 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
> index b180cac61a9d..013469faa5e7 100644
> --- a/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -218,9 +218,7 @@ static ssize_t flag_store(struct device *dev, const char *page, size_t count,
>   	else
>   		lim.integrity.flags |= flag;
>   
> -	blk_mq_freeze_queue(q);
> -	err = queue_limits_commit_update(q, &lim);
> -	blk_mq_unfreeze_queue(q);
> +	err = queue_limits_commit_update_frozen(q, &lim);
>   	if (err)
>   		return err;
>   	return count;
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 89d8366fd43c..7c099d686dd8 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -444,6 +444,30 @@ int queue_limits_commit_update(struct request_queue *q,
>   }
>   EXPORT_SYMBOL_GPL(queue_limits_commit_update);
>   
> +/**
> + * queue_limits_commit_update_frozen - commit an atomic update of queue limits

nit: this has same brief description of queue_limits_commit_update(), 
but it does not do the same thing. I know that space is limited here for 
the comment, though.

> + * @q:		queue to update
> + * @lim:	limits to apply
> + *
> + * Apply the limits in @lim that were obtained from queue_limits_start_update()
> + * and updated by the caller to @q. 

"updated by the caller to @q" - this sounds odd

> Freezes the queue before the updated and
> + * unfreezes it after.

should that be "before the update"?

> + *
> + * Returns 0 if successful, else a negative error code.
> + */
> +int queue_limits_commit_update_frozen(struct request_queue *q,
> +		struct queue_limits *lim)
> +{
> +	int ret;
> +
> +	blk_mq_freeze_queue(q);
> +	ret = queue_limits_commit_update(q, lim);
> +	blk_mq_unfreeze_queue(q);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(queue_limits_commit_update_frozen);
> +