Re: [PATCH 6/6] nvme: Add consistency check for zone count

["Javier González" <javier@javigon.com>]

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@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.
>>
>>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/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