On 25.06.2020 15:16, Matias Bjørling wrote:
On 25/06/2020 14.21, Javier González wrote:From: Javier González <javier.gonz@xxxxxxxxxxx> 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. Signed-off-by: Javier González <javier.gonz@xxxxxxxxxxx> Signed-off-by: SelvaKumar S <selvakuma.s1@xxxxxxxxxxx> Signed-off-by: Kanchan Joshi <joshi.k@xxxxxxxxxxx> Signed-off-by: Nitesh Shetty <nj.shetty@xxxxxxxxxxx> --- drivers/nvme/host/zns.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c index 7d8381fe7665..de806788a184 100644 --- a/drivers/nvme/host/zns.c +++ b/drivers/nvme/host/zns.c @@ -234,6 +234,13 @@ static int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector, sector += ns->zsze * nz; } + if (nr_zones < 0 && zone_idx != ns->nr_zones) { + dev_err(ns->ctrl->device, "inconsistent zone count %u/%u\n", + zone_idx, ns->nr_zones); + ret = -EINVAL; + goto out_free; + } + ret = zone_idx; out_free: kvfree(report);Sounds like a check for a broken implementation. For implementations in the wild that exhibits this behavior, a quirk can be added. This kind of check is generally not needed. This can easily be checked by having a test case in a validation suite. The kernel should not have to check for it.
I don't believe it hurts to validate as ZNS provides a method to retrieve the actual number of zones. It can help people detecting an issue that can hide for some time. If the general opinion is that this belongs to a test suite, we can add it to blktests (already have it there internally). We can also have it in both places. Javier
