On 2021/01/12 15:11, Chaitanya Kulkarni wrote:
[...]

quoted

quoted

+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.

quoted

quoted

+
+	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