Re: [PATCH 2/9] nvmet: add ZNS support for bdev-ns

[Damien Le Moal <Damien.LeMoal@wdc.com>]

On 2020/11/28 9:11, Chaitanya Kulkarni wrote:
> On 11/26/20 01:06, Damien Le Moal wrote:
>>> +
>>> +	desc_size = nvmet_zones_to_descsize(blk_queue_nr_zones(q));
>>> +	status = nvmet_bdev_zns_checks(req);
>>> +	if (status)
>>> +		goto out;
>>> +
>>> +	zones = kvcalloc(blkdev_nr_zones(nvmet_bdev(req)->bd_disk),
>>> +			      sizeof(struct blk_zone), GFP_KERNEL);
>>> +	if (!zones) {
>>> +		status = NVME_SC_INTERNAL;
>>> +		goto out;
>>> +	}
>>> +
>>> +	rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY);
>>> +	if (!rz) {
>>> +		status = NVME_SC_INTERNAL;
>>> +		goto out_free_zones;
>>> +	}
>>> +
>>> +	sect = nvmet_lba_to_sect(req->ns, le64_to_cpu(req->cmd->zmr.slba));
>>> +
>>> +	for (nz = blk_queue_nr_zones(q); desc_size >= bufsize; nz--)
>>> +		desc_size = nvmet_zones_to_descsize(nz);
>> desc_size is actually not used anywhere to do something. So what is the purpose
>> of this ? If only to determine nz, the number of zones that can be reported,
>> surely you can calculate it instead of using this loop.
>>
> It reads nicely. Let me see if I can get rid of the loop without having to
> add complex calculations.

I do not think it reads nicely at all: it makes what is being "calculated" hard
to understand. And that definitely looks to me like a waste of CPU cycles
compared to a real calculation.


-- 
Damien Le Moal
Western Digital Research