On 8/9/22 9:51 AM, Keith Busch wrote: > On Tue, Aug 09, 2022 at 10:56:55AM +0000, Chaitanya Kulkarni wrote: >> On 8/8/22 17:04, Mike Christie wrote: >>> + >>> + c.common.opcode = nvme_cmd_resv_report; >>> + c.common.cdw10 = cpu_to_le32(nvme_bytes_to_numd(data_len)); >>> + c.common.cdw11 = 1; >>> + *eds = true; >>> + >>> +retry: >>> + if (IS_ENABLED(CONFIG_NVME_MULTIPATH) && >>> + bdev->bd_disk->fops == &nvme_ns_head_ops) >>> + ret = nvme_send_ns_head_pr_command(bdev, &c, data, data_len); >>> + else >>> + ret = nvme_send_ns_pr_command(bdev->bd_disk->private_data, &c, >>> + data, data_len); >>> + if (ret == NVME_SC_HOST_ID_INCONSIST && c.common.cdw11) { >>> + c.common.cdw11 = 0; >>> + *eds = false; >>> + goto retry; >> >> Unconditional retries without any limit can create problems, >> perhaps consider adding some soft limits. > > It's already conditioned on cdw11, which is cleared to 0 on the 2nd try. Not > that that's particularly clear. I'd suggest naming an enum value for it so the > code tells us what the signficance of cdw11 is in this context (it's the > Extended Data Structure control flag). Will do that. Chaitanya for your comment, with a bad device we could hit an issue where we we cleared the Extended Data Structure control flag and it also returned NVME_SC_HOST_ID_INCONSIST and we'd be in an infinite loop, so I'll handle that. -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel