* [PATCH 01/20] csiostor: use fc_block_rport()
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 02/20] fc_fcp: " Hannes Reinecke
` (18 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Bart Van Assche, Johannes Thumshirn
Use fc_block_rport() instead of fc_block_scsi_eh() as the SCSI command
will be removed as argument for SCSI EH functions.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/csiostor/csio_scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/csiostor/csio_scsi.c b/drivers/scsi/csiostor/csio_scsi.c
index 9aafe0002ab1..c1c410a1cfe0 100644
--- a/drivers/scsi/csiostor/csio_scsi.c
+++ b/drivers/scsi/csiostor/csio_scsi.c
@@ -2088,7 +2088,7 @@ csio_eh_lun_reset_handler(struct scsi_cmnd *cmnd)
}
/* Lnode is ready, now wait on rport node readiness */
- ret = fc_block_scsi_eh(cmnd);
+ ret = fc_block_rport(rn->rport);
if (ret)
return ret;
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 02/20] fc_fcp: use fc_block_rport()
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
2022-05-12 11:12 ` [PATCH 01/20] csiostor: use fc_block_rport() Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 03/20] mptfc: simplify mpt_fc_block_error_handler() Hannes Reinecke
` (17 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Bart Van Assche, Johannes Thumshirn
Use fc_block_rport() instead of fc_block_scsi_eh() as the SCSI command
will be removed as argument for SCSI EH functions.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/libfc/fc_fcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index bce90eb56c9c..074996319df5 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -2157,7 +2157,7 @@ int fc_eh_device_reset(struct scsi_cmnd *sc_cmd)
int rc = FAILED;
int rval;
- rval = fc_block_scsi_eh(sc_cmd);
+ rval = fc_block_rport(rport);
if (rval)
return rval;
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 03/20] mptfc: simplify mpt_fc_block_error_handler()
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
2022-05-12 11:12 ` [PATCH 01/20] csiostor: use fc_block_rport() Hannes Reinecke
2022-05-12 11:12 ` [PATCH 02/20] fc_fcp: " Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 04/20] mptfusion: correct definitions for mptscsih_dev_reset() Hannes Reinecke
` (16 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Johannes Thumshirn
Instead of passing the a function we can as well return a status
and call the function directly afterwards.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/message/fusion/mptfc.c | 83 ++++++++++++++++++++++------------
1 file changed, 54 insertions(+), 29 deletions(-)
diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
index fac747109209..ea8fc32dacfa 100644
--- a/drivers/message/fusion/mptfc.c
+++ b/drivers/message/fusion/mptfc.c
@@ -183,73 +183,98 @@ static struct fc_function_template mptfc_transport_functions = {
};
static int
-mptfc_block_error_handler(struct scsi_cmnd *SCpnt,
- int (*func)(struct scsi_cmnd *SCpnt),
- const char *caller)
+mptfc_block_error_handler(struct fc_rport *rport)
{
MPT_SCSI_HOST *hd;
- struct scsi_device *sdev = SCpnt->device;
- struct Scsi_Host *shost = sdev->host;
- struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
+ struct Scsi_Host *shost = rport_to_shost(rport);
unsigned long flags;
int ready;
- MPT_ADAPTER *ioc;
+ MPT_ADAPTER *ioc;
int loops = 40; /* seconds */
- hd = shost_priv(SCpnt->device->host);
+ hd = shost_priv(shost);
ioc = hd->ioc;
spin_lock_irqsave(shost->host_lock, flags);
while ((ready = fc_remote_port_chkready(rport) >> 16) == DID_IMM_RETRY
|| (loops > 0 && ioc->active == 0)) {
spin_unlock_irqrestore(shost->host_lock, flags);
dfcprintk (ioc, printk(MYIOC_s_DEBUG_FMT
- "mptfc_block_error_handler.%d: %d:%llu, port status is "
- "%x, active flag %d, deferring %s recovery.\n",
+ "mptfc_block_error_handler.%d: %s, port status is "
+ "%x, active flag %d, deferring recovery.\n",
ioc->name, ioc->sh->host_no,
- SCpnt->device->id, SCpnt->device->lun,
- ready, ioc->active, caller));
+ dev_name(&rport->dev), ready, ioc->active));
msleep(1000);
spin_lock_irqsave(shost->host_lock, flags);
loops --;
}
spin_unlock_irqrestore(shost->host_lock, flags);
- if (ready == DID_NO_CONNECT || !SCpnt->device->hostdata
- || ioc->active == 0) {
+ if (ready == DID_NO_CONNECT || ioc->active == 0) {
dfcprintk (ioc, printk(MYIOC_s_DEBUG_FMT
- "%s.%d: %d:%llu, failing recovery, "
- "port state %x, active %d, vdevice %p.\n", caller,
+ "mpt_block_error_handler.%d: %s, failing recovery, "
+ "port state %x, active %d.\n",
ioc->name, ioc->sh->host_no,
- SCpnt->device->id, SCpnt->device->lun, ready,
- ioc->active, SCpnt->device->hostdata));
+ dev_name(&rport->dev), ready, ioc->active));
return FAILED;
}
- dfcprintk (ioc, printk(MYIOC_s_DEBUG_FMT
- "%s.%d: %d:%llu, executing recovery.\n", caller,
- ioc->name, ioc->sh->host_no,
- SCpnt->device->id, SCpnt->device->lun));
- return (*func)(SCpnt);
+ return SUCCESS;
}
static int
mptfc_abort(struct scsi_cmnd *SCpnt)
{
- return
- mptfc_block_error_handler(SCpnt, mptscsih_abort, __func__);
+ struct Scsi_Host *shost = SCpnt->device->host;
+ struct fc_rport *rport = starget_to_rport(scsi_target(SCpnt->device));
+ MPT_SCSI_HOST __maybe_unused *hd = shost_priv(shost);
+ int rtn;
+
+ rtn = mptfc_block_error_handler(rport);
+ if (rtn == SUCCESS) {
+ dfcprintk (hd->ioc, printk(MYIOC_s_DEBUG_FMT
+ "%s.%d: %d:%llu, executing recovery.\n", __func__,
+ hd->ioc->name, shost->host_no,
+ SCpnt->device->id, SCpnt->device->lun));
+ rtn = mptscsih_abort(SCpnt);
+ }
+ return rtn;
}
static int
mptfc_dev_reset(struct scsi_cmnd *SCpnt)
{
- return
- mptfc_block_error_handler(SCpnt, mptscsih_dev_reset, __func__);
+ struct Scsi_Host *shost = SCpnt->device->host;
+ struct fc_rport *rport = starget_to_rport(scsi_target(SCpnt->device));
+ MPT_SCSI_HOST __maybe_unused *hd = shost_priv(shost);
+ int rtn;
+
+ rtn = mptfc_block_error_handler(rport);
+ if (rtn == SUCCESS) {
+ dfcprintk (hd->ioc, printk(MYIOC_s_DEBUG_FMT
+ "%s.%d: %d:%llu, executing recovery.\n", __func__,
+ hd->ioc->name, shost->host_no,
+ SCpnt->device->id, SCpnt->device->lun));
+ rtn = mptscsih_dev_reset(SCpnt);
+ }
+ return rtn;
}
static int
mptfc_bus_reset(struct scsi_cmnd *SCpnt)
{
- return
- mptfc_block_error_handler(SCpnt, mptscsih_bus_reset, __func__);
+ struct Scsi_Host *shost = SCpnt->device->host;
+ struct fc_rport *rport = starget_to_rport(scsi_target(SCpnt->device));
+ MPT_SCSI_HOST __maybe_unused *hd = shost_priv(shost);
+ int rtn;
+
+ rtn = mptfc_block_error_handler(rport);
+ if (rtn == SUCCESS) {
+ dfcprintk (hd->ioc, printk(MYIOC_s_DEBUG_FMT
+ "%s.%d: %d:%llu, executing recovery.\n", __func__,
+ hd->ioc->name, shost->host_no,
+ SCpnt->device->id, SCpnt->device->lun));
+ rtn = mptscsih_bus_reset(SCpnt);
+ }
+ return rtn;
}
static void
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 04/20] mptfusion: correct definitions for mptscsih_dev_reset()
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (2 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 03/20] mptfc: simplify mpt_fc_block_error_handler() Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-19 17:27 ` Ewan Milne
2022-05-12 11:12 ` [PATCH 05/20] mptfc: open-code mptfc_block_error_handler() for bus reset Hannes Reinecke
` (15 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
From: Hannes Reinecke <hare@suse.com>
mptscsih_dev_reset() is _not_ a device reset, but rather a
target reset. Nevertheless it's being used for either purpose.
This patch adds a correct implementation for mptscsih_dev_reset(),
and renames the original function to mptscsih_target_reset().
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/message/fusion/mptscsih.c | 55 ++++++++++++++++++++++++++++++-
drivers/message/fusion/mptscsih.h | 1 +
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
index 276084ed04a6..ed21cc4d2c77 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1794,7 +1794,7 @@ mptscsih_abort(struct scsi_cmnd * SCpnt)
/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
/**
- * mptscsih_dev_reset - Perform a SCSI TARGET_RESET! new_eh variant
+ * mptscsih_dev_reset - Perform a SCSI LOGICAL_UNIT_RESET!
* @SCpnt: Pointer to scsi_cmnd structure, IO which reset is due to
*
* (linux scsi_host_template.eh_dev_reset_handler routine)
@@ -1809,6 +1809,58 @@ mptscsih_dev_reset(struct scsi_cmnd * SCpnt)
VirtDevice *vdevice;
MPT_ADAPTER *ioc;
+ /* If we can't locate our host adapter structure, return FAILED status.
+ */
+ if ((hd = shost_priv(SCpnt->device->host)) == NULL){
+ printk(KERN_ERR MYNAM ": lun reset: "
+ "Can't locate host! (sc=%p)\n", SCpnt);
+ return FAILED;
+ }
+
+ ioc = hd->ioc;
+ printk(MYIOC_s_INFO_FMT "attempting lun reset! (sc=%p)\n",
+ ioc->name, SCpnt);
+ scsi_print_command(SCpnt);
+
+ vdevice = SCpnt->device->hostdata;
+ if (!vdevice || !vdevice->vtarget) {
+ retval = 0;
+ goto out;
+ }
+
+ retval = mptscsih_IssueTaskMgmt(hd,
+ MPI_SCSITASKMGMT_TASKTYPE_LOGICAL_UNIT_RESET,
+ vdevice->vtarget->channel,
+ vdevice->vtarget->id, vdevice->lun, 0,
+ mptscsih_get_tm_timeout(ioc));
+
+ out:
+ printk (MYIOC_s_INFO_FMT "lun reset: %s (sc=%p)\n",
+ ioc->name, ((retval == 0) ? "SUCCESS" : "FAILED" ), SCpnt);
+
+ if (retval == 0)
+ return SUCCESS;
+ else
+ return FAILED;
+}
+
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/**
+ * mptscsih_target_reset - Perform a SCSI TARGET_RESET!
+ * @SCpnt: Pointer to scsi_cmnd structure, IO which reset is due to
+ *
+ * (linux scsi_host_template.eh_target_reset_handler routine)
+ *
+ * Returns SUCCESS or FAILED.
+ **/
+int
+mptscsih_target_reset(struct scsi_cmnd * SCpnt)
+{
+ MPT_SCSI_HOST *hd;
+ int retval;
+ VirtDevice *vdevice;
+ MPT_ADAPTER *ioc;
+
/* If we can't locate our host adapter structure, return FAILED status.
*/
if ((hd = shost_priv(SCpnt->device->host)) == NULL){
@@ -3257,6 +3309,7 @@ EXPORT_SYMBOL(mptscsih_slave_destroy);
EXPORT_SYMBOL(mptscsih_slave_configure);
EXPORT_SYMBOL(mptscsih_abort);
EXPORT_SYMBOL(mptscsih_dev_reset);
+EXPORT_SYMBOL(mptscsih_target_reset);
EXPORT_SYMBOL(mptscsih_bus_reset);
EXPORT_SYMBOL(mptscsih_host_reset);
EXPORT_SYMBOL(mptscsih_bios_param);
diff --git a/drivers/message/fusion/mptscsih.h b/drivers/message/fusion/mptscsih.h
index a22c5eaf703c..e3d92c392673 100644
--- a/drivers/message/fusion/mptscsih.h
+++ b/drivers/message/fusion/mptscsih.h
@@ -120,6 +120,7 @@ extern void mptscsih_slave_destroy(struct scsi_device *device);
extern int mptscsih_slave_configure(struct scsi_device *device);
extern int mptscsih_abort(struct scsi_cmnd * SCpnt);
extern int mptscsih_dev_reset(struct scsi_cmnd * SCpnt);
+extern int mptscsih_target_reset(struct scsi_cmnd * SCpnt);
extern int mptscsih_bus_reset(struct scsi_cmnd * SCpnt);
extern int mptscsih_host_reset(struct scsi_cmnd *SCpnt);
extern int mptscsih_bios_param(struct scsi_device * sdev, struct block_device *bdev, sector_t capacity, int geom[]);
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 04/20] mptfusion: correct definitions for mptscsih_dev_reset()
2022-05-12 11:12 ` [PATCH 04/20] mptfusion: correct definitions for mptscsih_dev_reset() Hannes Reinecke
@ 2022-05-19 17:27 ` Ewan Milne
2022-05-23 12:25 ` Hannes Reinecke
0 siblings, 1 reply; 34+ messages in thread
From: Ewan Milne @ 2022-05-19 17:27 UTC (permalink / raw)
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Johannes Thumshirn
This patch appears to change the EH activity from issuing a target
reset to a LUN reset only
(since it does not add an entry in the host templates for
.eh_target_reset_handler to invoke
the new mptscsih_target_reset() function, nor does any other patch in
the series).
Is this going to work properly, and without escalating to a host reset
which might not have happened before?
-Ewan
On Thu, May 12, 2022 at 7:13 AM Hannes Reinecke <hare@suse.de> wrote:
>
> From: Hannes Reinecke <hare@suse.com>
>
> mptscsih_dev_reset() is _not_ a device reset, but rather a
> target reset. Nevertheless it's being used for either purpose.
> This patch adds a correct implementation for mptscsih_dev_reset(),
> and renames the original function to mptscsih_target_reset().
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
> drivers/message/fusion/mptscsih.c | 55 ++++++++++++++++++++++++++++++-
> drivers/message/fusion/mptscsih.h | 1 +
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
> index 276084ed04a6..ed21cc4d2c77 100644
> --- a/drivers/message/fusion/mptscsih.c
> +++ b/drivers/message/fusion/mptscsih.c
> @@ -1794,7 +1794,7 @@ mptscsih_abort(struct scsi_cmnd * SCpnt)
>
> /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> /**
> - * mptscsih_dev_reset - Perform a SCSI TARGET_RESET! new_eh variant
> + * mptscsih_dev_reset - Perform a SCSI LOGICAL_UNIT_RESET!
> * @SCpnt: Pointer to scsi_cmnd structure, IO which reset is due to
> *
> * (linux scsi_host_template.eh_dev_reset_handler routine)
> @@ -1809,6 +1809,58 @@ mptscsih_dev_reset(struct scsi_cmnd * SCpnt)
> VirtDevice *vdevice;
> MPT_ADAPTER *ioc;
>
> + /* If we can't locate our host adapter structure, return FAILED status.
> + */
> + if ((hd = shost_priv(SCpnt->device->host)) == NULL){
> + printk(KERN_ERR MYNAM ": lun reset: "
> + "Can't locate host! (sc=%p)\n", SCpnt);
> + return FAILED;
> + }
> +
> + ioc = hd->ioc;
> + printk(MYIOC_s_INFO_FMT "attempting lun reset! (sc=%p)\n",
> + ioc->name, SCpnt);
> + scsi_print_command(SCpnt);
> +
> + vdevice = SCpnt->device->hostdata;
> + if (!vdevice || !vdevice->vtarget) {
> + retval = 0;
> + goto out;
> + }
> +
> + retval = mptscsih_IssueTaskMgmt(hd,
> + MPI_SCSITASKMGMT_TASKTYPE_LOGICAL_UNIT_RESET,
> + vdevice->vtarget->channel,
> + vdevice->vtarget->id, vdevice->lun, 0,
> + mptscsih_get_tm_timeout(ioc));
> +
> + out:
> + printk (MYIOC_s_INFO_FMT "lun reset: %s (sc=%p)\n",
> + ioc->name, ((retval == 0) ? "SUCCESS" : "FAILED" ), SCpnt);
> +
> + if (retval == 0)
> + return SUCCESS;
> + else
> + return FAILED;
> +}
> +
> +/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> +/**
> + * mptscsih_target_reset - Perform a SCSI TARGET_RESET!
> + * @SCpnt: Pointer to scsi_cmnd structure, IO which reset is due to
> + *
> + * (linux scsi_host_template.eh_target_reset_handler routine)
> + *
> + * Returns SUCCESS or FAILED.
> + **/
> +int
> +mptscsih_target_reset(struct scsi_cmnd * SCpnt)
> +{
> + MPT_SCSI_HOST *hd;
> + int retval;
> + VirtDevice *vdevice;
> + MPT_ADAPTER *ioc;
> +
> /* If we can't locate our host adapter structure, return FAILED status.
> */
> if ((hd = shost_priv(SCpnt->device->host)) == NULL){
> @@ -3257,6 +3309,7 @@ EXPORT_SYMBOL(mptscsih_slave_destroy);
> EXPORT_SYMBOL(mptscsih_slave_configure);
> EXPORT_SYMBOL(mptscsih_abort);
> EXPORT_SYMBOL(mptscsih_dev_reset);
> +EXPORT_SYMBOL(mptscsih_target_reset);
> EXPORT_SYMBOL(mptscsih_bus_reset);
> EXPORT_SYMBOL(mptscsih_host_reset);
> EXPORT_SYMBOL(mptscsih_bios_param);
> diff --git a/drivers/message/fusion/mptscsih.h b/drivers/message/fusion/mptscsih.h
> index a22c5eaf703c..e3d92c392673 100644
> --- a/drivers/message/fusion/mptscsih.h
> +++ b/drivers/message/fusion/mptscsih.h
> @@ -120,6 +120,7 @@ extern void mptscsih_slave_destroy(struct scsi_device *device);
> extern int mptscsih_slave_configure(struct scsi_device *device);
> extern int mptscsih_abort(struct scsi_cmnd * SCpnt);
> extern int mptscsih_dev_reset(struct scsi_cmnd * SCpnt);
> +extern int mptscsih_target_reset(struct scsi_cmnd * SCpnt);
> extern int mptscsih_bus_reset(struct scsi_cmnd * SCpnt);
> extern int mptscsih_host_reset(struct scsi_cmnd *SCpnt);
> extern int mptscsih_bios_param(struct scsi_device * sdev, struct block_device *bdev, sector_t capacity, int geom[]);
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 04/20] mptfusion: correct definitions for mptscsih_dev_reset()
2022-05-19 17:27 ` Ewan Milne
@ 2022-05-23 12:25 ` Hannes Reinecke
0 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-23 12:25 UTC (permalink / raw)
To: Ewan Milne
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Johannes Thumshirn
On 5/19/22 19:27, Ewan Milne wrote:
> This patch appears to change the EH activity from issuing a target
> reset to a LUN reset only
> (since it does not add an entry in the host templates for
> .eh_target_reset_handler to invoke
> the new mptscsih_target_reset() function, nor does any other patch in
> the series).
>
> Is this going to work properly, and without escalating to a host reset
> which might not have happened before?
>
You are correct, the drivers need to be changed to refer to target_reset
now. Will be fixing it with the next version.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 05/20] mptfc: open-code mptfc_block_error_handler() for bus reset
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (3 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 04/20] mptfusion: correct definitions for mptscsih_dev_reset() Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-19 17:05 ` Ewan Milne
2022-05-12 11:12 ` [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf() Hannes Reinecke
` (14 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Johannes Thumshirn
When calling bus_reset we have potentially several ports to be
reset, so this patch open-codes the existing mptfc_block_error_handler()
to wait for all ports attached to this bus.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/message/fusion/mptfc.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
index ea8fc32dacfa..4a621380a4eb 100644
--- a/drivers/message/fusion/mptfc.c
+++ b/drivers/message/fusion/mptfc.c
@@ -262,12 +262,23 @@ static int
mptfc_bus_reset(struct scsi_cmnd *SCpnt)
{
struct Scsi_Host *shost = SCpnt->device->host;
- struct fc_rport *rport = starget_to_rport(scsi_target(SCpnt->device));
MPT_SCSI_HOST __maybe_unused *hd = shost_priv(shost);
+ int channel = SCpnt->device->channel;
+ struct mptfc_rport_info *ri;
int rtn;
- rtn = mptfc_block_error_handler(rport);
- if (rtn == SUCCESS) {
+ list_for_each_entry(ri, &hd->ioc->fc_rports, list) {
+ if (ri->flags & MPT_RPORT_INFO_FLAGS_REGISTERED) {
+ VirtTarget *vtarget = ri->starget->hostdata;
+
+ if (!vtarget || vtarget->channel != channel)
+ continue;
+ rtn = fc_block_rport(ri->rport);
+ if (rtn != 0)
+ break;
+ }
+ }
+ if (rtn == 0) {
dfcprintk (hd->ioc, printk(MYIOC_s_DEBUG_FMT
"%s.%d: %d:%llu, executing recovery.\n", __func__,
hd->ioc->name, shost->host_no,
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 05/20] mptfc: open-code mptfc_block_error_handler() for bus reset
2022-05-12 11:12 ` [PATCH 05/20] mptfc: open-code mptfc_block_error_handler() for bus reset Hannes Reinecke
@ 2022-05-19 17:05 ` Ewan Milne
2022-05-20 6:42 ` Hannes Reinecke
0 siblings, 1 reply; 34+ messages in thread
From: Ewan Milne @ 2022-05-19 17:05 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn
This change removes the debug log messages that mptfc_block_error_handler()
would have generated, and also doesn't do the same thing, at all, for each rport
due to the differences in the logic between fc_block_rport() and
fc_remote_port_chkready()
and waiting for ioc->active == 0.
Perhaps this is desirable and intentional, if so, could you explain
this in the commit message?
-Ewan
On Thu, May 12, 2022 at 7:13 AM Hannes Reinecke <hare@suse.de> wrote:
>
> When calling bus_reset we have potentially several ports to be
> reset, so this patch open-codes the existing mptfc_block_error_handler()
> to wait for all ports attached to this bus.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
> drivers/message/fusion/mptfc.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
> index ea8fc32dacfa..4a621380a4eb 100644
> --- a/drivers/message/fusion/mptfc.c
> +++ b/drivers/message/fusion/mptfc.c
> @@ -262,12 +262,23 @@ static int
> mptfc_bus_reset(struct scsi_cmnd *SCpnt)
> {
> struct Scsi_Host *shost = SCpnt->device->host;
> - struct fc_rport *rport = starget_to_rport(scsi_target(SCpnt->device));
> MPT_SCSI_HOST __maybe_unused *hd = shost_priv(shost);
> + int channel = SCpnt->device->channel;
> + struct mptfc_rport_info *ri;
> int rtn;
>
> - rtn = mptfc_block_error_handler(rport);
> - if (rtn == SUCCESS) {
> + list_for_each_entry(ri, &hd->ioc->fc_rports, list) {
> + if (ri->flags & MPT_RPORT_INFO_FLAGS_REGISTERED) {
> + VirtTarget *vtarget = ri->starget->hostdata;
> +
> + if (!vtarget || vtarget->channel != channel)
> + continue;
> + rtn = fc_block_rport(ri->rport);
> + if (rtn != 0)
> + break;
> + }
> + }
> + if (rtn == 0) {
> dfcprintk (hd->ioc, printk(MYIOC_s_DEBUG_FMT
> "%s.%d: %d:%llu, executing recovery.\n", __func__,
> hd->ioc->name, shost->host_no,
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/20] mptfc: open-code mptfc_block_error_handler() for bus reset
2022-05-19 17:05 ` Ewan Milne
@ 2022-05-20 6:42 ` Hannes Reinecke
0 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-20 6:42 UTC (permalink / raw)
To: Ewan Milne
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn
On 5/19/22 10:05, Ewan Milne wrote:
> This change removes the debug log messages that mptfc_block_error_handler()
> would have generated, and also doesn't do the same thing, at all, for each rport
> due to the differences in the logic between fc_block_rport() and
> fc_remote_port_chkready()
> and waiting for ioc->active == 0.
>
Yes, I'll be adding back the checks for ioc->active.
> Perhaps this is desirable and intentional, if so, could you explain
> this in the commit message?
>
Removing the debug calls was intentional; if needs be they can be
activated via scsi_logging_level.
I'll be updating the commit message.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf()
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (4 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 05/20] mptfc: open-code mptfc_block_error_handler() for bus reset Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-19 18:22 ` Ewan Milne
2022-05-12 11:12 ` [PATCH 07/20] bnx2fc: Do not rely on a scsi command for lun or target reset Hannes Reinecke
` (13 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Johannes Thumshirn, Saurav Kashyap
When sending a TMF we're only concerned with the rport and the LUN ID,
so use struct fc_rport as argument for qedf_initiate_tmf().
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Saurav Kashyap <skashyap@marvell.com>
---
drivers/scsi/qedf/qedf.h | 5 ++-
drivers/scsi/qedf/qedf_io.c | 75 ++++++++++-------------------------
drivers/scsi/qedf/qedf_main.c | 19 ++++-----
3 files changed, 33 insertions(+), 66 deletions(-)
diff --git a/drivers/scsi/qedf/qedf.h b/drivers/scsi/qedf/qedf.h
index c5c0bbdafc4e..80814bcf4db1 100644
--- a/drivers/scsi/qedf/qedf.h
+++ b/drivers/scsi/qedf/qedf.h
@@ -112,6 +112,7 @@ struct qedf_ioreq {
#define QEDF_CMD_ERR_SCSI_DONE 0x5
u8 io_req_flags;
uint8_t tm_flags;
+ u64 tm_lun;
struct qedf_rport *fcport;
#define QEDF_CMD_ST_INACTIVE 0
#define QEDFC_CMD_ST_IO_ACTIVE 1
@@ -497,7 +498,7 @@ extern void qedf_process_warning_compl(struct qedf_ctx *qedf,
struct fcoe_cqe *cqe, struct qedf_ioreq *io_req);
extern void qedf_process_error_detect(struct qedf_ctx *qedf,
struct fcoe_cqe *cqe, struct qedf_ioreq *io_req);
-extern void qedf_flush_active_ios(struct qedf_rport *fcport, int lun);
+extern void qedf_flush_active_ios(struct qedf_rport *fcport, u64 lun);
extern void qedf_release_cmd(struct kref *ref);
extern int qedf_initiate_abts(struct qedf_ioreq *io_req,
bool return_scsi_cmd_on_abts);
@@ -522,7 +523,7 @@ extern int qedf_initiate_cleanup(struct qedf_ioreq *io_req,
bool return_scsi_cmd_on_abts);
extern void qedf_process_cleanup_compl(struct qedf_ctx *qedf,
struct fcoe_cqe *cqe, struct qedf_ioreq *io_req);
-extern int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags);
+extern int qedf_initiate_tmf(struct fc_rport *rport, u64 lun, u8 tm_flags);
extern void qedf_process_tmf_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
struct qedf_ioreq *io_req);
extern void qedf_process_cqe(struct qedf_ctx *qedf, struct fcoe_cqe *cqe);
diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index 2ec1f710fd1d..5d82fd95e74c 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -546,7 +546,7 @@ static int qedf_build_bd_list_from_sg(struct qedf_ioreq *io_req)
}
static void qedf_build_fcp_cmnd(struct qedf_ioreq *io_req,
- struct fcp_cmnd *fcp_cmnd)
+ struct fcp_cmnd *fcp_cmnd)
{
struct scsi_cmnd *sc_cmd = io_req->sc_cmd;
@@ -554,8 +554,12 @@ static void qedf_build_fcp_cmnd(struct qedf_ioreq *io_req,
memset(fcp_cmnd, 0, FCP_CMND_LEN);
/* 8 bytes: SCSI LUN info */
- int_to_scsilun(sc_cmd->device->lun,
- (struct scsi_lun *)&fcp_cmnd->fc_lun);
+ if (io_req->cmd_type == QEDF_TASK_MGMT_CMD)
+ int_to_scsilun(io_req->tm_lun,
+ (struct scsi_lun *)&fcp_cmnd->fc_lun);
+ else
+ int_to_scsilun(sc_cmd->device->lun,
+ (struct scsi_lun *)&fcp_cmnd->fc_lun);
/* 4 bytes: flag info */
fcp_cmnd->fc_pri_ta = 0;
@@ -1096,7 +1100,7 @@ static void qedf_parse_fcp_rsp(struct qedf_ioreq *io_req,
}
/* The sense buffer can be NULL for TMF commands */
- if (sc_cmd->sense_buffer) {
+ if (sc_cmd && sc_cmd->sense_buffer) {
memset(sc_cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
if (fcp_sns_len)
memcpy(sc_cmd->sense_buffer, sense_data,
@@ -1581,7 +1585,7 @@ static void qedf_flush_els_req(struct qedf_ctx *qedf,
/* A value of -1 for lun is a wild card that means flush all
* active SCSI I/Os for the target.
*/
-void qedf_flush_active_ios(struct qedf_rport *fcport, int lun)
+void qedf_flush_active_ios(struct qedf_rport *fcport, u64 lun)
{
struct qedf_ioreq *io_req;
struct qedf_ctx *qedf;
@@ -1769,10 +1773,6 @@ void qedf_flush_active_ios(struct qedf_rport *fcport, int lun)
kref_put(&io_req->refcount, qedf_release_cmd);
continue;
}
- if (lun > -1) {
- if (io_req->lun != lun)
- continue;
- }
/*
* Use kref_get_unless_zero in the unlikely case the command
@@ -2282,7 +2282,7 @@ void qedf_process_cleanup_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
complete(&io_req->cleanup_done);
}
-static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
+static int qedf_execute_tmf(struct qedf_rport *fcport, u64 tm_lun,
uint8_t tm_flags)
{
struct qedf_ioreq *io_req;
@@ -2292,17 +2292,10 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
int rc = 0;
uint16_t xid;
int tmo = 0;
- int lun = 0;
unsigned long flags;
struct fcoe_wqe *sqe;
u16 sqe_idx;
- if (!sc_cmd) {
- QEDF_ERR(&qedf->dbg_ctx, "sc_cmd is NULL\n");
- return FAILED;
- }
-
- lun = (int)sc_cmd->device->lun;
if (!test_bit(QEDF_RPORT_SESSION_READY, &fcport->flags)) {
QEDF_ERR(&(qedf->dbg_ctx), "fcport not offloaded\n");
rc = FAILED;
@@ -2322,7 +2315,7 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
qedf->target_resets++;
/* Initialize rest of io_req fields */
- io_req->sc_cmd = sc_cmd;
+ io_req->sc_cmd = NULL;
io_req->fcport = fcport;
io_req->cmd_type = QEDF_TASK_MGMT_CMD;
@@ -2336,6 +2329,7 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
/* Default is to return a SCSI command when an error occurs */
io_req->return_scsi_cmd_on_abts = false;
+ io_req->tm_lun = tm_lun;
/* Obtain exchange id */
xid = io_req->xid;
@@ -2390,7 +2384,7 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
if (tm_flags == FCP_TMF_LUN_RESET)
- qedf_flush_active_ios(fcport, lun);
+ qedf_flush_active_ios(fcport, tm_lun);
else
qedf_flush_active_ios(fcport, -1);
@@ -2405,23 +2399,18 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
return rc;
}
-int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
+int qedf_initiate_tmf(struct fc_rport *rport, u64 lun, u8 tm_flags)
{
- struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
struct fc_rport_libfc_priv *rp = rport->dd_data;
struct qedf_rport *fcport = (struct qedf_rport *)&rp[1];
- struct qedf_ctx *qedf;
- struct fc_lport *lport = shost_priv(sc_cmd->device->host);
+ struct qedf_ctx *qedf = fcport->qedf;
+ struct fc_lport *lport = rp->local_port;
int rc = SUCCESS;
- int rval;
- struct qedf_ioreq *io_req = NULL;
- int ref_cnt = 0;
struct fc_rport_priv *rdata = fcport->rdata;
QEDF_ERR(NULL,
- "tm_flags 0x%x sc_cmd %p op = 0x%02x target_id = 0x%x lun=%d\n",
- tm_flags, sc_cmd, sc_cmd->cmd_len ? sc_cmd->cmnd[0] : 0xff,
- rport->scsi_target_id, (int)sc_cmd->device->lun);
+ "tm_flags 0x%x target_id = 0x%x lun=%llu\n",
+ tm_flags, rport->scsi_target_id, lun);
if (!rdata || !kref_get_unless_zero(&rdata->kref)) {
QEDF_ERR(NULL, "stale rport\n");
@@ -2432,33 +2421,10 @@ int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
(tm_flags == FCP_TMF_TGT_RESET) ? "TARGET RESET" :
"LUN RESET");
- if (qedf_priv(sc_cmd)->io_req) {
- io_req = qedf_priv(sc_cmd)->io_req;
- ref_cnt = kref_read(&io_req->refcount);
- QEDF_ERR(NULL,
- "orig io_req = %p xid = 0x%x ref_cnt = %d.\n",
- io_req, io_req->xid, ref_cnt);
- }
-
- rval = fc_remote_port_chkready(rport);
- if (rval) {
- QEDF_ERR(NULL, "device_reset rport not ready\n");
- rc = FAILED;
- goto tmf_err;
- }
-
- rc = fc_block_scsi_eh(sc_cmd);
+ rc = fc_block_rport(rport);
if (rc)
goto tmf_err;
- if (!fcport) {
- QEDF_ERR(NULL, "device_reset: rport is NULL\n");
- rc = FAILED;
- goto tmf_err;
- }
-
- qedf = fcport->qedf;
-
if (!qedf) {
QEDF_ERR(NULL, "qedf is NULL.\n");
rc = FAILED;
@@ -2495,7 +2461,7 @@ int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
goto tmf_err;
}
- rc = qedf_execute_tmf(fcport, sc_cmd, tm_flags);
+ rc = qedf_execute_tmf(fcport, lun, tm_flags);
tmf_err:
kref_put(&rdata->kref, fc_rport_destroy);
@@ -2512,7 +2478,6 @@ void qedf_process_tmf_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
fcp_rsp = &cqe->cqe_info.rsp_info;
qedf_parse_fcp_rsp(io_req, fcp_rsp);
- io_req->sc_cmd = NULL;
complete(&io_req->tm_done);
}
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 18dc68d577b6..85ccfbc5cd26 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -773,7 +773,7 @@ static int qedf_eh_abort(struct scsi_cmnd *sc_cmd)
goto drop_rdata_kref;
}
- rc = fc_block_scsi_eh(sc_cmd);
+ rc = fc_block_rport(rport);
if (rc)
goto drop_rdata_kref;
@@ -857,18 +857,19 @@ static int qedf_eh_abort(struct scsi_cmnd *sc_cmd)
static int qedf_eh_target_reset(struct scsi_cmnd *sc_cmd)
{
- QEDF_ERR(NULL, "%d:0:%d:%lld: TARGET RESET Issued...",
- sc_cmd->device->host->host_no, sc_cmd->device->id,
- sc_cmd->device->lun);
- return qedf_initiate_tmf(sc_cmd, FCP_TMF_TGT_RESET);
+ struct scsi_target *starget = scsi_target(sc_cmd->device);
+ struct fc_rport *rport = starget_to_rport(starget);
+
+ QEDF_ERR(NULL, "TARGET RESET Issued...");
+ return qedf_initiate_tmf(rport, 0, FCP_TMF_TGT_RESET);
}
static int qedf_eh_device_reset(struct scsi_cmnd *sc_cmd)
{
- QEDF_ERR(NULL, "%d:0:%d:%lld: LUN RESET Issued... ",
- sc_cmd->device->host->host_no, sc_cmd->device->id,
- sc_cmd->device->lun);
- return qedf_initiate_tmf(sc_cmd, FCP_TMF_LUN_RESET);
+ struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
+
+ QEDF_ERR(NULL, "LUN RESET Issued...\n");
+ return qedf_initiate_tmf(rport, sc_cmd->device->lun, FCP_TMF_LUN_RESET);
}
bool qedf_wait_for_upload(struct qedf_ctx *qedf)
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf()
2022-05-12 11:12 ` [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf() Hannes Reinecke
@ 2022-05-19 18:22 ` Ewan Milne
2022-05-20 6:49 ` Hannes Reinecke
0 siblings, 1 reply; 34+ messages in thread
From: Ewan Milne @ 2022-05-19 18:22 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn, Saurav Kashyap
The patch changes the data type of the 'lun' argument to qedf_flush_active_ios()
to a u64, but the remaining code still uses a wildcard of -1, perhaps
this needs a
#define or enum of a value that is unsigned also?
Removing the call to fc_remote_port_chkready() in qedf_initiate_tmf()
will result
in different semantics for whether the TMF will be issued.
Changing the debug logging in qedf_eh_target_reset() and qedf_eh_device_reset()
might make identifying the target more difficult, although
qedf_initiate_tmf() will
also log a message, the rport->scsi_target_id is not the same value as
the sdev->id.
-Ewan
On Thu, May 12, 2022 at 7:13 AM Hannes Reinecke <hare@suse.de> wrote:
>
> When sending a TMF we're only concerned with the rport and the LUN ID,
> so use struct fc_rport as argument for qedf_initiate_tmf().
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Cc: Saurav Kashyap <skashyap@marvell.com>
> ---
> drivers/scsi/qedf/qedf.h | 5 ++-
> drivers/scsi/qedf/qedf_io.c | 75 ++++++++++-------------------------
> drivers/scsi/qedf/qedf_main.c | 19 ++++-----
> 3 files changed, 33 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/scsi/qedf/qedf.h b/drivers/scsi/qedf/qedf.h
> index c5c0bbdafc4e..80814bcf4db1 100644
> --- a/drivers/scsi/qedf/qedf.h
> +++ b/drivers/scsi/qedf/qedf.h
> @@ -112,6 +112,7 @@ struct qedf_ioreq {
> #define QEDF_CMD_ERR_SCSI_DONE 0x5
> u8 io_req_flags;
> uint8_t tm_flags;
> + u64 tm_lun;
> struct qedf_rport *fcport;
> #define QEDF_CMD_ST_INACTIVE 0
> #define QEDFC_CMD_ST_IO_ACTIVE 1
> @@ -497,7 +498,7 @@ extern void qedf_process_warning_compl(struct qedf_ctx *qedf,
> struct fcoe_cqe *cqe, struct qedf_ioreq *io_req);
> extern void qedf_process_error_detect(struct qedf_ctx *qedf,
> struct fcoe_cqe *cqe, struct qedf_ioreq *io_req);
> -extern void qedf_flush_active_ios(struct qedf_rport *fcport, int lun);
> +extern void qedf_flush_active_ios(struct qedf_rport *fcport, u64 lun);
> extern void qedf_release_cmd(struct kref *ref);
> extern int qedf_initiate_abts(struct qedf_ioreq *io_req,
> bool return_scsi_cmd_on_abts);
> @@ -522,7 +523,7 @@ extern int qedf_initiate_cleanup(struct qedf_ioreq *io_req,
> bool return_scsi_cmd_on_abts);
> extern void qedf_process_cleanup_compl(struct qedf_ctx *qedf,
> struct fcoe_cqe *cqe, struct qedf_ioreq *io_req);
> -extern int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags);
> +extern int qedf_initiate_tmf(struct fc_rport *rport, u64 lun, u8 tm_flags);
> extern void qedf_process_tmf_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
> struct qedf_ioreq *io_req);
> extern void qedf_process_cqe(struct qedf_ctx *qedf, struct fcoe_cqe *cqe);
> diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
> index 2ec1f710fd1d..5d82fd95e74c 100644
> --- a/drivers/scsi/qedf/qedf_io.c
> +++ b/drivers/scsi/qedf/qedf_io.c
> @@ -546,7 +546,7 @@ static int qedf_build_bd_list_from_sg(struct qedf_ioreq *io_req)
> }
>
> static void qedf_build_fcp_cmnd(struct qedf_ioreq *io_req,
> - struct fcp_cmnd *fcp_cmnd)
> + struct fcp_cmnd *fcp_cmnd)
> {
> struct scsi_cmnd *sc_cmd = io_req->sc_cmd;
>
> @@ -554,8 +554,12 @@ static void qedf_build_fcp_cmnd(struct qedf_ioreq *io_req,
> memset(fcp_cmnd, 0, FCP_CMND_LEN);
>
> /* 8 bytes: SCSI LUN info */
> - int_to_scsilun(sc_cmd->device->lun,
> - (struct scsi_lun *)&fcp_cmnd->fc_lun);
> + if (io_req->cmd_type == QEDF_TASK_MGMT_CMD)
> + int_to_scsilun(io_req->tm_lun,
> + (struct scsi_lun *)&fcp_cmnd->fc_lun);
> + else
> + int_to_scsilun(sc_cmd->device->lun,
> + (struct scsi_lun *)&fcp_cmnd->fc_lun);
>
> /* 4 bytes: flag info */
> fcp_cmnd->fc_pri_ta = 0;
> @@ -1096,7 +1100,7 @@ static void qedf_parse_fcp_rsp(struct qedf_ioreq *io_req,
> }
>
> /* The sense buffer can be NULL for TMF commands */
> - if (sc_cmd->sense_buffer) {
> + if (sc_cmd && sc_cmd->sense_buffer) {
> memset(sc_cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
> if (fcp_sns_len)
> memcpy(sc_cmd->sense_buffer, sense_data,
> @@ -1581,7 +1585,7 @@ static void qedf_flush_els_req(struct qedf_ctx *qedf,
> /* A value of -1 for lun is a wild card that means flush all
> * active SCSI I/Os for the target.
> */
> -void qedf_flush_active_ios(struct qedf_rport *fcport, int lun)
> +void qedf_flush_active_ios(struct qedf_rport *fcport, u64 lun)
> {
> struct qedf_ioreq *io_req;
> struct qedf_ctx *qedf;
> @@ -1769,10 +1773,6 @@ void qedf_flush_active_ios(struct qedf_rport *fcport, int lun)
> kref_put(&io_req->refcount, qedf_release_cmd);
> continue;
> }
> - if (lun > -1) {
> - if (io_req->lun != lun)
> - continue;
> - }
>
> /*
> * Use kref_get_unless_zero in the unlikely case the command
> @@ -2282,7 +2282,7 @@ void qedf_process_cleanup_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
> complete(&io_req->cleanup_done);
> }
>
> -static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
> +static int qedf_execute_tmf(struct qedf_rport *fcport, u64 tm_lun,
> uint8_t tm_flags)
> {
> struct qedf_ioreq *io_req;
> @@ -2292,17 +2292,10 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
> int rc = 0;
> uint16_t xid;
> int tmo = 0;
> - int lun = 0;
> unsigned long flags;
> struct fcoe_wqe *sqe;
> u16 sqe_idx;
>
> - if (!sc_cmd) {
> - QEDF_ERR(&qedf->dbg_ctx, "sc_cmd is NULL\n");
> - return FAILED;
> - }
> -
> - lun = (int)sc_cmd->device->lun;
> if (!test_bit(QEDF_RPORT_SESSION_READY, &fcport->flags)) {
> QEDF_ERR(&(qedf->dbg_ctx), "fcport not offloaded\n");
> rc = FAILED;
> @@ -2322,7 +2315,7 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
> qedf->target_resets++;
>
> /* Initialize rest of io_req fields */
> - io_req->sc_cmd = sc_cmd;
> + io_req->sc_cmd = NULL;
> io_req->fcport = fcport;
> io_req->cmd_type = QEDF_TASK_MGMT_CMD;
>
> @@ -2336,6 +2329,7 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
>
> /* Default is to return a SCSI command when an error occurs */
> io_req->return_scsi_cmd_on_abts = false;
> + io_req->tm_lun = tm_lun;
>
> /* Obtain exchange id */
> xid = io_req->xid;
> @@ -2390,7 +2384,7 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
>
>
> if (tm_flags == FCP_TMF_LUN_RESET)
> - qedf_flush_active_ios(fcport, lun);
> + qedf_flush_active_ios(fcport, tm_lun);
> else
> qedf_flush_active_ios(fcport, -1);
>
> @@ -2405,23 +2399,18 @@ static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
> return rc;
> }
>
> -int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
> +int qedf_initiate_tmf(struct fc_rport *rport, u64 lun, u8 tm_flags)
> {
> - struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
> struct fc_rport_libfc_priv *rp = rport->dd_data;
> struct qedf_rport *fcport = (struct qedf_rport *)&rp[1];
> - struct qedf_ctx *qedf;
> - struct fc_lport *lport = shost_priv(sc_cmd->device->host);
> + struct qedf_ctx *qedf = fcport->qedf;
> + struct fc_lport *lport = rp->local_port;
> int rc = SUCCESS;
> - int rval;
> - struct qedf_ioreq *io_req = NULL;
> - int ref_cnt = 0;
> struct fc_rport_priv *rdata = fcport->rdata;
>
> QEDF_ERR(NULL,
> - "tm_flags 0x%x sc_cmd %p op = 0x%02x target_id = 0x%x lun=%d\n",
> - tm_flags, sc_cmd, sc_cmd->cmd_len ? sc_cmd->cmnd[0] : 0xff,
> - rport->scsi_target_id, (int)sc_cmd->device->lun);
> + "tm_flags 0x%x target_id = 0x%x lun=%llu\n",
> + tm_flags, rport->scsi_target_id, lun);
>
> if (!rdata || !kref_get_unless_zero(&rdata->kref)) {
> QEDF_ERR(NULL, "stale rport\n");
> @@ -2432,33 +2421,10 @@ int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
> (tm_flags == FCP_TMF_TGT_RESET) ? "TARGET RESET" :
> "LUN RESET");
>
> - if (qedf_priv(sc_cmd)->io_req) {
> - io_req = qedf_priv(sc_cmd)->io_req;
> - ref_cnt = kref_read(&io_req->refcount);
> - QEDF_ERR(NULL,
> - "orig io_req = %p xid = 0x%x ref_cnt = %d.\n",
> - io_req, io_req->xid, ref_cnt);
> - }
> -
> - rval = fc_remote_port_chkready(rport);
> - if (rval) {
> - QEDF_ERR(NULL, "device_reset rport not ready\n");
> - rc = FAILED;
> - goto tmf_err;
> - }
> -
> - rc = fc_block_scsi_eh(sc_cmd);
> + rc = fc_block_rport(rport);
> if (rc)
> goto tmf_err;
>
> - if (!fcport) {
> - QEDF_ERR(NULL, "device_reset: rport is NULL\n");
> - rc = FAILED;
> - goto tmf_err;
> - }
> -
> - qedf = fcport->qedf;
> -
> if (!qedf) {
> QEDF_ERR(NULL, "qedf is NULL.\n");
> rc = FAILED;
> @@ -2495,7 +2461,7 @@ int qedf_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
> goto tmf_err;
> }
>
> - rc = qedf_execute_tmf(fcport, sc_cmd, tm_flags);
> + rc = qedf_execute_tmf(fcport, lun, tm_flags);
>
> tmf_err:
> kref_put(&rdata->kref, fc_rport_destroy);
> @@ -2512,7 +2478,6 @@ void qedf_process_tmf_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
> fcp_rsp = &cqe->cqe_info.rsp_info;
> qedf_parse_fcp_rsp(io_req, fcp_rsp);
>
> - io_req->sc_cmd = NULL;
> complete(&io_req->tm_done);
> }
>
> diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
> index 18dc68d577b6..85ccfbc5cd26 100644
> --- a/drivers/scsi/qedf/qedf_main.c
> +++ b/drivers/scsi/qedf/qedf_main.c
> @@ -773,7 +773,7 @@ static int qedf_eh_abort(struct scsi_cmnd *sc_cmd)
> goto drop_rdata_kref;
> }
>
> - rc = fc_block_scsi_eh(sc_cmd);
> + rc = fc_block_rport(rport);
> if (rc)
> goto drop_rdata_kref;
>
> @@ -857,18 +857,19 @@ static int qedf_eh_abort(struct scsi_cmnd *sc_cmd)
>
> static int qedf_eh_target_reset(struct scsi_cmnd *sc_cmd)
> {
> - QEDF_ERR(NULL, "%d:0:%d:%lld: TARGET RESET Issued...",
> - sc_cmd->device->host->host_no, sc_cmd->device->id,
> - sc_cmd->device->lun);
> - return qedf_initiate_tmf(sc_cmd, FCP_TMF_TGT_RESET);
> + struct scsi_target *starget = scsi_target(sc_cmd->device);
> + struct fc_rport *rport = starget_to_rport(starget);
> +
> + QEDF_ERR(NULL, "TARGET RESET Issued...");
> + return qedf_initiate_tmf(rport, 0, FCP_TMF_TGT_RESET);
> }
>
> static int qedf_eh_device_reset(struct scsi_cmnd *sc_cmd)
> {
> - QEDF_ERR(NULL, "%d:0:%d:%lld: LUN RESET Issued... ",
> - sc_cmd->device->host->host_no, sc_cmd->device->id,
> - sc_cmd->device->lun);
> - return qedf_initiate_tmf(sc_cmd, FCP_TMF_LUN_RESET);
> + struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
> +
> + QEDF_ERR(NULL, "LUN RESET Issued...\n");
> + return qedf_initiate_tmf(rport, sc_cmd->device->lun, FCP_TMF_LUN_RESET);
> }
>
> bool qedf_wait_for_upload(struct qedf_ctx *qedf)
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf()
2022-05-19 18:22 ` Ewan Milne
@ 2022-05-20 6:49 ` Hannes Reinecke
2022-05-25 19:06 ` Ewan Milne
0 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-20 6:49 UTC (permalink / raw)
To: Ewan Milne
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn, Saurav Kashyap
On 5/19/22 11:22, Ewan Milne wrote:
> The patch changes the data type of the 'lun' argument to qedf_flush_active_ios()
> to a u64, but the remaining code still uses a wildcard of -1, perhaps
> this needs a
> #define or enum of a value that is unsigned also?
>
Ah, no, I went slightly overboard there. That needs to be changed back
to be an 'int'.
> Removing the call to fc_remote_port_chkready() in qedf_initiate_tmf()
> will result
> in different semantics for whether the TMF will be issued.
>
Really? 'fc_remote_port_chkready()' just evaluates the port state;
this is also done by fc_block_rport().
So dropping the first shouldn't make a difference.
> Changing the debug logging in qedf_eh_target_reset() and qedf_eh_device_reset()
> might make identifying the target more difficult, although
> qedf_initiate_tmf() will
> also log a message, the rport->scsi_target_id is not the same value as
> the sdev->id.
>
Sigh. Yes, the error logging is suboptimal.
Will be updating it.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf()
2022-05-20 6:49 ` Hannes Reinecke
@ 2022-05-25 19:06 ` Ewan Milne
2022-05-27 5:39 ` Hannes Reinecke
0 siblings, 1 reply; 34+ messages in thread
From: Ewan Milne @ 2022-05-25 19:06 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn, Saurav Kashyap,
James Smart
Well, fc_remote_port_chkready() has in the _ONLINE and _MARGINAL case:
if (rport->roles & FC_PORT_ROLE_FCP_TARGET)
result = 0;
else if (rport->flags & FC_RPORT_DEVLOSS_PENDING)
result = DID_IMM_RETRY << 16;
else
result = DID_NO_CONNECT << 16;
break;
which fc_block_rport() does not have. Admittedly, I would have thought that
the rport would be blocked while devloss was pending but there is code in
fc_timeout_deleted_rport() that indicates otherwise, maybe this only happens
if there is a role change.
-Ewan
On Fri, May 20, 2022 at 2:50 AM Hannes Reinecke <hare@suse.de> wrote:
>
> On 5/19/22 11:22, Ewan Milne wrote:
> > The patch changes the data type of the 'lun' argument to qedf_flush_active_ios()
> > to a u64, but the remaining code still uses a wildcard of -1, perhaps
> > this needs a
> > #define or enum of a value that is unsigned also?
> >
> Ah, no, I went slightly overboard there. That needs to be changed back
> to be an 'int'.
>
> > Removing the call to fc_remote_port_chkready() in qedf_initiate_tmf()
> > will result
> > in different semantics for whether the TMF will be issued.
> >
> Really? 'fc_remote_port_chkready()' just evaluates the port state;
> this is also done by fc_block_rport().
> So dropping the first shouldn't make a difference.
>
> > Changing the debug logging in qedf_eh_target_reset() and qedf_eh_device_reset()
> > might make identifying the target more difficult, although
> > qedf_initiate_tmf() will
> > also log a message, the rport->scsi_target_id is not the same value as
> > the sdev->id.
> >
> Sigh. Yes, the error logging is suboptimal.
> Will be updating it.
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke Kernel Storage Architect
> hare@suse.de +49 911 74053 688
> SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
> HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
> Myers, Andrew McDonald, Martje Boudien Moerman
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf()
2022-05-25 19:06 ` Ewan Milne
@ 2022-05-27 5:39 ` Hannes Reinecke
0 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-27 5:39 UTC (permalink / raw)
To: Ewan Milne
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn, Saurav Kashyap,
James Smart
On 5/25/22 21:06, Ewan Milne wrote:
> Well, fc_remote_port_chkready() has in the _ONLINE and _MARGINAL case:
>
> if (rport->roles & FC_PORT_ROLE_FCP_TARGET)
> result = 0;
> else if (rport->flags & FC_RPORT_DEVLOSS_PENDING)
> result = DID_IMM_RETRY << 16;
> else
> result = DID_NO_CONNECT << 16;
> break;
>
> which fc_block_rport() does not have. Admittedly, I would have thought that
> the rport would be blocked while devloss was pending but there is code in
> fc_timeout_deleted_rport() that indicates otherwise, maybe this only happens
> if there is a role change.
>
Sort of. But the code in qedf only deals with FCP target ports, so we'll
always take the first branch and the entire code is pointless.
Am I wrong?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 07/20] bnx2fc: Do not rely on a scsi command for lun or target reset
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (5 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 06/20] qedf: use fc rport as argument for qedf_initiate_tmf() Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 08/20] ibmvfc: open-code reset loop for " Hannes Reinecke
` (12 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Johannes Thumshirn, Saurav Kashyap
When a LUN or target reset is issued we should not rely on a scsi
command to be present; we'll have to reset the entire device or
target anyway.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Saurav Kashyap <skashyap@marvell.com>
---
drivers/scsi/bnx2fc/bnx2fc.h | 1 +
drivers/scsi/bnx2fc/bnx2fc_hwi.c | 14 +++--
drivers/scsi/bnx2fc/bnx2fc_io.c | 94 ++++++++++++++++----------------
3 files changed, 57 insertions(+), 52 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 046247420cfa..7e74f77da14f 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -384,6 +384,7 @@ struct bnx2fc_rport {
};
struct bnx2fc_mp_req {
+ u64 tm_lun;
u8 tm_flags;
u32 req_len;
diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
index 776544385598..090d436bcef8 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
@@ -1709,7 +1709,8 @@ void bnx2fc_init_task(struct bnx2fc_cmd *io_req,
struct fcoe_cached_sge_ctx *cached_sge;
struct fcoe_ext_mul_sges_ctx *sgl;
int dev_type = tgt->dev_type;
- u64 *fcp_cmnd;
+ struct fcp_cmnd *fcp_cmnd;
+ u64 *raw_fcp_cmnd;
u64 tmp_fcp_cmnd[4];
u32 context_id;
int cnt, i;
@@ -1778,16 +1779,19 @@ void bnx2fc_init_task(struct bnx2fc_cmd *io_req,
task->txwr_rxrd.union_ctx.tx_seq.ctx.seq_cnt = 1;
/* Fill FCP_CMND IU */
- fcp_cmnd = (u64 *)
+ fcp_cmnd = (struct fcp_cmnd *)&tmp_fcp_cmnd;
+ bnx2fc_build_fcp_cmnd(io_req, fcp_cmnd);
+ int_to_scsilun(sc_cmd->device->lun, &fcp_cmnd->fc_lun);
+ memcpy(fcp_cmnd->fc_cdb, sc_cmd->cmnd, sc_cmd->cmd_len);
+ raw_fcp_cmnd = (u64 *)
task->txwr_rxrd.union_ctx.fcp_cmd.opaque;
- bnx2fc_build_fcp_cmnd(io_req, (struct fcp_cmnd *)&tmp_fcp_cmnd);
/* swap fcp_cmnd */
cnt = sizeof(struct fcp_cmnd) / sizeof(u64);
for (i = 0; i < cnt; i++) {
- *fcp_cmnd = cpu_to_be64(tmp_fcp_cmnd[i]);
- fcp_cmnd++;
+ *raw_fcp_cmnd = cpu_to_be64(tmp_fcp_cmnd[i]);
+ raw_fcp_cmnd++;
}
/* Rx Write Tx Read */
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 962454f2e2b1..055d9424bc0d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -658,10 +658,9 @@ int bnx2fc_init_mp_req(struct bnx2fc_cmd *io_req)
return SUCCESS;
}
-static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
+static int bnx2fc_initiate_tmf(struct fc_lport *lport, struct fc_rport *rport,
+ u64 tm_lun, u8 tm_flags)
{
- struct fc_lport *lport;
- struct fc_rport *rport;
struct fc_rport_libfc_priv *rp;
struct fcoe_port *port;
struct bnx2fc_interface *interface;
@@ -670,7 +669,6 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
struct bnx2fc_mp_req *tm_req;
struct fcoe_task_ctx_entry *task;
struct fcoe_task_ctx_entry *task_page;
- struct Scsi_Host *host = sc_cmd->device->host;
struct fc_frame_header *fc_hdr;
struct fcp_cmnd *fcp_cmnd;
int task_idx, index;
@@ -679,8 +677,6 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
u32 sid, did;
unsigned long start = jiffies;
- lport = shost_priv(host);
- rport = starget_to_rport(scsi_target(sc_cmd->device));
port = lport_priv(lport);
interface = port->priv;
@@ -691,7 +687,7 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
}
rp = rport->dd_data;
- rc = fc_block_scsi_eh(sc_cmd);
+ rc = fc_block_rport(rport);
if (rc)
return rc;
@@ -720,7 +716,7 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
goto retry_tmf;
}
/* Initialize rest of io_req fields */
- io_req->sc_cmd = sc_cmd;
+ io_req->sc_cmd = NULL;
io_req->port = port;
io_req->tgt = tgt;
@@ -738,11 +734,13 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
/* Set TM flags */
io_req->io_req_flags = 0;
tm_req->tm_flags = tm_flags;
+ tm_req->tm_lun = tm_lun;
/* Fill FCP_CMND */
bnx2fc_build_fcp_cmnd(io_req, (struct fcp_cmnd *)tm_req->req_buf);
fcp_cmnd = (struct fcp_cmnd *)tm_req->req_buf;
- memset(fcp_cmnd->fc_cdb, 0, sc_cmd->cmd_len);
+ int_to_scsilun(tm_lun, &fcp_cmnd->fc_lun);
+ memset(fcp_cmnd->fc_cdb, 0, BNX2FC_MAX_CMD_LEN);
fcp_cmnd->fc_dl = 0;
/* Fill FC header */
@@ -765,8 +763,6 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8 tm_flags)
task = &(task_page[index]);
bnx2fc_init_mp_task(io_req, task);
- bnx2fc_priv(sc_cmd)->io_req = io_req;
-
/* Obtain free SQ entry */
spin_lock_bh(&tgt->tgt_lock);
bnx2fc_add_2_sq(tgt, xid);
@@ -1064,7 +1060,10 @@ int bnx2fc_initiate_cleanup(struct bnx2fc_cmd *io_req)
*/
int bnx2fc_eh_target_reset(struct scsi_cmnd *sc_cmd)
{
- return bnx2fc_initiate_tmf(sc_cmd, FCP_TMF_TGT_RESET);
+ struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
+ struct fc_lport *lport = shost_priv(rport_to_shost(rport));
+
+ return bnx2fc_initiate_tmf(lport, rport, 0, FCP_TMF_TGT_RESET);
}
/**
@@ -1077,7 +1076,11 @@ int bnx2fc_eh_target_reset(struct scsi_cmnd *sc_cmd)
*/
int bnx2fc_eh_device_reset(struct scsi_cmnd *sc_cmd)
{
- return bnx2fc_initiate_tmf(sc_cmd, FCP_TMF_LUN_RESET);
+ struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
+ struct fc_lport *lport = shost_priv(rport_to_shost(rport));
+
+ return bnx2fc_initiate_tmf(lport, rport, sc_cmd->device->lun,
+ FCP_TMF_LUN_RESET);
}
static int bnx2fc_abts_cleanup(struct bnx2fc_cmd *io_req)
@@ -1452,10 +1455,9 @@ void bnx2fc_process_abts_compl(struct bnx2fc_cmd *io_req,
static void bnx2fc_lun_reset_cmpl(struct bnx2fc_cmd *io_req)
{
- struct scsi_cmnd *sc_cmd = io_req->sc_cmd;
struct bnx2fc_rport *tgt = io_req->tgt;
struct bnx2fc_cmd *cmd, *tmp;
- u64 tm_lun = sc_cmd->device->lun;
+ struct bnx2fc_mp_req *tm_req = &io_req->mp_req;
u64 lun;
int rc = 0;
@@ -1467,8 +1469,10 @@ static void bnx2fc_lun_reset_cmpl(struct bnx2fc_cmd *io_req)
*/
list_for_each_entry_safe(cmd, tmp, &tgt->active_cmd_queue, link) {
BNX2FC_TGT_DBG(tgt, "LUN RST cmpl: scan for pending IOs\n");
+ if (!cmd->sc_cmd)
+ continue;
lun = cmd->sc_cmd->device->lun;
- if (lun == tm_lun) {
+ if (lun == tm_req->tm_lun) {
/* Initiate ABTS on this cmd */
if (!test_and_set_bit(BNX2FC_FLAG_ISSUE_ABTS,
&cmd->req_flags)) {
@@ -1572,31 +1576,36 @@ void bnx2fc_process_tm_compl(struct bnx2fc_cmd *io_req,
printk(KERN_ERR PFX "tmf's fc_hdr r_ctl = 0x%x\n",
fc_hdr->fh_r_ctl);
}
- if (!bnx2fc_priv(sc_cmd)->io_req) {
- printk(KERN_ERR PFX "tm_compl: io_req is NULL\n");
- return;
- }
- switch (io_req->fcp_status) {
- case FC_GOOD:
- if (io_req->cdb_status == 0) {
- /* Good IO completion */
- sc_cmd->result = DID_OK << 16;
- } else {
- /* Transport status is good, SCSI status not good */
- sc_cmd->result = (DID_OK << 16) | io_req->cdb_status;
+ if (sc_cmd) {
+ if (!bnx2fc_priv(sc_cmd)->io_req) {
+ printk(KERN_ERR PFX "tm_compl: io_req is NULL\n");
+ return;
+ }
+ switch (io_req->fcp_status) {
+ case FC_GOOD:
+ if (io_req->cdb_status == 0) {
+ /* Good IO completion */
+ sc_cmd->result = DID_OK << 16;
+ } else {
+ /* Transport status is good, SCSI status not good */
+ sc_cmd->result = (DID_OK << 16) | io_req->cdb_status;
+ }
+ if (io_req->fcp_resid)
+ scsi_set_resid(sc_cmd, io_req->fcp_resid);
+ break;
+
+ default:
+ BNX2FC_IO_DBG(io_req, "process_tm_compl: fcp_status = %d\n",
+ io_req->fcp_status);
+ break;
}
- if (io_req->fcp_resid)
- scsi_set_resid(sc_cmd, io_req->fcp_resid);
- break;
- default:
- BNX2FC_IO_DBG(io_req, "process_tm_compl: fcp_status = %d\n",
- io_req->fcp_status);
- break;
- }
+ sc_cmd = io_req->sc_cmd;
+ io_req->sc_cmd = NULL;
- sc_cmd = io_req->sc_cmd;
- io_req->sc_cmd = NULL;
+ bnx2fc_priv(sc_cmd)->io_req = NULL;
+ scsi_done(sc_cmd);
+ }
/* check if the io_req exists in tgt's tmf_q */
if (io_req->on_tmf_queue) {
@@ -1609,9 +1618,6 @@ void bnx2fc_process_tm_compl(struct bnx2fc_cmd *io_req,
return;
}
- bnx2fc_priv(sc_cmd)->io_req = NULL;
- scsi_done(sc_cmd);
-
kref_put(&io_req->refcount, bnx2fc_cmd_release);
if (io_req->wait_for_abts_comp) {
BNX2FC_IO_DBG(io_req, "tm_compl - wake up the waiter\n");
@@ -1740,15 +1746,9 @@ static void bnx2fc_unmap_sg_list(struct bnx2fc_cmd *io_req)
void bnx2fc_build_fcp_cmnd(struct bnx2fc_cmd *io_req,
struct fcp_cmnd *fcp_cmnd)
{
- struct scsi_cmnd *sc_cmd = io_req->sc_cmd;
-
memset(fcp_cmnd, 0, sizeof(struct fcp_cmnd));
- int_to_scsilun(sc_cmd->device->lun, &fcp_cmnd->fc_lun);
-
fcp_cmnd->fc_dl = htonl(io_req->data_xfer_len);
- memcpy(fcp_cmnd->fc_cdb, sc_cmd->cmnd, sc_cmd->cmd_len);
-
fcp_cmnd->fc_cmdref = 0;
fcp_cmnd->fc_pri_ta = 0;
fcp_cmnd->fc_tm_flags = io_req->mp_req.tm_flags;
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 08/20] ibmvfc: open-code reset loop for target reset
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (6 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 07/20] bnx2fc: Do not rely on a scsi command for lun or target reset Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-19 20:05 ` Ewan Milne
2022-05-12 11:12 ` [PATCH 09/20] ibmvfc: use fc_block_rport() Hannes Reinecke
` (11 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn, Tyrel Datwyler
From: Hannes Reinecke <hare@suse.com>
For target reset we need a device to send the target reset to,
so open-code the loop in target reset to send the target reset TMF
to the correct device.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 42 +++++++++++++++++++---------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index d0eab5700dc5..721d965f4b0e 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2925,18 +2925,6 @@ static void ibmvfc_dev_cancel_all_noreset(struct scsi_device *sdev, void *data)
*rc |= ibmvfc_cancel_all(sdev, IBMVFC_TMF_SUPPRESS_ABTS);
}
-/**
- * ibmvfc_dev_cancel_all_reset - Device iterated cancel all function
- * @sdev: scsi device struct
- * @data: return code
- *
- **/
-static void ibmvfc_dev_cancel_all_reset(struct scsi_device *sdev, void *data)
-{
- unsigned long *rc = data;
- *rc |= ibmvfc_cancel_all(sdev, IBMVFC_TMF_TGT_RESET);
-}
-
/**
* ibmvfc_eh_target_reset_handler - Reset the target
* @cmd: scsi command struct
@@ -2946,22 +2934,38 @@ static void ibmvfc_dev_cancel_all_reset(struct scsi_device *sdev, void *data)
**/
static int ibmvfc_eh_target_reset_handler(struct scsi_cmnd *cmd)
{
- struct scsi_device *sdev = cmd->device;
- struct ibmvfc_host *vhost = shost_priv(sdev->host);
- struct scsi_target *starget = scsi_target(sdev);
+ struct scsi_target *starget = scsi_target(cmd->device);
+ struct fc_rport *rport = starget_to_rport(starget);
+ struct Scsi_Host *shost = rport_to_shost(rport);
+ struct ibmvfc_host *vhost = shost_priv(shost);
int block_rc;
int reset_rc = 0;
int rc = FAILED;
unsigned long cancel_rc = 0;
+ bool tgt_reset = false;
ENTER;
- block_rc = fc_block_scsi_eh(cmd);
+ block_rc = fc_block_rport(rport);
ibmvfc_wait_while_resetting(vhost);
if (block_rc != FAST_IO_FAIL) {
- starget_for_each_device(starget, &cancel_rc, ibmvfc_dev_cancel_all_reset);
- reset_rc = ibmvfc_reset_device(sdev, IBMVFC_TARGET_RESET, "target");
+ struct scsi_device *sdev;
+
+ shost_for_each_device(sdev, shost) {
+ if ((sdev->channel != starget->channel) ||
+ (sdev->id != starget->id))
+ continue;
+
+ cancel_rc |= ibmvfc_cancel_all(sdev,
+ IBMVFC_TMF_TGT_RESET);
+ if (!tgt_reset) {
+ reset_rc = ibmvfc_reset_device(sdev,
+ IBMVFC_TARGET_RESET, "target");
+ tgt_reset = true;
+ }
+ }
} else
- starget_for_each_device(starget, &cancel_rc, ibmvfc_dev_cancel_all_noreset);
+ starget_for_each_device(starget, &cancel_rc,
+ ibmvfc_dev_cancel_all_noreset);
if (!cancel_rc && !reset_rc)
rc = ibmvfc_wait_for_ops(vhost, starget, ibmvfc_match_target);
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 08/20] ibmvfc: open-code reset loop for target reset
2022-05-12 11:12 ` [PATCH 08/20] ibmvfc: open-code reset loop for " Hannes Reinecke
@ 2022-05-19 20:05 ` Ewan Milne
2022-05-20 5:52 ` Hannes Reinecke
0 siblings, 1 reply; 34+ messages in thread
From: Ewan Milne @ 2022-05-19 20:05 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn, Tyrel Datwyler
This patch looks like it will call ibmvfc_reset_device() w/IBMVFC_TARGET_RESET
before it has called ibmvfc_cancel_all() on all the devices, the
existing code calls
ibmvfc_reset_device() w/IBMVFC_TARGET_RESET after the iterator.
Since you have the starget, why change to use shost_for_each_device()
and check the
sdev->channel and sdev->id ? That's what starget_for_each_device() does.
-Ewan
On Thu, May 12, 2022 at 7:13 AM Hannes Reinecke <hare@suse.de> wrote:
>
> From: Hannes Reinecke <hare@suse.com>
>
> For target reset we need a device to send the target reset to,
> so open-code the loop in target reset to send the target reset TMF
> to the correct device.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvfc.c | 42 +++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index d0eab5700dc5..721d965f4b0e 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -2925,18 +2925,6 @@ static void ibmvfc_dev_cancel_all_noreset(struct scsi_device *sdev, void *data)
> *rc |= ibmvfc_cancel_all(sdev, IBMVFC_TMF_SUPPRESS_ABTS);
> }
>
> -/**
> - * ibmvfc_dev_cancel_all_reset - Device iterated cancel all function
> - * @sdev: scsi device struct
> - * @data: return code
> - *
> - **/
> -static void ibmvfc_dev_cancel_all_reset(struct scsi_device *sdev, void *data)
> -{
> - unsigned long *rc = data;
> - *rc |= ibmvfc_cancel_all(sdev, IBMVFC_TMF_TGT_RESET);
> -}
> -
> /**
> * ibmvfc_eh_target_reset_handler - Reset the target
> * @cmd: scsi command struct
> @@ -2946,22 +2934,38 @@ static void ibmvfc_dev_cancel_all_reset(struct scsi_device *sdev, void *data)
> **/
> static int ibmvfc_eh_target_reset_handler(struct scsi_cmnd *cmd)
> {
> - struct scsi_device *sdev = cmd->device;
> - struct ibmvfc_host *vhost = shost_priv(sdev->host);
> - struct scsi_target *starget = scsi_target(sdev);
> + struct scsi_target *starget = scsi_target(cmd->device);
> + struct fc_rport *rport = starget_to_rport(starget);
> + struct Scsi_Host *shost = rport_to_shost(rport);
> + struct ibmvfc_host *vhost = shost_priv(shost);
> int block_rc;
> int reset_rc = 0;
> int rc = FAILED;
> unsigned long cancel_rc = 0;
> + bool tgt_reset = false;
>
> ENTER;
> - block_rc = fc_block_scsi_eh(cmd);
> + block_rc = fc_block_rport(rport);
> ibmvfc_wait_while_resetting(vhost);
> if (block_rc != FAST_IO_FAIL) {
> - starget_for_each_device(starget, &cancel_rc, ibmvfc_dev_cancel_all_reset);
> - reset_rc = ibmvfc_reset_device(sdev, IBMVFC_TARGET_RESET, "target");
> + struct scsi_device *sdev;
> +
> + shost_for_each_device(sdev, shost) {
> + if ((sdev->channel != starget->channel) ||
> + (sdev->id != starget->id))
> + continue;
> +
> + cancel_rc |= ibmvfc_cancel_all(sdev,
> + IBMVFC_TMF_TGT_RESET);
> + if (!tgt_reset) {
> + reset_rc = ibmvfc_reset_device(sdev,
> + IBMVFC_TARGET_RESET, "target");
> + tgt_reset = true;
> + }
> + }
> } else
> - starget_for_each_device(starget, &cancel_rc, ibmvfc_dev_cancel_all_noreset);
> + starget_for_each_device(starget, &cancel_rc,
> + ibmvfc_dev_cancel_all_noreset);
>
> if (!cancel_rc && !reset_rc)
> rc = ibmvfc_wait_for_ops(vhost, starget, ibmvfc_match_target);
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 08/20] ibmvfc: open-code reset loop for target reset
2022-05-19 20:05 ` Ewan Milne
@ 2022-05-20 5:52 ` Hannes Reinecke
0 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-20 5:52 UTC (permalink / raw)
To: Ewan Milne
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Johannes Thumshirn, Tyrel Datwyler
On 5/19/22 13:05, Ewan Milne wrote:
> This patch looks like it will call ibmvfc_reset_device() w/IBMVFC_TARGET_RESET
> before it has called ibmvfc_cancel_all() on all the devices, the
> existing code calls
> ibmvfc_reset_device() w/IBMVFC_TARGET_RESET after the iterator.
>
> Since you have the starget, why change to use shost_for_each_device()
> and check the
> sdev->channel and sdev->id ? That's what starget_for_each_device() does.
>
> -Ewan
>
> On Thu, May 12, 2022 at 7:13 AM Hannes Reinecke <hare@suse.de> wrote:
>>
>> From: Hannes Reinecke <hare@suse.com>
>>
>> For target reset we need a device to send the target reset to,
>> so open-code the loop in target reset to send the target reset TMF
>> to the correct device.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
>> ---
>> drivers/scsi/ibmvscsi/ibmvfc.c | 42 +++++++++++++++++++---------------
>> 1 file changed, 23 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
>> index d0eab5700dc5..721d965f4b0e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>> @@ -2925,18 +2925,6 @@ static void ibmvfc_dev_cancel_all_noreset(struct scsi_device *sdev, void *data)
>> *rc |= ibmvfc_cancel_all(sdev, IBMVFC_TMF_SUPPRESS_ABTS);
>> }
>>
>> -/**
>> - * ibmvfc_dev_cancel_all_reset - Device iterated cancel all function
>> - * @sdev: scsi device struct
>> - * @data: return code
>> - *
>> - **/
>> -static void ibmvfc_dev_cancel_all_reset(struct scsi_device *sdev, void *data)
>> -{
>> - unsigned long *rc = data;
>> - *rc |= ibmvfc_cancel_all(sdev, IBMVFC_TMF_TGT_RESET);
>> -}
>> -
>> /**
>> * ibmvfc_eh_target_reset_handler - Reset the target
>> * @cmd: scsi command struct
>> @@ -2946,22 +2934,38 @@ static void ibmvfc_dev_cancel_all_reset(struct scsi_device *sdev, void *data)
>> **/
>> static int ibmvfc_eh_target_reset_handler(struct scsi_cmnd *cmd)
>> {
>> - struct scsi_device *sdev = cmd->device;
>> - struct ibmvfc_host *vhost = shost_priv(sdev->host);
>> - struct scsi_target *starget = scsi_target(sdev);
>> + struct scsi_target *starget = scsi_target(cmd->device);
>> + struct fc_rport *rport = starget_to_rport(starget);
>> + struct Scsi_Host *shost = rport_to_shost(rport);
>> + struct ibmvfc_host *vhost = shost_priv(shost);
>> int block_rc;
>> int reset_rc = 0;
>> int rc = FAILED;
>> unsigned long cancel_rc = 0;
>> + bool tgt_reset = false;
>>
>> ENTER;
>> - block_rc = fc_block_scsi_eh(cmd);
>> + block_rc = fc_block_rport(rport);
>> ibmvfc_wait_while_resetting(vhost);
>> if (block_rc != FAST_IO_FAIL) {
>> - starget_for_each_device(starget, &cancel_rc, ibmvfc_dev_cancel_all_reset);
>> - reset_rc = ibmvfc_reset_device(sdev, IBMVFC_TARGET_RESET, "target");
>> + struct scsi_device *sdev;
>> +
>> + shost_for_each_device(sdev, shost) {
>> + if ((sdev->channel != starget->channel) ||
>> + (sdev->id != starget->id))
>> + continue;
>> +
>> + cancel_rc |= ibmvfc_cancel_all(sdev,
>> + IBMVFC_TMF_TGT_RESET);
>> + if (!tgt_reset) {
>> + reset_rc = ibmvfc_reset_device(sdev,
>> + IBMVFC_TARGET_RESET, "target");
>> + tgt_reset = true;
>> + }
>> + }
>> } else
>> - starget_for_each_device(starget, &cancel_rc, ibmvfc_dev_cancel_all_noreset);
>> + starget_for_each_device(starget, &cancel_rc,
>> + ibmvfc_dev_cancel_all_noreset);
>>
>> if (!cancel_rc && !reset_rc)
>> rc = ibmvfc_wait_for_ops(vhost, starget, ibmvfc_match_target);
>> --
>> 2.29.2
>>
>
You are right; the call to ibmvfc_reset_device() needs to be moved out
of the loop.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 09/20] ibmvfc: use fc_block_rport()
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (7 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 08/20] ibmvfc: open-code reset loop for " Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 10/20] fnic: use dedicated device reset command Hannes Reinecke
` (10 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Bart Van Assche, Johannes Thumshirn
Use fc_block_rport() instead of fc_block_scsi_eh() as the latter
will be deprecated.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 721d965f4b0e..2e7128f0d905 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2857,12 +2857,13 @@ static int ibmvfc_abort_task_set(struct scsi_device *sdev)
static int ibmvfc_eh_abort_handler(struct scsi_cmnd *cmd)
{
struct scsi_device *sdev = cmd->device;
+ struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
struct ibmvfc_host *vhost = shost_priv(sdev->host);
int cancel_rc, block_rc;
int rc = FAILED;
ENTER;
- block_rc = fc_block_scsi_eh(cmd);
+ block_rc = fc_block_rport(rport);
ibmvfc_wait_while_resetting(vhost);
if (block_rc != FAST_IO_FAIL) {
cancel_rc = ibmvfc_cancel_all(sdev, IBMVFC_TMF_ABORT_TASK_SET);
@@ -2890,12 +2891,13 @@ static int ibmvfc_eh_abort_handler(struct scsi_cmnd *cmd)
static int ibmvfc_eh_device_reset_handler(struct scsi_cmnd *cmd)
{
struct scsi_device *sdev = cmd->device;
+ struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
struct ibmvfc_host *vhost = shost_priv(sdev->host);
int cancel_rc, block_rc, reset_rc = 0;
int rc = FAILED;
ENTER;
- block_rc = fc_block_scsi_eh(cmd);
+ block_rc = fc_block_rport(rport);
ibmvfc_wait_while_resetting(vhost);
if (block_rc != FAST_IO_FAIL) {
cancel_rc = ibmvfc_cancel_all(sdev, IBMVFC_TMF_LUN_RESET);
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 10/20] fnic: use dedicated device reset command
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (8 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 09/20] ibmvfc: use fc_block_rport() Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 11/20] fnic: use fc_block_rport() correctly Hannes Reinecke
` (9 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
Use a dedicated command to send a device reset instead of relying
on using the command which triggered the device failure.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/fnic/fnic_main.c | 9 +++
drivers/scsi/fnic/fnic_scsi.c | 132 ++++++++++------------------------
2 files changed, 46 insertions(+), 95 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 9161bd2fd421..23a53aeb3ea3 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -831,6 +831,15 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_out_free_rq_buf;
}
+ /*
+ * Hack alert:
+ * Reduce can_queue by one after calling scsi_add_host()
+ * to reserve the topmost command for TMF.
+ * Should be replaced by using a reserved command once
+ * support for that gets merged.
+ */
+ lp->host->can_queue--;
+
/* Start local port initiatialization */
lp->link_up = 0;
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 3d64877bda8d..4f72fac583b7 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -1998,7 +1998,6 @@ static inline int fnic_queue_dr_io_req(struct fnic *fnic,
struct fnic_pending_aborts_iter_data {
struct fnic *fnic;
- struct scsi_cmnd *lr_sc;
struct scsi_device *lun_dev;
int ret;
};
@@ -2017,7 +2016,7 @@ static bool fnic_pending_aborts_iter(struct scsi_cmnd *sc,
DECLARE_COMPLETION_ONSTACK(tm_done);
enum fnic_ioreq_state old_ioreq_state;
- if (sc == iter_data->lr_sc || sc->device != lun_dev)
+ if (abt_tag == (fnic->fnic_max_tag_id - 1) || sc->device != lun_dev)
return true;
if (reserved)
return true;
@@ -2122,17 +2121,11 @@ static bool fnic_pending_aborts_iter(struct scsi_cmnd *sc,
return false;
}
fnic_priv(sc)->state = FNIC_IOREQ_ABTS_COMPLETE;
-
- /* original sc used for lr is handled by dev reset code */
- if (sc != iter_data->lr_sc)
- fnic_priv(sc)->io_req = NULL;
+ fnic_priv(sc)->io_req = NULL;
spin_unlock_irqrestore(io_lock, flags);
- /* original sc used for lr is handled by dev reset code */
- if (sc != iter_data->lr_sc) {
- fnic_release_ioreq_buf(fnic, io_req, sc);
- mempool_free(io_req, fnic->io_req_pool);
- }
+ fnic_release_ioreq_buf(fnic, io_req, sc);
+ mempool_free(io_req, fnic->io_req_pool);
/*
* Any IO is returned during reset, it needs to call scsi_done
@@ -2152,8 +2145,7 @@ static bool fnic_pending_aborts_iter(struct scsi_cmnd *sc,
* successfully aborted, 1 otherwise
*/
static int fnic_clean_pending_aborts(struct fnic *fnic,
- struct scsi_cmnd *lr_sc,
- bool new_sc)
+ struct scsi_cmnd *lr_sc)
{
int ret = SUCCESS;
@@ -2163,9 +2155,6 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
.ret = SUCCESS,
};
- if (new_sc)
- iter_data.lr_sc = lr_sc;
-
scsi_host_busy_iter(fnic->lport->host,
fnic_pending_aborts_iter, &iter_data);
if (iter_data.ret == FAILED) {
@@ -2182,39 +2171,6 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
return ret;
}
-/*
- * fnic_scsi_host_start_tag
- * Allocates tagid from host's tag list
- **/
-static inline int
-fnic_scsi_host_start_tag(struct fnic *fnic, struct scsi_cmnd *sc)
-{
- struct request *rq = scsi_cmd_to_rq(sc);
- struct request_queue *q = rq->q;
- struct request *dummy;
-
- dummy = blk_mq_alloc_request(q, REQ_OP_WRITE, BLK_MQ_REQ_NOWAIT);
- if (IS_ERR(dummy))
- return SCSI_NO_TAG;
-
- rq->tag = dummy->tag;
- sc->host_scribble = (unsigned char *)dummy;
-
- return dummy->tag;
-}
-
-/*
- * fnic_scsi_host_end_tag
- * frees tag allocated by fnic_scsi_host_start_tag.
- **/
-static inline void
-fnic_scsi_host_end_tag(struct fnic *fnic, struct scsi_cmnd *sc)
-{
- struct request *dummy = (struct request *)sc->host_scribble;
-
- blk_mq_free_request(dummy);
-}
-
/*
* SCSI Eh thread issues a Lun Reset when one or more commands on a LUN
* fail to get aborted. It calls driver's eh_device_reset with a SCSI command
@@ -2222,7 +2178,7 @@ fnic_scsi_host_end_tag(struct fnic *fnic, struct scsi_cmnd *sc)
*/
int fnic_device_reset(struct scsi_cmnd *sc)
{
- struct request *rq = scsi_cmd_to_rq(sc);
+ struct scsi_device *sdev = sc->device;
struct fc_lport *lp;
struct fnic *fnic;
struct fnic_io_req *io_req = NULL;
@@ -2235,16 +2191,17 @@ int fnic_device_reset(struct scsi_cmnd *sc)
struct scsi_lun fc_lun;
struct fnic_stats *fnic_stats;
struct reset_stats *reset_stats;
- int tag = rq->tag;
+ int tag;
DECLARE_COMPLETION_ONSTACK(tm_done);
- int tag_gen_flag = 0; /*to track tags allocated by fnic driver*/
- bool new_sc = 0;
/* Wait for rport to unblock */
- fc_block_scsi_eh(sc);
+ rport = starget_to_rport(scsi_target(sdev));
+ ret = fc_block_rport(rport);
+ if (ret)
+ return ret;
/* Get local-port, check ready and link up */
- lp = shost_priv(sc->device->host);
+ lp = shost_priv(sdev->host);
fnic = lport_priv(lp);
fnic_stats = &fnic->fnic_stats;
@@ -2252,10 +2209,9 @@ int fnic_device_reset(struct scsi_cmnd *sc)
atomic64_inc(&reset_stats->device_resets);
- rport = starget_to_rport(scsi_target(sc->device));
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "Device reset called FCID 0x%x, LUN 0x%llx sc 0x%p\n",
- rport->port_id, sc->device->lun, sc);
+ "Device reset called FCID 0x%x, LUN 0x%llx\n",
+ rport->port_id, sdev->lun);
if (lp->state != LPORT_ST_READY || !(lp->link_up))
goto fnic_device_reset_end;
@@ -2266,39 +2222,29 @@ int fnic_device_reset(struct scsi_cmnd *sc)
goto fnic_device_reset_end;
}
- fnic_priv(sc)->flags = FNIC_DEVICE_RESET;
- /* Allocate tag if not present */
+ /* The last tag is reserved for device reset */
+ sc = scsi_host_find_tag(sdev->host, fnic->fnic_max_tag_id - 1);
+ if (unlikely(!sc))
+ goto fnic_device_reset_end;
- if (unlikely(tag < 0)) {
- /*
- * Really should fix the midlayer to pass in a proper
- * request for ioctls...
- */
- tag = fnic_scsi_host_start_tag(fnic, sc);
- if (unlikely(tag == SCSI_NO_TAG))
- goto fnic_device_reset_end;
- tag_gen_flag = 1;
- new_sc = 1;
- }
+ tag = scsi_cmd_to_rq(sc)->tag;
io_lock = fnic_io_lock_hash(fnic, sc);
spin_lock_irqsave(io_lock, flags);
io_req = fnic_priv(sc)->io_req;
+ if (io_req)
+ goto fnic_device_reset_end;
- /*
- * If there is a io_req attached to this command, then use it,
- * else allocate a new one.
- */
+ io_req = mempool_alloc(fnic->io_req_pool, GFP_ATOMIC);
if (!io_req) {
- io_req = mempool_alloc(fnic->io_req_pool, GFP_ATOMIC);
- if (!io_req) {
- spin_unlock_irqrestore(io_lock, flags);
- goto fnic_device_reset_end;
- }
- memset(io_req, 0, sizeof(*io_req));
- io_req->port_id = rport->port_id;
- fnic_priv(sc)->io_req = io_req;
+ spin_unlock_irqrestore(io_lock, flags);
+ goto fnic_device_reset_end;
}
+ memset(io_req, 0, sizeof(*io_req));
+ io_req->port_id = rport->port_id;
+ fnic_priv(sc)->io_req = io_req;
+
io_req->dr_done = &tm_done;
+ fnic_priv(sc)->flags = FNIC_DEVICE_RESET;
fnic_priv(sc)->state = FNIC_IOREQ_CMD_PENDING;
fnic_priv(sc)->lr_status = FCPIO_INVALID_CODE;
spin_unlock_irqrestore(io_lock, flags);
@@ -2332,7 +2278,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
if (!io_req) {
spin_unlock_irqrestore(io_lock, flags);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
- "io_req is null tag 0x%x sc 0x%p\n", tag, sc);
+ "io_req is null tag 0x%x\n", tag);
goto fnic_device_reset_end;
}
io_req->dr_done = NULL;
@@ -2375,7 +2321,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
spin_unlock_irqrestore(io_lock, flags);
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
"Abort and terminate issued on Device reset "
- "tag 0x%x sc 0x%p\n", tag, sc);
+ "tag 0x%x\n", tag);
break;
}
}
@@ -2413,7 +2359,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
* the lun reset cmd. If all cmds get cleaned, the lun reset
* succeeds
*/
- if (fnic_clean_pending_aborts(fnic, sc, new_sc)) {
+ if (fnic_clean_pending_aborts(fnic, sc)) {
spin_lock_irqsave(io_lock, flags);
io_req = fnic_priv(sc)->io_req;
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
@@ -2442,17 +2388,14 @@ int fnic_device_reset(struct scsi_cmnd *sc)
}
fnic_device_reset_end:
- FNIC_TRACE(fnic_device_reset, sc->device->host->host_no, rq->tag, sc,
+ FNIC_TRACE(fnic_device_reset, sc->device->host->host_no,
+ scsi_cmd_to_rq(sc)->tag, sc,
jiffies_to_msecs(jiffies - start_time),
0, ((u64)sc->cmnd[0] << 32 |
(u64)sc->cmnd[2] << 24 | (u64)sc->cmnd[3] << 16 |
(u64)sc->cmnd[4] << 8 | sc->cmnd[5]),
fnic_flags_and_state(sc));
- /* free tag if it is allocated */
- if (unlikely(tag_gen_flag))
- fnic_scsi_host_end_tag(fnic, sc);
-
FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
"Returning from device reset %s\n",
(ret == SUCCESS) ?
@@ -2675,6 +2618,7 @@ static bool fnic_abts_pending_iter(struct scsi_cmnd *sc, void *data,
{
struct fnic_pending_aborts_iter_data *iter_data = data;
struct fnic *fnic = iter_data->fnic;
+ int tag = scsi_cmd_to_rq(sc)->tag;
int cmd_state;
struct fnic_io_req *io_req;
spinlock_t *io_lock;
@@ -2684,7 +2628,7 @@ static bool fnic_abts_pending_iter(struct scsi_cmnd *sc, void *data,
* ignore this lun reset cmd or cmds that do not belong to
* this lun
*/
- if (iter_data->lr_sc && sc == iter_data->lr_sc)
+ if (tag == (fnic->fnic_max_tag_id - 1))
return true;
if (iter_data->lun_dev && sc->device != iter_data->lun_dev)
return true;
@@ -2728,10 +2672,8 @@ int fnic_is_abts_pending(struct fnic *fnic, struct scsi_cmnd *lr_sc)
.ret = 0,
};
- if (lr_sc) {
+ if (lr_sc)
iter_data.lun_dev = lr_sc->device;
- iter_data.lr_sc = lr_sc;
- }
/* walk again to check, if IOs are still pending in fw */
scsi_host_busy_iter(fnic->lport->host,
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 11/20] fnic: use fc_block_rport() correctly
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (9 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 10/20] fnic: use dedicated device reset command Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 12/20] aic7xxx: make BUILD_SCSIID() a function Hannes Reinecke
` (8 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
Use fc_block_rport() instead of fc_block_scsi_eh() and evaluate
the return value to avoid issuing TMF commands when the port
is still blocked.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/fnic/fnic_scsi.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 4f72fac583b7..155662b13c83 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -1733,9 +1733,6 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
unsigned long abt_issued_time;
DECLARE_COMPLETION_ONSTACK(tm_done);
- /* Wait for rport to unblock */
- fc_block_scsi_eh(sc);
-
/* Get local-port, check ready and link up */
lp = shost_priv(sc->device->host);
@@ -1745,6 +1742,10 @@ int fnic_abort_cmd(struct scsi_cmnd *sc)
term_stats = &fnic->fnic_stats.term_stats;
rport = starget_to_rport(scsi_target(sc->device));
+ ret = fc_block_rport(rport);
+ if (ret)
+ return ret;
+
FNIC_SCSI_DBG(KERN_DEBUG,
fnic->lport->host,
"Abort Cmd called FCID 0x%x, LUN 0x%llx TAG %x flags %x\n",
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 12/20] aic7xxx: make BUILD_SCSIID() a function
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (10 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 11/20] fnic: use fc_block_rport() correctly Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-16 5:49 ` Christoph Hellwig
2022-05-12 11:12 ` [PATCH 13/20] aic79xx: " Hannes Reinecke
` (7 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
kernel test robot, Johannes Thumshirn
Convert BUILD_SCSIID() into a function and add a scsi_device argument.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/aic7xxx/aic7xxx_osm.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c b/drivers/scsi/aic7xxx/aic7xxx_osm.c
index d3b1082654d5..15e53b33b955 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
@@ -798,11 +798,18 @@ struct scsi_host_template aic7xxx_driver_template = {
/**************************** Tasklet Handler *********************************/
-/******************************** Macros **************************************/
-#define BUILD_SCSIID(ahc, cmd) \
- ((((cmd)->device->id << TID_SHIFT) & TID) \
- | (((cmd)->device->channel == 0) ? (ahc)->our_id : (ahc)->our_id_b) \
- | (((cmd)->device->channel == 0) ? 0 : TWIN_CHNLB))
+
+static inline unsigned int ahc_build_scsiid(struct ahc_softc *ahc,
+ struct scsi_device *sdev)
+{
+ unsigned int scsiid = (sdev->id << TID_SHIFT) & TID;
+
+ if (sdev->channel == 0)
+ scsiid |= ahc->our_id;
+ else
+ scsiid |= ahc->our_id_b | TWIN_CHNLB;
+ return scsiid;
+}
/******************************** Bus DMA *************************************/
int
@@ -1457,7 +1464,7 @@ ahc_linux_run_command(struct ahc_softc *ahc, struct ahc_linux_device *dev,
* Fill out basics of the HSCB.
*/
hscb->control = 0;
- hscb->scsiid = BUILD_SCSIID(ahc, cmd);
+ hscb->scsiid = ahc_build_scsiid(ahc, cmd->device);
hscb->lun = cmd->device->lun;
mask = SCB_GET_TARGET_MASK(ahc, scb);
tinfo = ahc_fetch_transinfo(ahc, SCB_GET_CHANNEL(ahc, scb),
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 13/20] aic79xx: make BUILD_SCSIID() a function
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (11 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 12/20] aic7xxx: make BUILD_SCSIID() a function Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-16 5:49 ` Christoph Hellwig
2022-05-12 11:12 ` [PATCH 14/20] aic7xxx: do not reference scsi command when resetting device Hannes Reinecke
` (6 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
Convert BUILD_SCSIID() into a function and add a scsi_device argument.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/aic7xxx/aic79xx_osm.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 928099163f0f..8172ec43736c 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -541,8 +541,11 @@ ahd_linux_unmap_scb(struct ahd_softc *ahd, struct scb *scb)
}
/******************************** Macros **************************************/
-#define BUILD_SCSIID(ahd, cmd) \
- (((scmd_id(cmd) << TID_SHIFT) & TID) | (ahd)->our_id)
+static inline unsigned int ahd_build_scsiid(struct ahd_softc *ahd,
+ struct scsi_device *sdev)
+{
+ return ((sdev_id(sdev) << TID_SHIFT) & TID) | (ahd)->our_id;
+}
/*
* Return a string describing the driver.
@@ -818,7 +821,7 @@ ahd_linux_dev_reset(struct scsi_cmnd *cmd)
ahd_set_sense_residual(reset_scb, 0);
reset_scb->platform_data->xfer_len = 0;
reset_scb->hscb->control = 0;
- reset_scb->hscb->scsiid = BUILD_SCSIID(ahd,cmd);
+ reset_scb->hscb->scsiid = ahd_build_scsiid(ahd, cmd->device);
reset_scb->hscb->lun = cmd->device->lun;
reset_scb->hscb->cdb_len = 0;
reset_scb->hscb->task_management = SIU_TASKMGMT_LUN_RESET;
@@ -1577,7 +1580,7 @@ ahd_linux_run_command(struct ahd_softc *ahd, struct ahd_linux_device *dev,
* Fill out basics of the HSCB.
*/
hscb->control = 0;
- hscb->scsiid = BUILD_SCSIID(ahd, cmd);
+ hscb->scsiid = ahd_build_scsiid(ahd, cmd->device);
hscb->lun = cmd->device->lun;
scb->hscb->task_management = 0;
mask = SCB_GET_TARGET_MASK(ahd, scb);
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 14/20] aic7xxx: do not reference scsi command when resetting device
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (12 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 13/20] aic79xx: " Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 15/20] aic79xx: " Hannes Reinecke
` (5 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
When sending a device reset we should not take a reference to the
scsi command.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/aic7xxx/aic7xxx_osm.c | 108 +++++++++++++++--------------
1 file changed, 57 insertions(+), 51 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c b/drivers/scsi/aic7xxx/aic7xxx_osm.c
index 15e53b33b955..4ae0a1c4d374 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
@@ -366,7 +366,8 @@ static void ahc_linux_queue_cmd_complete(struct ahc_softc *ahc,
struct scsi_cmnd *cmd);
static void ahc_linux_freeze_simq(struct ahc_softc *ahc);
static void ahc_linux_release_simq(struct ahc_softc *ahc);
-static int ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag);
+static int ahc_linux_queue_recovery_cmd(struct scsi_device *sdev,
+ struct scsi_cmnd *cmd);
static void ahc_linux_initialize_scsi_bus(struct ahc_softc *ahc);
static u_int ahc_linux_user_tagdepth(struct ahc_softc *ahc,
struct ahc_devinfo *devinfo);
@@ -728,7 +729,7 @@ ahc_linux_abort(struct scsi_cmnd *cmd)
{
int error;
- error = ahc_linux_queue_recovery_cmd(cmd, SCB_ABORT);
+ error = ahc_linux_queue_recovery_cmd(cmd->device, cmd);
if (error != SUCCESS)
printk("aic7xxx_abort returns 0x%x\n", error);
return (error);
@@ -742,7 +743,7 @@ ahc_linux_dev_reset(struct scsi_cmnd *cmd)
{
int error;
- error = ahc_linux_queue_recovery_cmd(cmd, SCB_DEVICE_RESET);
+ error = ahc_linux_queue_recovery_cmd(cmd->device, NULL);
if (error != SUCCESS)
printk("aic7xxx_dev_reset returns 0x%x\n", error);
return (error);
@@ -2036,11 +2037,12 @@ ahc_linux_release_simq(struct ahc_softc *ahc)
}
static int
-ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
+ahc_linux_queue_recovery_cmd(struct scsi_device *sdev,
+ struct scsi_cmnd *cmd)
{
struct ahc_softc *ahc;
struct ahc_linux_device *dev;
- struct scb *pending_scb;
+ struct scb *pending_scb = NULL, *scb;
u_int saved_scbptr;
u_int active_scb_index;
u_int last_phase;
@@ -2053,18 +2055,19 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
int disconnected;
unsigned long flags;
- pending_scb = NULL;
paused = FALSE;
wait = FALSE;
- ahc = *(struct ahc_softc **)cmd->device->host->hostdata;
+ ahc = *(struct ahc_softc **)sdev->host->hostdata;
- scmd_printk(KERN_INFO, cmd, "Attempting to queue a%s message\n",
- flag == SCB_ABORT ? "n ABORT" : " TARGET RESET");
+ sdev_printk(KERN_INFO, sdev, "Attempting to queue a%s message\n",
+ cmd ? "n ABORT" : " TARGET RESET");
- printk("CDB:");
- for (cdb_byte = 0; cdb_byte < cmd->cmd_len; cdb_byte++)
- printk(" 0x%x", cmd->cmnd[cdb_byte]);
- printk("\n");
+ if (cmd) {
+ printk("CDB:");
+ for (cdb_byte = 0; cdb_byte < cmd->cmd_len; cdb_byte++)
+ printk(" 0x%x", cmd->cmnd[cdb_byte]);
+ printk("\n");
+ }
ahc_lock(ahc, &flags);
@@ -2075,7 +2078,7 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
* at all, and the system wanted us to just abort the
* command, return success.
*/
- dev = scsi_transport_device_data(cmd->device);
+ dev = scsi_transport_device_data(sdev);
if (dev == NULL) {
/*
@@ -2083,13 +2086,12 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
* so we must not still own the command.
*/
printk("%s:%d:%d:%d: Is not an active device\n",
- ahc_name(ahc), cmd->device->channel, cmd->device->id,
- (u8)cmd->device->lun);
+ ahc_name(ahc), sdev->channel, sdev->id, (u8)sdev->lun);
retval = SUCCESS;
goto no_cmd;
}
- if ((dev->flags & (AHC_DEV_Q_BASIC|AHC_DEV_Q_TAGGED)) == 0
+ if (cmd && (dev->flags & (AHC_DEV_Q_BASIC|AHC_DEV_Q_TAGGED)) == 0
&& ahc_search_untagged_queues(ahc, cmd, cmd->device->id,
cmd->device->channel + 'A',
(u8)cmd->device->lun,
@@ -2104,25 +2106,28 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
/*
* See if we can find a matching cmd in the pending list.
*/
- LIST_FOREACH(pending_scb, &ahc->pending_scbs, pending_links) {
- if (pending_scb->io_ctx == cmd)
- break;
- }
-
- if (pending_scb == NULL && flag == SCB_DEVICE_RESET) {
-
+ if (cmd) {
+ LIST_FOREACH(scb, &ahc->pending_scbs, pending_links) {
+ if (scb->io_ctx == cmd) {
+ pending_scb = scb;
+ break;
+ }
+ }
+ } else {
/* Any SCB for this device will do for a target reset */
- LIST_FOREACH(pending_scb, &ahc->pending_scbs, pending_links) {
- if (ahc_match_scb(ahc, pending_scb, scmd_id(cmd),
- scmd_channel(cmd) + 'A',
+ LIST_FOREACH(scb, &ahc->pending_scbs, pending_links) {
+ if (ahc_match_scb(ahc, scb, sdev->id,
+ sdev->channel + 'A',
CAM_LUN_WILDCARD,
- SCB_LIST_NULL, ROLE_INITIATOR))
+ SCB_LIST_NULL, ROLE_INITIATOR)) {
+ pending_scb = scb;
break;
+ }
}
}
if (pending_scb == NULL) {
- scmd_printk(KERN_INFO, cmd, "Command not found\n");
+ sdev_printk(KERN_INFO, sdev, "Command not found\n");
goto no_cmd;
}
@@ -2153,22 +2158,22 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
ahc_dump_card_state(ahc);
disconnected = TRUE;
- if (flag == SCB_ABORT) {
- if (ahc_search_qinfifo(ahc, cmd->device->id,
- cmd->device->channel + 'A',
- cmd->device->lun,
+ if (cmd) {
+ if (ahc_search_qinfifo(ahc, sdev->id,
+ sdev->channel + 'A',
+ sdev->lun,
pending_scb->hscb->tag,
ROLE_INITIATOR, CAM_REQ_ABORTED,
SEARCH_COMPLETE) > 0) {
printk("%s:%d:%d:%d: Cmd aborted from QINFIFO\n",
- ahc_name(ahc), cmd->device->channel,
- cmd->device->id, (u8)cmd->device->lun);
+ ahc_name(ahc), sdev->channel,
+ sdev->id, (u8)sdev->lun);
retval = SUCCESS;
goto done;
}
- } else if (ahc_search_qinfifo(ahc, cmd->device->id,
- cmd->device->channel + 'A',
- cmd->device->lun,
+ } else if (ahc_search_qinfifo(ahc, sdev->id,
+ sdev->channel + 'A',
+ sdev->lun,
pending_scb->hscb->tag,
ROLE_INITIATOR, /*status*/0,
SEARCH_COUNT) > 0) {
@@ -2181,7 +2186,7 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
bus_scb = ahc_lookup_scb(ahc, ahc_inb(ahc, SCB_TAG));
if (bus_scb == pending_scb)
disconnected = FALSE;
- else if (flag != SCB_ABORT
+ else if (!cmd
&& ahc_inb(ahc, SAVED_SCSIID) == pending_scb->hscb->scsiid
&& ahc_inb(ahc, SAVED_LUN) == SCB_GET_LUN(pending_scb))
disconnected = FALSE;
@@ -2201,18 +2206,18 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
saved_scsiid = ahc_inb(ahc, SAVED_SCSIID);
if (last_phase != P_BUSFREE
&& (pending_scb->hscb->tag == active_scb_index
- || (flag == SCB_DEVICE_RESET
- && SCSIID_TARGET(ahc, saved_scsiid) == scmd_id(cmd)))) {
+ || (!cmd && SCSIID_TARGET(ahc, saved_scsiid) == sdev->id))) {
/*
* We're active on the bus, so assert ATN
* and hope that the target responds.
*/
pending_scb = ahc_lookup_scb(ahc, active_scb_index);
- pending_scb->flags |= SCB_RECOVERY_SCB|flag;
+ pending_scb->flags |= SCB_RECOVERY_SCB;
+ pending_scb->flags |= cmd ? SCB_ABORT : SCB_DEVICE_RESET;
ahc_outb(ahc, MSG_OUT, HOST_MSG);
ahc_outb(ahc, SCSISIGO, last_phase|ATNO);
- scmd_printk(KERN_INFO, cmd, "Device is active, asserting ATN\n");
+ sdev_printk(KERN_INFO, sdev, "Device is active, asserting ATN\n");
wait = TRUE;
} else if (disconnected) {
@@ -2233,7 +2238,8 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
* an unsolicited reselection occurred.
*/
pending_scb->hscb->control |= MK_MESSAGE|DISCONNECTED;
- pending_scb->flags |= SCB_RECOVERY_SCB|flag;
+ pending_scb->flags |= SCB_RECOVERY_SCB;
+ pending_scb->flags |= cmd ? SCB_ABORT : SCB_DEVICE_RESET;
/*
* Remove any cached copy of this SCB in the
@@ -2242,9 +2248,9 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
* same element in the SCB, SCB_NEXT, for
* both the qinfifo and the disconnected list.
*/
- ahc_search_disc_list(ahc, cmd->device->id,
- cmd->device->channel + 'A',
- cmd->device->lun, pending_scb->hscb->tag,
+ ahc_search_disc_list(ahc, sdev->id,
+ sdev->channel + 'A',
+ sdev->lun, pending_scb->hscb->tag,
/*stop_on_first*/TRUE,
/*remove*/TRUE,
/*save_state*/FALSE);
@@ -2267,9 +2273,9 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
* so we are the next SCB for this target
* to run.
*/
- ahc_search_qinfifo(ahc, cmd->device->id,
- cmd->device->channel + 'A',
- cmd->device->lun, SCB_LIST_NULL,
+ ahc_search_qinfifo(ahc, sdev->id,
+ sdev->channel + 'A',
+ (u8)sdev->lun, SCB_LIST_NULL,
ROLE_INITIATOR, CAM_REQUEUE_REQ,
SEARCH_COMPLETE);
ahc_qinfifo_requeue_tail(ahc, pending_scb);
@@ -2278,7 +2284,7 @@ ahc_linux_queue_recovery_cmd(struct scsi_cmnd *cmd, scb_flag flag)
printk("Device is disconnected, re-queuing SCB\n");
wait = TRUE;
} else {
- scmd_printk(KERN_INFO, cmd, "Unable to deliver message\n");
+ sdev_printk(KERN_INFO, sdev, "Unable to deliver message\n");
retval = FAILED;
goto done;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 15/20] aic79xx: do not reference scsi command when resetting device
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (13 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 14/20] aic7xxx: do not reference scsi command when resetting device Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 16/20] snic: reserve tag for TMF Hannes Reinecke
` (4 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
When sending a device reset we should not take a reference to the
scsi command.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/aic7xxx/aic79xx_osm.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 8172ec43736c..109a61162fcc 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -536,8 +536,10 @@ ahd_linux_unmap_scb(struct ahd_softc *ahd, struct scb *scb)
struct scsi_cmnd *cmd;
cmd = scb->io_ctx;
- ahd_sync_sglist(ahd, scb, BUS_DMASYNC_POSTWRITE);
- scsi_dma_unmap(cmd);
+ if (cmd) {
+ ahd_sync_sglist(ahd, scb, BUS_DMASYNC_POSTWRITE);
+ scsi_dma_unmap(cmd);
+ }
}
/******************************** Macros **************************************/
@@ -814,7 +816,7 @@ ahd_linux_dev_reset(struct scsi_cmnd *cmd)
tinfo = ahd_fetch_transinfo(ahd, 'A', ahd->our_id,
cmd->device->id, &tstate);
- reset_scb->io_ctx = cmd;
+ reset_scb->io_ctx = NULL;
reset_scb->platform_data->dev = dev;
reset_scb->sg_count = 0;
ahd_set_residual(reset_scb, 0);
@@ -1769,9 +1771,16 @@ ahd_done(struct ahd_softc *ahd, struct scb *scb)
dev = scb->platform_data->dev;
dev->active--;
dev->openings++;
- if ((cmd->result & (CAM_DEV_QFRZN << 16)) != 0) {
- cmd->result &= ~(CAM_DEV_QFRZN << 16);
- dev->qfrozen--;
+ if (cmd) {
+ if ((cmd->result & (CAM_DEV_QFRZN << 16)) != 0) {
+ cmd->result &= ~(CAM_DEV_QFRZN << 16);
+ dev->qfrozen--;
+ }
+ } else if (scb->flags & SCB_DEVICE_RESET) {
+ if (ahd->platform_data->eh_done)
+ complete(ahd->platform_data->eh_done);
+ ahd_free_scb(ahd, scb);
+ return;
}
ahd_linux_unmap_scb(ahd, scb);
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 16/20] snic: reserve tag for TMF
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (14 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 15/20] aic79xx: " Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-13 8:03 ` John Garry
2022-05-12 11:12 ` [PATCH 17/20] snic: use dedicated device reset command Hannes Reinecke
` (3 subsequent siblings)
19 siblings, 1 reply; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
Rather than re-using the failed command the snic driver should
reserve one command for TMFs.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/snic/snic.h | 2 +-
drivers/scsi/snic/snic_main.c | 8 +++++++
drivers/scsi/snic/snic_scsi.c | 44 +++++++++++++++++------------------
3 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/snic/snic.h b/drivers/scsi/snic/snic.h
index 4ec7e30678e1..28059b66f191 100644
--- a/drivers/scsi/snic/snic.h
+++ b/drivers/scsi/snic/snic.h
@@ -380,7 +380,7 @@ int snic_queuecommand(struct Scsi_Host *, struct scsi_cmnd *);
int snic_abort_cmd(struct scsi_cmnd *);
int snic_device_reset(struct scsi_cmnd *);
int snic_host_reset(struct scsi_cmnd *);
-int snic_reset(struct Scsi_Host *, struct scsi_cmnd *);
+int snic_reset(struct Scsi_Host *);
void snic_shutdown_scsi_cleanup(struct snic *);
diff --git a/drivers/scsi/snic/snic_main.c b/drivers/scsi/snic/snic_main.c
index 29d56396058c..3375153dd636 100644
--- a/drivers/scsi/snic/snic_main.c
+++ b/drivers/scsi/snic/snic_main.c
@@ -663,6 +663,14 @@ snic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_get_conf;
}
+ /*
+ * Hack alert: reduce can_queue by one after scsi_add_host()
+ * had been called.
+ * This essentially reserves the topmost request for TMF.
+ * Should be replaced by reserved command
+ * once support is being added.
+ */
+ shost->can_queue--;
snic_set_state(snic, SNIC_ONLINE);
ret = snic_disc_start(snic);
diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index 5f17666f3e1d..e69bed855a39 100644
--- a/drivers/scsi/snic/snic_scsi.c
+++ b/drivers/scsi/snic/snic_scsi.c
@@ -1018,17 +1018,6 @@ snic_hba_reset_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
"reset_cmpl: type = %x, hdr_stat = %x, cmnd_id = %x, hid = %x, ctx = %lx\n",
typ, hdr_stat, cmnd_id, hid, ctx);
- /* spl case, host reset issued through ioctl */
- if (cmnd_id == SCSI_NO_TAG) {
- rqi = (struct snic_req_info *) ctx;
- SNIC_HOST_INFO(snic->shost,
- "reset_cmpl:Tag %d ctx %lx cmpl stat %s\n",
- cmnd_id, ctx, snic_io_status_to_str(hdr_stat));
- sc = rqi->sc;
-
- goto ioctl_hba_rst;
- }
-
if (cmnd_id >= snic->max_tag_id) {
SNIC_HOST_ERR(snic->shost,
"reset_cmpl: Tag 0x%x out of Range,HdrStat %s\n",
@@ -1039,7 +1028,6 @@ snic_hba_reset_cmpl_handler(struct snic *snic, struct snic_fw_req *fwreq)
}
sc = scsi_host_find_tag(snic->shost, cmnd_id);
-ioctl_hba_rst:
if (!sc) {
atomic64_inc(&snic->s_stats.io.sc_null);
SNIC_HOST_ERR(snic->shost,
@@ -1725,7 +1713,7 @@ snic_dr_clean_single_req(struct snic *snic,
{
struct snic_req_info *rqi = NULL;
struct snic_tgt *tgt = NULL;
- struct scsi_cmnd *sc = NULL;
+ struct scsi_cmnd *sc;
spinlock_t *io_lock = NULL;
u32 sv_state = 0, tmf = 0;
DECLARE_COMPLETION_ONSTACK(tm_done);
@@ -2238,13 +2226,6 @@ snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
goto hba_rst_end;
}
- if (snic_cmd_tag(sc) == SCSI_NO_TAG) {
- memset(scsi_cmd_priv(sc), 0,
- sizeof(struct snic_internal_io_state));
- SNIC_HOST_INFO(snic->shost, "issu_hr:Host reset thru ioctl.\n");
- rqi->sc = sc;
- }
-
req = rqi_to_req(rqi);
io_lock = snic_io_lock_hash(snic, sc);
@@ -2319,11 +2300,13 @@ snic_issue_hba_reset(struct snic *snic, struct scsi_cmnd *sc)
} /* end of snic_issue_hba_reset */
int
-snic_reset(struct Scsi_Host *shost, struct scsi_cmnd *sc)
+snic_reset(struct Scsi_Host *shost)
{
struct snic *snic = shost_priv(shost);
+ struct scsi_cmnd *sc = NULL;
enum snic_state sv_state;
unsigned long flags;
+ u32 start_time = jiffies;
int ret = FAILED;
/* Set snic state as SNIC_FWRESET*/
@@ -2348,6 +2331,19 @@ snic_reset(struct Scsi_Host *shost, struct scsi_cmnd *sc)
while (atomic_read(&snic->ios_inflight))
schedule_timeout(msecs_to_jiffies(1));
+ /* The topmost command is reserved for TMF */
+ sc = scsi_host_find_tag(shost, snic->max_tag_id - 1);
+ if (!sc) {
+ SNIC_HOST_ERR(shost,
+ "reset:Host Reset Failed to allocate sc.\n");
+ spin_lock_irqsave(&snic->snic_lock, flags);
+ snic_set_state(snic, sv_state);
+ spin_unlock_irqrestore(&snic->snic_lock, flags);
+ atomic64_inc(&snic->s_stats.reset.hba_reset_fail);
+ ret = FAILED;
+
+ goto reset_end;
+ }
ret = snic_issue_hba_reset(snic, sc);
if (ret) {
SNIC_HOST_ERR(shost,
@@ -2365,6 +2361,10 @@ snic_reset(struct Scsi_Host *shost, struct scsi_cmnd *sc)
ret = SUCCESS;
reset_end:
+ SNIC_TRC(shost->host_no, sc ? snic_cmd_tag(sc) : SCSI_NO_TAG,
+ (ulong) sc, jiffies_to_msecs(jiffies - start_time),
+ 0, 0, 0);
+
return ret;
} /* end of snic_reset */
@@ -2387,7 +2387,7 @@ snic_host_reset(struct scsi_cmnd *sc)
sc, sc->cmnd[0], scsi_cmd_to_rq(sc),
snic_cmd_tag(sc), CMD_FLAGS(sc));
- ret = snic_reset(shost, sc);
+ ret = snic_reset(shost);
SNIC_TRC(shost->host_no, snic_cmd_tag(sc), (ulong) sc,
jiffies_to_msecs(jiffies - start_time),
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 16/20] snic: reserve tag for TMF
2022-05-12 11:12 ` [PATCH 16/20] snic: reserve tag for TMF Hannes Reinecke
@ 2022-05-13 8:03 ` John Garry
0 siblings, 0 replies; 34+ messages in thread
From: John Garry @ 2022-05-13 8:03 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Johannes Thumshirn
On 12/05/2022 12:12, Hannes Reinecke wrote:
> Rather than re-using the failed command the snic driver should
> reserve one command for TMFs.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
> drivers/scsi/snic/snic.h | 2 +-
> drivers/scsi/snic/snic_main.c | 8 +++++++
> drivers/scsi/snic/snic_scsi.c | 44 +++++++++++++++++------------------
> 3 files changed, 31 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/scsi/snic/snic.h b/drivers/scsi/snic/snic.h
> index 4ec7e30678e1..28059b66f191 100644
> --- a/drivers/scsi/snic/snic.h
> +++ b/drivers/scsi/snic/snic.h
> @@ -380,7 +380,7 @@ int snic_queuecommand(struct Scsi_Host *, struct scsi_cmnd *);
> int snic_abort_cmd(struct scsi_cmnd *);
> int snic_device_reset(struct scsi_cmnd *);
> int snic_host_reset(struct scsi_cmnd *);
> -int snic_reset(struct Scsi_Host *, struct scsi_cmnd *);
> +int snic_reset(struct Scsi_Host *);
> void snic_shutdown_scsi_cleanup(struct snic *);
>
>
> diff --git a/drivers/scsi/snic/snic_main.c b/drivers/scsi/snic/snic_main.c
> index 29d56396058c..3375153dd636 100644
> --- a/drivers/scsi/snic/snic_main.c
> +++ b/drivers/scsi/snic/snic_main.c
> @@ -663,6 +663,14 @@ snic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_get_conf;
> }
>
Hi Hannes,
> + /*
> + * Hack alert: reduce can_queue by one after scsi_add_host()
> + * had been called.
I am not sure how this helps as I did not think that reducing
shost->can_queue after scsi_add_host() had any effect. Maybe pre-
6eb045e092ef it did.
As an alternative interim solution, maybe you could just allocate a
regular single scsi request for this TMF at init time.
> + * This essentially reserves the topmost request for TMF.
> + * Should be replaced by reserved command
> + * once support is being added.
> + */
> + shost->can_queue--;
> snic_set_state(snic, SNIC_ONLINE);
>
Thanks,
John
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 17/20] snic: use dedicated device reset command
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (15 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 16/20] snic: reserve tag for TMF Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 18/20] snic: Use scsi_host_busy_iter() to traverse commands Hannes Reinecke
` (2 subsequent siblings)
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Johannes Thumshirn
Use a dedicated command to send a device reset instead of relying
on using the command which triggered the device failure.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/snic/snic_scsi.c | 52 ++++++++++++++++-------------------
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index e69bed855a39..6570d5bc203f 100644
--- a/drivers/scsi/snic/snic_scsi.c
+++ b/drivers/scsi/snic/snic_scsi.c
@@ -2128,57 +2128,53 @@ snic_unlink_and_release_req(struct snic *snic, struct scsi_cmnd *sc, int flag)
int
snic_device_reset(struct scsi_cmnd *sc)
{
- struct Scsi_Host *shost = sc->device->host;
+ struct scsi_device *sdev = sc->device;
+ struct Scsi_Host *shost = sdev->host;
struct snic *snic = shost_priv(shost);
struct snic_req_info *rqi = NULL;
- int tag = snic_cmd_tag(sc);
int start_time = jiffies;
int ret = FAILED;
int dr_supp = 0;
- SNIC_SCSI_DBG(shost, "dev_reset:sc %p :0x%x :req = %p :tag = %d\n",
- sc, sc->cmnd[0], scsi_cmd_to_rq(sc),
- snic_cmd_tag(sc));
- dr_supp = snic_dev_reset_supported(sc->device);
+ SNIC_SCSI_DBG(shost, "dev_reset\n");
+ dr_supp = snic_dev_reset_supported(sdev);
if (!dr_supp) {
/* device reset op is not supported */
SNIC_HOST_INFO(shost, "LUN Reset Op not supported.\n");
- snic_unlink_and_release_req(snic, sc, SNIC_DEV_RST_NOTSUP);
-
goto dev_rst_end;
}
if (unlikely(snic_get_state(snic) != SNIC_ONLINE)) {
- snic_unlink_and_release_req(snic, sc, 0);
SNIC_HOST_ERR(shost, "Devrst: Parent Devs are not online.\n");
goto dev_rst_end;
}
- /* There is no tag when lun reset is issue through ioctl. */
- if (unlikely(tag <= SNIC_NO_TAG)) {
- SNIC_HOST_INFO(snic->shost,
- "Devrst: LUN Reset Recvd thru IOCTL.\n");
-
- rqi = snic_req_init(snic, 0);
- if (!rqi)
- goto dev_rst_end;
-
- memset(scsi_cmd_priv(sc), 0,
- sizeof(struct snic_internal_io_state));
- CMD_SP(sc) = (char *)rqi;
- CMD_FLAGS(sc) = SNIC_NO_FLAGS;
+ rqi = snic_req_init(snic, 0);
+ if (!rqi)
+ goto dev_rst_end;
- /* Add special tag for dr coming from user spc */
- rqi->tm_tag = SNIC_TAG_IOCTL_DEV_RST;
- rqi->sc = sc;
+ /* The last tag is reserved for TMF */
+ sc = scsi_host_find_tag(snic->shost, snic->max_tag_id - 1);
+ if (!sc || CMD_SP(sc)) {
+ SNIC_HOST_ERR(snic->shost,
+ "Devrst: TMF busy\n");
+ goto dev_rst_end;
}
+ memset(scsi_cmd_priv(sc), 0,
+ sizeof(struct snic_internal_io_state));
+ CMD_SP(sc) = (char *)rqi;
+ CMD_FLAGS(sc) = SNIC_NO_FLAGS;
+
+ /* Add special tag for dr coming from user spc */
+ rqi->tm_tag = SNIC_TAG_IOCTL_DEV_RST;
+ rqi->sc = sc;
ret = snic_send_dr_and_wait(snic, sc);
if (ret) {
SNIC_HOST_ERR(snic->shost,
"Devrst: IO w/ Tag %x Failed w/ err = %d\n",
- tag, ret);
+ snic_cmd_tag(sc), ret);
snic_unlink_and_release_req(snic, sc, 0);
@@ -2188,7 +2184,7 @@ snic_device_reset(struct scsi_cmnd *sc)
ret = snic_dr_finish(snic, sc);
dev_rst_end:
- SNIC_TRC(snic->shost->host_no, tag, (ulong) sc,
+ SNIC_TRC(snic->shost->host_no, snic_cmd_tag(sc), (ulong) sc,
jiffies_to_msecs(jiffies - start_time),
0, SNIC_TRC_CMD(sc), SNIC_TRC_CMD_STATE_FLAGS(sc));
@@ -2333,7 +2329,7 @@ snic_reset(struct Scsi_Host *shost)
/* The topmost command is reserved for TMF */
sc = scsi_host_find_tag(shost, snic->max_tag_id - 1);
- if (!sc) {
+ if (!sc || CMD_SP(sc)) {
SNIC_HOST_ERR(shost,
"reset:Host Reset Failed to allocate sc.\n");
spin_lock_irqsave(&snic->snic_lock, flags);
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 18/20] snic: Use scsi_host_busy_iter() to traverse commands
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (16 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 17/20] snic: use dedicated device reset command Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 19/20] ips: Do not try to abort command from host reset Hannes Reinecke
2022-05-12 11:12 ` [PATCH 20/20] megaraid: pass in NULL scb for " Hannes Reinecke
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Johannes Thumshirn
Use scsi_host_busy_iter() to traverse commands instead of hand-crafted
routines walking the command list.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/snic/snic_scsi.c | 177 ++++++++++++++++------------------
1 file changed, 84 insertions(+), 93 deletions(-)
diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index 6570d5bc203f..50175b8f97bd 100644
--- a/drivers/scsi/snic/snic_scsi.c
+++ b/drivers/scsi/snic/snic_scsi.c
@@ -77,7 +77,7 @@ static const char * const snic_io_status_str[] = {
[SNIC_STAT_FATAL_ERROR] = "SNIC_STAT_FATAL_ERROR",
};
-static void snic_scsi_cleanup(struct snic *, int);
+static void snic_scsi_cleanup(struct snic *);
const char *
snic_state_to_str(unsigned int state)
@@ -977,7 +977,7 @@ snic_hba_reset_scsi_cleanup(struct snic *snic, struct scsi_cmnd *sc)
long act_ios = 0, act_fwreqs = 0;
SNIC_SCSI_DBG(snic->shost, "HBA Reset scsi cleanup.\n");
- snic_scsi_cleanup(snic, snic_cmd_tag(sc));
+ snic_scsi_cleanup(snic);
/* Update stats on pending IOs */
act_ios = atomic64_read(&st->io.active);
@@ -2421,53 +2421,36 @@ snic_cmpl_pending_tmreq(struct snic *snic, struct scsi_cmnd *sc)
complete(rqi->abts_done);
}
-/*
- * snic_scsi_cleanup: Walks through tag map and releases the reqs
- */
-static void
-snic_scsi_cleanup(struct snic *snic, int ex_tag)
+static bool
+snic_scsi_cleanup_iter(struct scsi_cmnd *sc, void *data, bool reserved)
{
+ struct snic *snic = data;
struct snic_req_info *rqi = NULL;
- struct scsi_cmnd *sc = NULL;
spinlock_t *io_lock = NULL;
unsigned long flags;
- int tag;
+ int tag = scsi_cmd_to_rq(sc)->tag;
u64 st_time = 0;
SNIC_SCSI_DBG(snic->shost, "sc_clean: scsi cleanup.\n");
- for (tag = 0; tag < snic->max_tag_id; tag++) {
- /* Skip ex_tag */
- if (tag == ex_tag)
- continue;
-
- io_lock = snic_io_lock_tag(snic, tag);
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(snic->shost, tag);
- if (!sc) {
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
-
- if (unlikely(snic_tmreq_pending(sc))) {
- /*
- * When FW Completes reset w/o sending completions
- * for outstanding ios.
- */
- snic_cmpl_pending_tmreq(snic, sc);
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
+ io_lock = snic_io_lock_tag(snic, tag);
+ spin_lock_irqsave(io_lock, flags);
- rqi = (struct snic_req_info *) CMD_SP(sc);
- if (!rqi) {
- spin_unlock_irqrestore(io_lock, flags);
+ if (unlikely(snic_tmreq_pending(sc))) {
+ /*
+ * When FW Completes reset w/o sending completions
+ * for outstanding ios.
+ */
+ snic_cmpl_pending_tmreq(snic, sc);
+ spin_unlock_irqrestore(io_lock, flags);
- goto cleanup;
- }
+ return true;
+ }
+ rqi = (struct snic_req_info *) CMD_SP(sc);
+ if (!rqi)
+ spin_unlock_irqrestore(io_lock, flags);
+ else {
SNIC_SCSI_DBG(snic->shost,
"sc_clean: sc %p, rqi %p, tag %d flags 0x%llx\n",
sc, rqi, tag, CMD_FLAGS(sc));
@@ -2482,24 +2465,34 @@ snic_scsi_cleanup(struct snic *snic, int ex_tag)
rqi, CMD_FLAGS(sc));
snic_release_req_buf(snic, rqi, sc);
+ }
+ sc->result = DID_TRANSPORT_DISRUPTED << 16;
+ SNIC_HOST_INFO(snic->shost,
+ "sc_clean: DID_TRANSPORT_DISRUPTED for sc %p, Tag %d flags 0x%llx rqi %p duration %u msecs\n",
+ sc, tag, CMD_FLAGS(sc), rqi,
+ jiffies_to_msecs(jiffies - st_time));
-cleanup:
- sc->result = DID_TRANSPORT_DISRUPTED << 16;
- SNIC_HOST_INFO(snic->shost,
- "sc_clean: DID_TRANSPORT_DISRUPTED for sc %p, Tag %d flags 0x%llx rqi %p duration %u msecs\n",
- sc, scsi_cmd_to_rq(sc)->tag, CMD_FLAGS(sc), rqi,
- jiffies_to_msecs(jiffies - st_time));
+ /* Update IO stats */
+ snic_stats_update_io_cmpl(&snic->s_stats);
- /* Update IO stats */
- snic_stats_update_io_cmpl(&snic->s_stats);
+ SNIC_TRC(snic->shost->host_no, tag, (ulong) sc,
+ jiffies_to_msecs(jiffies - st_time), 0,
+ SNIC_TRC_CMD(sc),
+ SNIC_TRC_CMD_STATE_FLAGS(sc));
- SNIC_TRC(snic->shost->host_no, tag, (ulong) sc,
- jiffies_to_msecs(jiffies - st_time), 0,
- SNIC_TRC_CMD(sc),
- SNIC_TRC_CMD_STATE_FLAGS(sc));
+ scsi_done(sc);
+ return true;
+}
- scsi_done(sc);
- }
+/*
+ * snic_scsi_cleanup: Walks through tag map and releases the reqs
+ */
+static void
+snic_scsi_cleanup(struct snic *snic)
+{
+ SNIC_SCSI_DBG(snic->shost, "sc_clean: scsi cleanup\n");
+
+ scsi_host_busy_iter(snic->shost, snic_scsi_cleanup_iter, snic);
} /* end of snic_scsi_cleanup */
void
@@ -2507,7 +2500,7 @@ snic_shutdown_scsi_cleanup(struct snic *snic)
{
SNIC_HOST_INFO(snic->shost, "Shutdown time SCSI Cleanup.\n");
- snic_scsi_cleanup(snic, SCSI_NO_TAG);
+ snic_scsi_cleanup(snic);
} /* end of snic_shutdown_scsi_cleanup */
/*
@@ -2521,7 +2514,7 @@ snic_internal_abort_io(struct snic *snic, struct scsi_cmnd *sc, int tmf)
spinlock_t *io_lock = NULL;
unsigned long flags;
u32 sv_state = 0;
- int ret = 0;
+ int ret = FAILED;
io_lock = snic_io_lock_hash(snic, sc);
spin_lock_irqsave(io_lock, flags);
@@ -2596,6 +2589,35 @@ snic_internal_abort_io(struct snic *snic, struct scsi_cmnd *sc, int tmf)
return ret;
} /* end of snic_internal_abort_io */
+struct snic_tgt_scsi_abort_io_data {
+ struct snic *snic;
+ struct snic_tgt *tgt;
+ int tmf;
+ int abt_cnt;
+};
+
+static bool snic_tgt_scsi_abort_io_iter(struct scsi_cmnd *sc, void *data,
+ bool reserved)
+{
+ struct snic_tgt_scsi_abort_io_data *iter_data = data;
+ struct snic *snic = iter_data->snic;
+ struct snic_tgt *sc_tgt;
+ int ret;
+
+ sc_tgt = starget_to_tgt(scsi_target(sc->device));
+ if (sc_tgt != iter_data->tgt)
+ return true;
+
+ ret = snic_internal_abort_io(snic, sc, iter_data->tmf);
+ if (ret == SUCCESS)
+ iter_data->abt_cnt++;
+ else
+ SNIC_HOST_ERR(snic->shost,
+ "tgt_abt_io: Tag %x, Failed w err = %d\n",
+ scsi_cmd_to_rq(sc)->tag, ret);
+ return true;
+}
+
/*
* snic_tgt_scsi_abort_io : called by snic_tgt_del
*/
@@ -2603,11 +2625,9 @@ int
snic_tgt_scsi_abort_io(struct snic_tgt *tgt)
{
struct snic *snic = NULL;
- struct scsi_cmnd *sc = NULL;
- struct snic_tgt *sc_tgt = NULL;
- spinlock_t *io_lock = NULL;
- unsigned long flags;
- int ret = 0, tag, abt_cnt = 0, tmf = 0;
+ struct snic_tgt_scsi_abort_io_data data = {
+ .abt_cnt = 0,
+ };
if (!tgt)
return -1;
@@ -2615,44 +2635,15 @@ snic_tgt_scsi_abort_io(struct snic_tgt *tgt)
snic = shost_priv(snic_tgt_to_shost(tgt));
SNIC_SCSI_DBG(snic->shost, "tgt_abt_io: Cleaning Pending IOs.\n");
+ data.snic = snic;
if (tgt->tdata.typ == SNIC_TGT_DAS)
- tmf = SNIC_ITMF_ABTS_TASK;
+ data.tmf = SNIC_ITMF_ABTS_TASK;
else
- tmf = SNIC_ITMF_ABTS_TASK_TERM;
-
- for (tag = 0; tag < snic->max_tag_id; tag++) {
- io_lock = snic_io_lock_tag(snic, tag);
-
- spin_lock_irqsave(io_lock, flags);
- sc = scsi_host_find_tag(snic->shost, tag);
- if (!sc) {
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
+ data.tmf = SNIC_ITMF_ABTS_TASK_TERM;
- sc_tgt = starget_to_tgt(scsi_target(sc->device));
- if (sc_tgt != tgt) {
- spin_unlock_irqrestore(io_lock, flags);
-
- continue;
- }
- spin_unlock_irqrestore(io_lock, flags);
-
- ret = snic_internal_abort_io(snic, sc, tmf);
- if (ret < 0) {
- SNIC_HOST_ERR(snic->shost,
- "tgt_abt_io: Tag %x, Failed w err = %d\n",
- tag, ret);
-
- continue;
- }
-
- if (ret == SUCCESS)
- abt_cnt++;
- }
+ scsi_host_busy_iter(snic->shost, snic_tgt_scsi_abort_io_iter, &data);
- SNIC_SCSI_DBG(snic->shost, "tgt_abt_io: abt_cnt = %d\n", abt_cnt);
+ SNIC_SCSI_DBG(snic->shost, "tgt_abt_io: abt_cnt = %d\n", data.abt_cnt);
return 0;
} /* end of snic_tgt_scsi_abort_io */
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 19/20] ips: Do not try to abort command from host reset
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (17 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 18/20] snic: Use scsi_host_busy_iter() to traverse commands Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
2022-05-12 11:12 ` [PATCH 20/20] megaraid: pass in NULL scb for " Hannes Reinecke
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Johannes Thumshirn, Adaptec OEM Raid Solutions
The code for aborting an outstanding command is a copy of the
functionality from command abort. As we already have called this
function once we reach host reset there's no point in trying to
do so again.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Adaptec OEM Raid Solutions <aacraid@microsemi.com>
---
drivers/scsi/ips.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 16419aeec02d..d49c1d6df5d6 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -835,7 +835,6 @@ static int __ips_eh_reset(struct scsi_cmnd *SC)
int i;
ips_ha_t *ha;
ips_scb_t *scb;
- ips_copp_wait_item_t *item;
METHOD_TRACE("ips_eh_reset", 1);
@@ -860,23 +859,6 @@ static int __ips_eh_reset(struct scsi_cmnd *SC)
if (!ha->active)
return (FAILED);
- /* See if the command is on the copp queue */
- item = ha->copp_waitlist.head;
- while ((item) && (item->scsi_cmd != SC))
- item = item->next;
-
- if (item) {
- /* Found it */
- ips_removeq_copp(&ha->copp_waitlist, item);
- return (SUCCESS);
- }
-
- /* See if the command is on the wait queue */
- if (ips_removeq_wait(&ha->scb_waitlist, SC)) {
- /* command not sent yet */
- return (SUCCESS);
- }
-
/* An explanation for the casual observer: */
/* Part of the function of a RAID controller is automatic error */
/* detection and recovery. As such, the only problem that physically */
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 20/20] megaraid: pass in NULL scb for host reset
2022-05-12 11:12 [PATCHv3 00/20] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (18 preceding siblings ...)
2022-05-12 11:12 ` [PATCH 19/20] ips: Do not try to abort command from host reset Hannes Reinecke
@ 2022-05-12 11:12 ` Hannes Reinecke
19 siblings, 0 replies; 34+ messages in thread
From: Hannes Reinecke @ 2022-05-12 11:12 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Johannes Thumshirn
When calling a host reset we shouldn't rely on the command triggering
the reset, so allow megaraid_abort_and_reset() to be called with a
NULL scb.
And drop the pointless 'bus_reset' and 'target_reset' handlers, which
just call the same function as host_reset.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
drivers/scsi/megaraid.c | 42 ++++++++++++++++-------------------------
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index a5d8cee2d510..9484632ffed6 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -1897,7 +1897,7 @@ megaraid_reset(struct scsi_cmnd *cmd)
spin_lock_irq(&adapter->lock);
- rval = megaraid_abort_and_reset(adapter, cmd, SCB_RESET);
+ rval = megaraid_abort_and_reset(adapter, NULL, SCB_RESET);
/*
* This is required here to complete any completed requests
@@ -1936,7 +1936,7 @@ megaraid_abort_and_reset(adapter_t *adapter, struct scsi_cmnd *cmd, int aor)
scb = list_entry(pos, scb_t, list);
- if (scb->cmd == cmd) { /* Found command */
+ if (!cmd || scb->cmd == cmd) { /* Found command */
scb->state |= aor;
@@ -1955,31 +1955,23 @@ megaraid_abort_and_reset(adapter_t *adapter, struct scsi_cmnd *cmd, int aor)
return FAILED;
}
- else {
-
- /*
- * Not yet issued! Remove from the pending
- * list
- */
- dev_warn(&adapter->dev->dev,
- "%s-[%x], driver owner\n",
- (aor==SCB_ABORT) ? "ABORTING":"RESET",
- scb->idx);
-
- mega_free_scb(adapter, scb);
-
- if( aor == SCB_ABORT ) {
- cmd->result = (DID_ABORT << 16);
- }
- else {
- cmd->result = (DID_RESET << 16);
- }
+ /*
+ * Not yet issued! Remove from the pending
+ * list
+ */
+ dev_warn(&adapter->dev->dev,
+ "%s-[%x], driver owner\n",
+ (cmd) ? "ABORTING":"RESET",
+ scb->idx);
+ mega_free_scb(adapter, scb);
+ if (cmd) {
+ cmd->result = (DID_ABORT << 16);
list_add_tail(SCSI_LIST(cmd),
- &adapter->completed_list);
-
- return SUCCESS;
+ &adapter->completed_list);
}
+
+ return SUCCESS;
}
}
@@ -4113,8 +4105,6 @@ static struct scsi_host_template megaraid_template = {
.sg_tablesize = MAX_SGLIST,
.cmd_per_lun = DEF_CMD_PER_LUN,
.eh_abort_handler = megaraid_abort,
- .eh_device_reset_handler = megaraid_reset,
- .eh_bus_reset_handler = megaraid_reset,
.eh_host_reset_handler = megaraid_reset,
.no_write_same = 1,
.cmd_size = sizeof(struct megaraid_cmd_priv),
--
2.29.2
^ permalink raw reply related [flat|nested] 34+ messages in thread