Re: [PATCH v2 1/4] nvme: factor namespace-head queue-limit update

John Garry <[email protected]>
Newsgroups org.infradead.lists.linux-nvme,org.kernel.vger.linux-block
Organization Oracle Corporation
Message-ID <[email protected]>
On 06/08/2026 03:46, Yao Sang wrote:
> Move the namespace head queue-limit update out of nvme_update_ns_info().
> The new helper keeps the current queue_limits_stack_bdev() based behavior
> intact, including zoned resource handling, write-stream assignment,
> integrity setup, capacity and readonly updates, path revalidation, and
> namespace-head zone revalidation.
> 
> Keep queue-limit commit failures on the existing short-circuit path so
> capacity and namespace-head state are only updated after a successful
> limits update.
> 
> The helper gives namespace-head queue-limit updates a single NVMe-local
> entry point while keeping the namespace information refresh sequencing
> unchanged.

Like the cover letter, this message is too verbose. So much so that I 
lose track of what is important to note - that being the motivation for 
the change.

The motivation seems to be to just factor out the NS head update into a 
separate function as it deserves its own function and the code will be 
neater, but not because it will in future have multiple callsites.

> 
> Reviewed-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Yao Sang <[email protected]>
> ---
>   drivers/nvme/host/core.c | 104 +++++++++++++++++++++------------------
>   1 file changed, 56 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index cb93ada4376a..e3d27c0440db 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2528,6 +2528,60 @@ static void nvme_stack_zone_resources(struct queue_limits *t,
>   		min_not_zero(t->max_active_zones, b->max_active_zones);
>   }
>   
> +static int nvme_update_ns_head_limits(struct nvme_ns *ns,
> +		struct nvme_ns_info *info, bool unsupported)
> +{
> +	struct queue_limits *ns_lim = &ns->disk->queue->limits;
> +	struct request_queue *head_q = ns->head->disk->queue;
> +	struct queue_limits lim;
> +	unsigned int memflags;
> +	int ret;
> +
> +	lim = queue_limits_start_update(head_q);
> +	memflags = blk_mq_freeze_queue(head_q);
> +	/*
> +	 * queue_limits mixes values that are the hardware limitations
> +	 * for bio splitting with what is the device configuration.
> +	 *
> +	 * For NVMe the device configuration can change after e.g. a
> +	 * Format command, and we really want to pick up the new format
> +	 * value here. But we must still stack the queue limits to the
> +	 * least common denominator for multipathing to split the bios
> +	 * properly.
> +	 *
> +	 * To work around this, we explicitly set the device
> +	 * configuration to those that we just queried, but only stack
> +	 * the splitting limits in to make sure we still obey possibly
> +	 * lower limitations of other controllers.
> +	 */
> +	lim.logical_block_size = ns_lim->logical_block_size;
> +	lim.physical_block_size = ns_lim->physical_block_size;
> +	lim.io_min = ns_lim->io_min;
> +	lim.io_opt = ns_lim->io_opt;
> +	queue_limits_stack_bdev(&lim, ns->disk->part0, 0,
> +				ns->head->disk->disk_name);
> +	if (lim.features & BLK_FEAT_ZONED)
> +		nvme_stack_zone_resources(&lim, ns_lim);
> +	if (unsupported)
> +		ns->head->disk->flags |= GENHD_FL_HIDDEN;
> +	else
> +		nvme_init_integrity(ns->head, &lim, info);
> +	lim.max_write_streams = ns_lim->max_write_streams;
> +	lim.write_stream_granularity = ns_lim->write_stream_granularity;
> +	ret = queue_limits_commit_update(head_q, &lim);
> +	if (ret)
> +		goto unfreeze_head_queue;
> +
> +	set_capacity_and_notify(ns->head->disk, get_capacity(ns->disk));
> +	set_disk_ro(ns->head->disk, nvme_ns_is_readonly(ns, info));
> +	nvme_mpath_revalidate_paths(ns->head);
> +	ret = nvme_mpath_revalidate_zones(ns->head);
> +
> +unfreeze_head_queue:
> +	blk_mq_unfreeze_queue(head_q, memflags);
> +	return ret;
> +}
> +
>   static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_ns_info *info)
>   {
>   	bool unsupported = false;
> @@ -2566,54 +2620,8 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_ns_info *info)
>   		ret = 0;
>   	}
>   
> -	if (!ret && nvme_ns_head_multipath(ns->head)) {
> -		struct queue_limits *ns_lim = &ns->disk->queue->limits;
> -		struct queue_limits lim;
> -		unsigned int memflags;
> -
> -		lim = queue_limits_start_update(ns->head->disk->queue);
> -		memflags = blk_mq_freeze_queue(ns->head->disk->queue);
> -		/*
> -		 * queue_limits mixes values that are the hardware limitations
> -		 * for bio splitting with what is the device configuration.
> -		 *
> -		 * For NVMe the device configuration can change after e.g. a
> -		 * Format command, and we really want to pick up the new format
> -		 * value here.  But we must still stack the queue limits to the
> -		 * least common denominator for multipathing to split the bios
> -		 * properly.
> -		 *
> -		 * To work around this, we explicitly set the device
> -		 * configuration to those that we just queried, but only stack
> -		 * the splitting limits in to make sure we still obey possibly
> -		 * lower limitations of other controllers.
> -		 */
> -		lim.logical_block_size = ns_lim->logical_block_size;
> -		lim.physical_block_size = ns_lim->physical_block_size;
> -		lim.io_min = ns_lim->io_min;
> -		lim.io_opt = ns_lim->io_opt;
> -		queue_limits_stack_bdev(&lim, ns->disk->part0, 0,
> -					ns->head->disk->disk_name);
> -		if (lim.features & BLK_FEAT_ZONED)
> -			nvme_stack_zone_resources(&lim, ns_lim);
> -		if (unsupported)
> -			ns->head->disk->flags |= GENHD_FL_HIDDEN;
> -		else
> -			nvme_init_integrity(ns->head, &lim, info);
> -		lim.max_write_streams = ns_lim->max_write_streams;
> -		lim.write_stream_granularity = ns_lim->write_stream_granularity;
> -		ret = queue_limits_commit_update(ns->head->disk->queue, &lim);
> -		if (ret)
> -			goto unfreeze_head_queue;
> -
> -		set_capacity_and_notify(ns->head->disk, get_capacity(ns->disk));
> -		set_disk_ro(ns->head->disk, nvme_ns_is_readonly(ns, info));
> -		nvme_mpath_revalidate_paths(ns->head);
> -		ret = nvme_mpath_revalidate_zones(ns->head);
> -
> -unfreeze_head_queue:
> -		blk_mq_unfreeze_queue(ns->head->disk->queue, memflags);
> -	}
> +	if (!ret && nvme_ns_head_multipath(ns->head))
> +		ret = nvme_update_ns_head_limits(ns, info, unsupported);
>   
>   	return ret;
>   }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.