Re: [PATCH v2 09/20] nvme: Add helper to execute Reservation Report

Keith Busch
2022-08-09T14:51:19Z

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