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
