| Message ID | 20210128044733.503606-3-damien.lemoal@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: add zone write granularity limit | expand |
> int nvme_revalidate_zones(struct nvme_ns *ns) > { > - struct request_queue *q = ns->queue; > - int ret; > - > - ret = blk_revalidate_disk_zones(ns->disk, NULL); > - if (!ret) > - blk_queue_max_zone_append_sectors(q, ns->ctrl->max_zone_append); > - return ret; > + return blk_revalidate_disk_zones(ns->disk, NULL); We can just kill off nvme_revalidate_zones now and open code it in the caller as the stub is no needed now that blk_queue_is_zoned always return false for the !CONFIG_BLK_DEV_ZONED case. Otherwise this look great, nice cleanup: Reviewed-by: Christoph Hellwig <hch@lst.de>
On 2021/01/28 18:17, Christoph Hellwig wrote: >> int nvme_revalidate_zones(struct nvme_ns *ns) >> { >> - struct request_queue *q = ns->queue; >> - int ret; >> - >> - ret = blk_revalidate_disk_zones(ns->disk, NULL); >> - if (!ret) >> - blk_queue_max_zone_append_sectors(q, ns->ctrl->max_zone_append); >> - return ret; >> + return blk_revalidate_disk_zones(ns->disk, NULL); > > We can just kill off nvme_revalidate_zones now and open code it in > the caller as the stub is no needed now that blk_queue_is_zoned always > return false for the !CONFIG_BLK_DEV_ZONED case. I tried that first, but it did not work. I end up with blk_revalidate_disk_zones() undefined error with !CONFIG_BLK_DEV_ZONED. This is because blk_queue_is_zoned() is *not* stubbed for !CONFIG_BLK_DEV_ZONED. It will simply always return 0/none in that case. We would need to have: if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && blk_queue_is_zoned()) { ret = blk_revalidate_disk_zones(ns->disk, NULL); ... Or stub blk_queue_is_zoned()... > > Otherwise this look great, nice cleanup: > > Reviewed-by: Christoph Hellwig <hch@lst.de> >
On Thu, Jan 28, 2021 at 09:27:41AM +0000, Damien Le Moal wrote: > > I tried that first, but it did not work. I end up with > blk_revalidate_disk_zones() undefined error with !CONFIG_BLK_DEV_ZONED. > This is because blk_queue_is_zoned() is *not* stubbed for !CONFIG_BLK_DEV_ZONED. > It will simply always return 0/none in that case. We would need to have: Hmm. blk_queue_is_zoned is calls blk_queue_zoned_model, which always returns BLK_ZONED_NONE for !CONFIG_BLK_DEV_ZONED, and I thought we rely on that elsewhere in nvme. That is a fairly recent change from me, though. > > if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && blk_queue_is_zoned()) { > ret = blk_revalidate_disk_zones(ns->disk, NULL); > ... > > Or stub blk_queue_is_zoned()... If the above really doesn't work we should properly stub it out. Anyway, I think we can go with your current patch: Reviewed-by: Christoph Hellwig <hch@lst.de> and refine all the zoned stubs later. I have a few more ideas for improvements there anyway.
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 1a3cdc6b1036..81a1c7f6223f 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2176,17 +2176,18 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id) ns->lba_shift = id->lbaf[lbaf].ds; nvme_set_queue_limits(ns->ctrl, ns->queue); + ret = nvme_configure_metadata(ns, id); + if (ret) + goto out_unfreeze; + nvme_set_chunk_sectors(ns, id); + nvme_update_disk_info(ns->disk, ns, id); + if (ns->head->ids.csi == NVME_CSI_ZNS) { ret = nvme_update_zone_info(ns, lbaf); if (ret) goto out_unfreeze; } - ret = nvme_configure_metadata(ns, id); - if (ret) - goto out_unfreeze; - nvme_set_chunk_sectors(ns, id); - nvme_update_disk_info(ns->disk, ns, id); blk_mq_unfreeze_queue(ns->disk->queue); if (blk_queue_is_zoned(ns->queue)) { diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c index 1dfe9a3500e3..c7e3ec561ba0 100644 --- a/drivers/nvme/host/zns.c +++ b/drivers/nvme/host/zns.c @@ -9,13 +9,7 @@ int nvme_revalidate_zones(struct nvme_ns *ns) { - struct request_queue *q = ns->queue; - int ret; - - ret = blk_revalidate_disk_zones(ns->disk, NULL); - if (!ret) - blk_queue_max_zone_append_sectors(q, ns->ctrl->max_zone_append); - return ret; + return blk_revalidate_disk_zones(ns->disk, NULL); } static int nvme_set_max_append(struct nvme_ctrl *ctrl) @@ -109,10 +103,11 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf) goto free_data; } - q->limits.zoned = BLK_ZONED_HM; + blk_queue_set_zoned(ns->disk, BLK_ZONED_HM); blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q); blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1); blk_queue_max_active_zones(q, le32_to_cpu(id->mar) + 1); + blk_queue_max_zone_append_sectors(q, ns->ctrl->max_zone_append); free_data: kfree(id); return status;
For a zoned namespace, in nvme_update_ns_info(), call nvme_update_zone_info() after executing nvme_update_disk_info() so that the namespace queue logical and physical block size limits are set. This allows setting the namespace queue max_zone_append_sectors limit in nvme_update_zone_info() instead of nvme_revalidate_zones(), simplifying this function. Also use blk_queue_set_zoned() to set the namespace zoned model. Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- drivers/nvme/host/core.c | 11 ++++++----- drivers/nvme/host/zns.c | 11 +++-------- 2 files changed, 9 insertions(+), 13 deletions(-)