| Message ID | 20200604100745.89250-1-dwagner@suse.de (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | ef2e3ec520a8c20661ca4e7d17a5c7110d3a7828 |
| Headers | show |
| Series | qla2xxx: Set NVME status code for failed NVME FCP request | expand |
On 6/4/20 12:07 PM, Daniel Wagner wrote: > The qla2xxx driver knows when request was processed successfully or > not. But it always sets the NVME status code to 0/NVME_SC_SUCCESS. The > upper layer needs to figure out from the rcv_rsplen and > transferred_length variables if the request was successfully. This is > not always possible, e.g. when the request data length is 0, the > transferred_length is also set 0 which is interpreted as success in > nvme_fc_fcpio_done(). Let's inform the upper > layer (nvme_fc_fcpio_done()) when something went wrong. > > nvme_fc_fcpio_done() maps all non NVME_SC_SUCCESS status codes to > NVME_SC_HOST_PATH_ERROR. There isn't any benefit to map the QLA status > code to the NVME status code. Therefore, let's use NVME_SC_INTERNAL to > indicate an error which aligns it with the lpfc driver. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > drivers/scsi/qla2xxx/qla_nvme.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c > index d66d47a0f958..fa695a4007f8 100644 > --- a/drivers/scsi/qla2xxx/qla_nvme.c > +++ b/drivers/scsi/qla2xxx/qla_nvme.c > @@ -139,11 +139,12 @@ static void qla_nvme_release_fcp_cmd_kref(struct kref *kref) > sp->priv = NULL; > if (priv->comp_status == QLA_SUCCESS) { > fd->rcv_rsplen = le16_to_cpu(nvme->u.nvme.rsp_pyld_len); > + fd->status = NVME_SC_SUCCESS; > } else { > fd->rcv_rsplen = 0; > fd->transferred_length = 0; > + fd->status = NVME_SC_INTERNAL; > } > - fd->status = 0; > spin_unlock_irqrestore(&priv->cmd_lock, flags); > > fd->done(fd); > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
On 6/4/20 5:07 AM, Daniel Wagner wrote: > The qla2xxx driver knows when request was processed successfully or > not. But it always sets the NVME status code to 0/NVME_SC_SUCCESS. The > upper layer needs to figure out from the rcv_rsplen and > transferred_length variables if the request was successfully. This is > not always possible, e.g. when the request data length is 0, the > transferred_length is also set 0 which is interpreted as success in > nvme_fc_fcpio_done(). Let's inform the upper > layer (nvme_fc_fcpio_done()) when something went wrong. > > nvme_fc_fcpio_done() maps all non NVME_SC_SUCCESS status codes to > NVME_SC_HOST_PATH_ERROR. There isn't any benefit to map the QLA status > code to the NVME status code. Therefore, let's use NVME_SC_INTERNAL to > indicate an error which aligns it with the lpfc driver. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > drivers/scsi/qla2xxx/qla_nvme.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c > index d66d47a0f958..fa695a4007f8 100644 > --- a/drivers/scsi/qla2xxx/qla_nvme.c > +++ b/drivers/scsi/qla2xxx/qla_nvme.c > @@ -139,11 +139,12 @@ static void qla_nvme_release_fcp_cmd_kref(struct kref *kref) > sp->priv = NULL; > if (priv->comp_status == QLA_SUCCESS) { > fd->rcv_rsplen = le16_to_cpu(nvme->u.nvme.rsp_pyld_len); > + fd->status = NVME_SC_SUCCESS; > } else { > fd->rcv_rsplen = 0; > fd->transferred_length = 0; > + fd->status = NVME_SC_INTERNAL; > } > - fd->status = 0; > spin_unlock_irqrestore(&priv->cmd_lock, flags); > > fd->done(fd); > Makes sense. Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
On Thu, 4 Jun 2020 12:07:45 +0200, Daniel Wagner wrote: > The qla2xxx driver knows when request was processed successfully or > not. But it always sets the NVME status code to 0/NVME_SC_SUCCESS. The > upper layer needs to figure out from the rcv_rsplen and > transferred_length variables if the request was successfully. This is > not always possible, e.g. when the request data length is 0, the > transferred_length is also set 0 which is interpreted as success in > nvme_fc_fcpio_done(). Let's inform the upper > layer (nvme_fc_fcpio_done()) when something went wrong. > > [...] Applied to 5.8/scsi-fixes, thanks! [1/1] scsi: qla2xxx: Set NVMe status code for failed NVMe FCP request https://git.kernel.org/mkp/scsi/c/ef2e3ec520a8
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c index d66d47a0f958..fa695a4007f8 100644 --- a/drivers/scsi/qla2xxx/qla_nvme.c +++ b/drivers/scsi/qla2xxx/qla_nvme.c @@ -139,11 +139,12 @@ static void qla_nvme_release_fcp_cmd_kref(struct kref *kref) sp->priv = NULL; if (priv->comp_status == QLA_SUCCESS) { fd->rcv_rsplen = le16_to_cpu(nvme->u.nvme.rsp_pyld_len); + fd->status = NVME_SC_SUCCESS; } else { fd->rcv_rsplen = 0; fd->transferred_length = 0; + fd->status = NVME_SC_INTERNAL; } - fd->status = 0; spin_unlock_irqrestore(&priv->cmd_lock, flags); fd->done(fd);
The qla2xxx driver knows when request was processed successfully or not. But it always sets the NVME status code to 0/NVME_SC_SUCCESS. The upper layer needs to figure out from the rcv_rsplen and transferred_length variables if the request was successfully. This is not always possible, e.g. when the request data length is 0, the transferred_length is also set 0 which is interpreted as success in nvme_fc_fcpio_done(). Let's inform the upper layer (nvme_fc_fcpio_done()) when something went wrong. nvme_fc_fcpio_done() maps all non NVME_SC_SUCCESS status codes to NVME_SC_HOST_PATH_ERROR. There isn't any benefit to map the QLA status code to the NVME status code. Therefore, let's use NVME_SC_INTERNAL to indicate an error which aligns it with the lpfc driver. Signed-off-by: Daniel Wagner <dwagner@suse.de> --- drivers/scsi/qla2xxx/qla_nvme.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)