[PATCH V7 4/6] nvmet: add ZBD over ZNS backend support

[Chaitanya Kulkarni]

On 12/15/20 15:13, Damien Le Moal wrote:
> On 2020/12/16 8:06, Chaitanya Kulkarni wrote:
> [...]
>>>> +void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
>>>> +{
>>>> +	sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
>>>> +	u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
>>>> +	struct nvmet_report_zone_data data = { .ns = req->ns };
>>>> +	unsigned int nr_zones;
>>>> +	int reported_zones;
>>>> +	u16 status;
>>>> +
>>>> +	nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
>>>> +			sizeof(struct nvme_zone_descriptor);
>>> You could move this right before the vmalloc call since it is first used there.
>> There are only three lines between the this and the vmalloc, does that
>> really
>> going to make any difference ?
> It makes the code far easier to read and understand at a quick glance without
> having to go up and down reading to be reminded what nr_zones was. That also
> would avoid changes to sneak in between these related statements, making things
> even harder to read.
>
> I personally like to think of code as a natural language text: if statements
> related to each other are not grouped in a single paragraph, the text is really
> hard to understand...
>
hmmm, why can't we use a macro and like everywhere else in zns.c
we can declare-init the nr_zones which will make nr_zones initialization
uniform withall the code with a meaningful name.

How about following (untested) ?

diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 149bc8ce7010..6c497b60cd98 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -201,18 +201,19 @@ static int nvmet_bdev_report_zone_cb(struct
blk_zone *z, unsigned int idx,
        return 0;
 }
 
+#define NVMET_NR_ZONES_FROM_BUFSIZE(bufsize) \
+       ((bufsize - sizeof(struct nvme_zone_report)) / \
+                       sizeof(struct nvme_zone_descriptor))
+
 void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req)
 {
        sector_t sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
        u32 bufsize = (le32_to_cpu(req->cmd->zmr.numd) + 1) << 2;
        struct nvmet_report_zone_data data = { .ns = req->ns };
-       unsigned int nr_zones;
+       unsigned int nr_zones = NVMET_NR_ZONES_FROM_BUFSIZE(bufsize);
        int reported_zones;
        u16 status;
 
-       nr_zones = (bufsize - sizeof(struct nvme_zone_report)) /
-                       sizeof(struct nvme_zone_descriptor);
-
        status = nvmet_bdev_zns_checks(req);
        if (status)
                goto out;

> -- Damien Le Moal Western Digital Research