Re: [PATCH v2 3/3] qla2xxx: Fix NVME cmd and LS cmd timeout race condition

[Bart Van Assche]

On 6/18/19 11:10 AM, Himanshu Madhani wrote:
+out:
+       /* kref_get was done before work was schedule. */
+       if (sp->type == SRB_NVME_CMD)
+               kref_put(&sp->cmd_kref, qla_nvme_release_fcp_cmd_kref);
+       else if (sp->type == SRB_NVME_LS)
+               kref_put(&sp->cmd_kref, qla_nvme_release_ls_cmd_kref);

Hi Himanshu and Quinn,

The above code looks ugly to me. I would prefer that the 
qla_nvme_release_fcp_cmd_kref() and qla_nvme_release_ls_cmd_kref() are 
consolidated into a single function such that the above code can be 
changed into a single kref_put() call.

Additionally, I think this patch introduces a behavior change. Today if 
a completion and abort request race, the NVMe command is completed. I 
think this patch changes that behavior into failing the NVMe command 
with status "aborted". That behavior change has not been described in 
the commit message. That makes me wonder whether that behavior change is 
unintentional?

Thanks,

Bart.