Re: [PATCH v2 1/4] nvme: factor namespace-head queue-limit update
John Garry <[email protected]>
| Newsgroups | org.kernel.vger.linux-block,org.infradead.lists.linux-nvme |
|---|---|
| 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; > }