| Message ID | 20200702065438.46350-4-javier@javigon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ZNS: Extra features for current patche | expand |
On 2020/07/02 15:55, Javier González wrote: > From: Javier González <javier.gonz@samsung.com> > > Since the number of zones is calculated through the reported device > capacity and the ZNS specification allows to report the total number of > zones in the device, add an extra check to guarantee consistency between > the device and the kernel. > > Also, fix a checkpatch warning: unsigned -> unsigned int in the process > > Signed-off-by: Javier González <javier.gonz@samsung.com> > Signed-off-by: SelvaKumar S <selvakuma.s1@samsung.com> > Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> > --- > drivers/nvme/host/core.c | 2 +- > drivers/nvme/host/nvme.h | 6 ++++-- > drivers/nvme/host/zns.c | 39 ++++++++++++++++++++++++++++++++++++++- > 3 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 1f5c7fc3d2c9..8b9c69172931 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -1994,7 +1994,7 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id) > case NVME_CSI_NVM: > break; > case NVME_CSI_ZNS: > - ret = nvme_update_zone_info(disk, ns, lbaf); > + ret = nvme_update_zone_info(disk, ns, lbaf, le64_to_cpu(id->nsze)); > if (ret) > return ret; > break; > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h > index 662f95fbd909..ef80e0b4df56 100644 > --- a/drivers/nvme/host/nvme.h > +++ b/drivers/nvme/host/nvme.h > @@ -407,6 +407,7 @@ struct nvme_ns { > u32 sws; > u8 pi_type; > #ifdef CONFIG_BLK_DEV_ZONED > + u64 nr_zones; > u64 zsze; > #endif > unsigned long features; > @@ -700,7 +701,7 @@ static inline void nvme_mpath_start_freeze(struct nvme_subsystem *subsys) > > #ifdef CONFIG_BLK_DEV_ZONED > int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, > - unsigned lbaf); > + unsigned int lbaf, sector_t nsze); > > int nvme_report_zones(struct gendisk *disk, sector_t sector, > unsigned int nr_zones, report_zones_cb cb, void *data); > @@ -720,7 +721,8 @@ static inline blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, > > static inline int nvme_update_zone_info(struct gendisk *disk, > struct nvme_ns *ns, > - unsigned lbaf) > + unsigned lbaf missing a comma here. Does this even compiles ? > + sector_t nsze) > { > dev_warn(ns->ctrl->device, > "Please enable CONFIG_BLK_DEV_ZONED to support ZNS devices\n"); > diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c > index b34d2ed13825..d785d179a343 100644 > --- a/drivers/nvme/host/zns.c > +++ b/drivers/nvme/host/zns.c > @@ -32,13 +32,36 @@ static int nvme_set_max_append(struct nvme_ctrl *ctrl) > return 0; > } > > +static u64 nvme_zns_nr_zones(struct nvme_ns *ns) > +{ > + struct nvme_command c = { }; > + struct nvme_zone_report report; > + int buflen = sizeof(struct nvme_zone_report); > + int ret; > + > + c.zmr.opcode = nvme_cmd_zone_mgmt_recv; > + c.zmr.nsid = cpu_to_le32(ns->head->ns_id); > + c.zmr.slba = cpu_to_le64(0); > + c.zmr.numd = cpu_to_le32(nvme_bytes_to_numd(buflen)); > + c.zmr.zra = NVME_ZRA_ZONE_REPORT; > + c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL; > + c.zmr.pr = 0; > + > + ret = nvme_submit_sync_cmd(ns->queue, &c, &report, buflen); > + if (ret) > + return ret; > + > + return le64_to_cpu(report.nr_zones); > +} > + > int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, > - unsigned lbaf) > + unsigned int lbaf, sector_t nsze) > { > // struct nvme_effects_log *log = ns->head->effects; > struct request_queue *q = disk->queue; > struct nvme_command c = { }; > struct nvme_id_ns_zns *id; > + sector_t cap_nr_zones; > int status; > > /* Driver requires zone append support */ > @@ -80,6 +103,20 @@ int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, > goto free_data; > } > > + ns->nr_zones = nvme_zns_nr_zones(ns); > + if (!ns->nr_zones) { > + status = -EINVAL; > + goto free_data; > + } > + > + cap_nr_zones = nvme_lba_to_sect(ns, le64_to_cpu(nsze)) >> ilog2(ns->zsze); > + if (ns->nr_zones != cap_nr_zones) { > + dev_err(ns->ctrl->device, "inconsistent zone count: %llu/%llu\n", > + ns->nr_zones, cap_nr_zones); > + status = -EINVAL; > + goto free_data; > + } > + I am not opposed to this, but I am not sure if this adds anything to the checks in blk_revalidate_disk_zones() as that does a full zone report that is checked against capacity. This seems more like a test of the drive conformance, checking that the report header nr_zones is correct... > q->limits.zoned = BLK_ZONED_HM; > q->zone_flags = BLK_ZONE_REP_CAPACITY | BLK_ZONE_REP_OFFLINE; > blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q); >
On 02/07/2020 08:55, Javier González wrote: > int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, > - unsigned lbaf) > + unsigned int lbaf, sector_t nsze) > { > // struct nvme_effects_log *log = ns->head->effects; > struct request_queue *q = disk->queue; Please no C++ style comments and no commenting out of code in an official submission.
On 02.07.2020 08:19, Johannes Thumshirn wrote: >On 02/07/2020 08:55, Javier González wrote: >> int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, >> - unsigned lbaf) >> + unsigned int lbaf, sector_t nsze) >> { >> // struct nvme_effects_log *log = ns->head->effects; >> struct request_queue *q = disk->queue; > > >Please no C++ style comments and no commenting out of code in an official submission. Sorry. I see some debug code leaked here. Javier
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 1f5c7fc3d2c9..8b9c69172931 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1994,7 +1994,7 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id) case NVME_CSI_NVM: break; case NVME_CSI_ZNS: - ret = nvme_update_zone_info(disk, ns, lbaf); + ret = nvme_update_zone_info(disk, ns, lbaf, le64_to_cpu(id->nsze)); if (ret) return ret; break; diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 662f95fbd909..ef80e0b4df56 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -407,6 +407,7 @@ struct nvme_ns { u32 sws; u8 pi_type; #ifdef CONFIG_BLK_DEV_ZONED + u64 nr_zones; u64 zsze; #endif unsigned long features; @@ -700,7 +701,7 @@ static inline void nvme_mpath_start_freeze(struct nvme_subsystem *subsys) #ifdef CONFIG_BLK_DEV_ZONED int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, - unsigned lbaf); + unsigned int lbaf, sector_t nsze); int nvme_report_zones(struct gendisk *disk, sector_t sector, unsigned int nr_zones, report_zones_cb cb, void *data); @@ -720,7 +721,8 @@ static inline blk_status_t nvme_setup_zone_mgmt_send(struct nvme_ns *ns, static inline int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, - unsigned lbaf) + unsigned lbaf + sector_t nsze) { dev_warn(ns->ctrl->device, "Please enable CONFIG_BLK_DEV_ZONED to support ZNS devices\n"); diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c index b34d2ed13825..d785d179a343 100644 --- a/drivers/nvme/host/zns.c +++ b/drivers/nvme/host/zns.c @@ -32,13 +32,36 @@ static int nvme_set_max_append(struct nvme_ctrl *ctrl) return 0; } +static u64 nvme_zns_nr_zones(struct nvme_ns *ns) +{ + struct nvme_command c = { }; + struct nvme_zone_report report; + int buflen = sizeof(struct nvme_zone_report); + int ret; + + c.zmr.opcode = nvme_cmd_zone_mgmt_recv; + c.zmr.nsid = cpu_to_le32(ns->head->ns_id); + c.zmr.slba = cpu_to_le64(0); + c.zmr.numd = cpu_to_le32(nvme_bytes_to_numd(buflen)); + c.zmr.zra = NVME_ZRA_ZONE_REPORT; + c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL; + c.zmr.pr = 0; + + ret = nvme_submit_sync_cmd(ns->queue, &c, &report, buflen); + if (ret) + return ret; + + return le64_to_cpu(report.nr_zones); +} + int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, - unsigned lbaf) + unsigned int lbaf, sector_t nsze) { // struct nvme_effects_log *log = ns->head->effects; struct request_queue *q = disk->queue; struct nvme_command c = { }; struct nvme_id_ns_zns *id; + sector_t cap_nr_zones; int status; /* Driver requires zone append support */ @@ -80,6 +103,20 @@ int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, goto free_data; } + ns->nr_zones = nvme_zns_nr_zones(ns); + if (!ns->nr_zones) { + status = -EINVAL; + goto free_data; + } + + cap_nr_zones = nvme_lba_to_sect(ns, le64_to_cpu(nsze)) >> ilog2(ns->zsze); + if (ns->nr_zones != cap_nr_zones) { + dev_err(ns->ctrl->device, "inconsistent zone count: %llu/%llu\n", + ns->nr_zones, cap_nr_zones); + status = -EINVAL; + goto free_data; + } + q->limits.zoned = BLK_ZONED_HM; q->zone_flags = BLK_ZONE_REP_CAPACITY | BLK_ZONE_REP_OFFLINE; blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);