On 2021/01/12 15:11, 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);
>> I really would prefer this code to be moved down, before the call to
>> blkdev_report_zones().
>>
>> You can also optimize this value a little with a min() of the value above and of
>> DIV_ROUND_UP(dev_capacity - sect, zone size). But not a big deal I think.
> I did that as per your last comment, when I did the code review with
> host side it didn't match, I've a cleanup patch series to fix nits and
> host side css checks for zns I've added this into that series.
>>> +
>>> + status = nvmet_bdev_zns_checks(req);
>>> + if (status)
>>> + goto out;
>>> +
>>> + data.rz = __vmalloc(bufsize, GFP_KERNEL | __GFP_NORETRY | __GFP_ZERO);
>> Shouldn't this be GFP_NOIO ? Also, is the NORETRY critical ?
> Yes on GFP_NOIO. NORETRY critical means how we areallocating the memory on
> the host side nvme_zns_alloc_report_buffer() ?
By critical, I mean that if __GFP_NORETRY is removed, things break ? Or is it
just an optimization to avoid overtaxing the host resources ? I suspect the
latter case, but wanted to make sure.
--
Damien Le Moal
Western Digital Research
