* [PATCH 01/24] csiostor: use fc_block_rport()
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
@ 2022-05-02 21:37 ` Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
` (2 more replies)
2022-05-02 21:37 ` [PATCH 02/24] fc_fcp: " Hannes Reinecke
` (22 subsequent siblings)
23 siblings, 3 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:37 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
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>
---
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] 75+ messages in thread
* Re: [PATCH 01/24] csiostor: use fc_block_rport()
2022-05-02 21:37 ` [PATCH 01/24] csiostor: use fc_block_rport() Hannes Reinecke
@ 2022-05-03 14:06 ` Christoph Hellwig
2022-05-03 14:43 ` Johannes Thumshirn
2022-05-03 16:09 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:06 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley, linux-scsi
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 01/24] csiostor: use fc_block_rport()
2022-05-02 21:37 ` [PATCH 01/24] csiostor: use fc_block_rport() Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
@ 2022-05-03 14:43 ` Johannes Thumshirn
2022-05-03 16:09 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 14:43 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 01/24] csiostor: use fc_block_rport()
2022-05-02 21:37 ` [PATCH 01/24] csiostor: use fc_block_rport() Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
2022-05-03 14:43 ` Johannes Thumshirn
@ 2022-05-03 16:09 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Bart Van Assche @ 2022-05-03 16:09 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 5/2/22 14:37, Hannes Reinecke wrote:
> Use fc_block_rport() instead of fc_block_scsi_eh() as the SCSI command
> will be removed as argument for SCSI EH functions.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 02/24] fc_fcp: use fc_block_rport()
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
2022-05-02 21:37 ` [PATCH 01/24] csiostor: use fc_block_rport() Hannes Reinecke
@ 2022-05-02 21:37 ` Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
` (2 more replies)
2022-05-02 21:37 ` [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset Hannes Reinecke
` (21 subsequent siblings)
23 siblings, 3 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:37 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
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>
---
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] 75+ messages in thread
* Re: [PATCH 02/24] fc_fcp: use fc_block_rport()
2022-05-02 21:37 ` [PATCH 02/24] fc_fcp: " Hannes Reinecke
@ 2022-05-03 14:06 ` Christoph Hellwig
2022-05-03 14:44 ` Johannes Thumshirn
2022-05-03 16:09 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:06 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley, linux-scsi
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 02/24] fc_fcp: use fc_block_rport()
2022-05-02 21:37 ` [PATCH 02/24] fc_fcp: " Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
@ 2022-05-03 14:44 ` Johannes Thumshirn
2022-05-03 16:09 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 14:44 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 02/24] fc_fcp: use fc_block_rport()
2022-05-02 21:37 ` [PATCH 02/24] fc_fcp: " Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
2022-05-03 14:44 ` Johannes Thumshirn
@ 2022-05-03 16:09 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Bart Van Assche @ 2022-05-03 16:09 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 5/2/22 14:37, Hannes Reinecke wrote:
> Use fc_block_rport() instead of fc_block_scsi_eh() as the SCSI command
> will be removed as argument for SCSI EH functions.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
2022-05-02 21:37 ` [PATCH 01/24] csiostor: use fc_block_rport() Hannes Reinecke
2022-05-02 21:37 ` [PATCH 02/24] fc_fcp: " Hannes Reinecke
@ 2022-05-02 21:37 ` Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
2022-05-03 17:21 ` Steffen Maier
2022-05-02 21:38 ` [PATCH 04/24] mptfc: simplify mpt_fc_block_error_handler() Hannes Reinecke
` (20 subsequent siblings)
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:37 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Steffen Maier, Benjamin Block
When issuing a host reset we should be waiting for all
ports to become unblocked; just waiting for one might
be resulting in host reset to return too early.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Cc: Steffen Maier <maier@linux.ibm.com>
Cc: Benjamin Block <bblock@linux.ibm.com>
---
drivers/s390/scsi/zfcp_scsi.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 526ac240d9fe..fb2b73fca381 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -373,9 +373,11 @@ static int zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)
static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
{
- struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
- struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
- int ret = SUCCESS, fc_ret;
+ struct Scsi_Host *host = scpnt->device->host;
+ struct zfcp_adapter *adapter = (struct zfcp_adapter *)host->hostdata[0];
+ int ret = SUCCESS;
+ unsigned long flags;
+ struct zfcp_port *port;
if (!(adapter->connection_features & FSF_FEATURE_NPIV_MODE)) {
zfcp_erp_port_forced_reopen_all(adapter, 0, "schrh_p");
@@ -383,9 +385,24 @@ static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
}
zfcp_erp_adapter_reopen(adapter, 0, "schrh_1");
zfcp_erp_wait(adapter);
- fc_ret = fc_block_scsi_eh(scpnt);
- if (fc_ret)
- ret = fc_ret;
+retry_rport_blocked:
+ spin_lock_irqsave(host->host_lock, flags);
+ list_for_each_entry(port, &adapter->port_list, list) {
+ struct fc_rport *rport = port->rport;
+
+ if (rport->port_state == FC_PORTSTATE_BLOCKED) {
+ if (rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT)
+ ret = FAST_IO_FAIL;
+ else
+ ret = NEEDS_RETRY;
+ break;
+ }
+ }
+ spin_unlock_irqrestore(host->host_lock, flags);
+ if (ret == NEEDS_RETRY) {
+ msleep(1000);
+ goto retry_rport_blocked;
+ }
zfcp_dbf_scsi_eh("schrh_r", adapter, ~0, ret);
return ret;
--
2.29.2
^ permalink raw reply related [flat|nested] 75+ messages in thread
* Re: [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset
2022-05-02 21:37 ` [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset Hannes Reinecke
@ 2022-05-03 14:06 ` Christoph Hellwig
2022-05-03 17:21 ` Steffen Maier
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:06 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Steffen Maier, Benjamin Block
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset
2022-05-02 21:37 ` [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset Hannes Reinecke
2022-05-03 14:06 ` Christoph Hellwig
@ 2022-05-03 17:21 ` Steffen Maier
2022-05-03 18:42 ` Hannes Reinecke
1 sibling, 1 reply; 75+ messages in thread
From: Steffen Maier @ 2022-05-03 17:21 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Benjamin Block
Hi Hannes,
On 5/2/22 23:37, Hannes Reinecke wrote:
> When issuing a host reset we should be waiting for all
> ports to become unblocked; just waiting for one might
> be resulting in host reset to return too early.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Cc: Steffen Maier <maier@linux.ibm.com>
> Cc: Benjamin Block <bblock@linux.ibm.com>
> ---
> drivers/s390/scsi/zfcp_scsi.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
> index 526ac240d9fe..fb2b73fca381 100644
> --- a/drivers/s390/scsi/zfcp_scsi.c
> +++ b/drivers/s390/scsi/zfcp_scsi.c
> @@ -373,9 +373,11 @@ static int zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)
>
> static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
> {
> - struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
> - struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
> - int ret = SUCCESS, fc_ret;
> + struct Scsi_Host *host = scpnt->device->host;
> + struct zfcp_adapter *adapter = (struct zfcp_adapter *)host->hostdata[0];
> + int ret = SUCCESS;
> + unsigned long flags;
> + struct zfcp_port *port;
>
> if (!(adapter->connection_features & FSF_FEATURE_NPIV_MODE)) {
> zfcp_erp_port_forced_reopen_all(adapter, 0, "schrh_p");
> @@ -383,9 +385,24 @@ static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
> }
> zfcp_erp_adapter_reopen(adapter, 0, "schrh_1");
> zfcp_erp_wait(adapter);
> - fc_ret = fc_block_scsi_eh(scpnt);
> - if (fc_ret)
> - ret = fc_ret;
> +retry_rport_blocked:
> + spin_lock_irqsave(host->host_lock, flags);
> + list_for_each_entry(port, &adapter->port_list, list) {
Reading adapter->port_list needs to be protected by
read_lock_irq(&adapter->port_list_lock);
Cf. Benjamin's last review
https://lore.kernel.org/linux-scsi/YRujHScPbb1Aokrj@t480-pf1aa2c2.linux.ibm.com/
> + struct fc_rport *rport = port->rport;
While an rport is blocked, port->rport == NULL [zfcp_scsi_rport_block() and
zfcp_scsi_rport_register()], so below could be a NULL pointer deref.
Or is there evidence we would never have a blocked rport while iterating
port_list here?
See also my previous review comments
https://lore.kernel.org/linux-scsi/a7950ea7-380c-bb01-7f31-5c555217ad2d@linux.vnet.ibm.com/
It also alludes to lock ordering.
> +
> + if (rport->port_state == FC_PORTSTATE_BLOCKED) {
> + if (rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT)
> + ret = FAST_IO_FAIL;
> + else
> + ret = NEEDS_RETRY;
> + break;
> + }
> + }
> + spin_unlock_irqrestore(host->host_lock, flags);
> + if (ret == NEEDS_RETRY) {
> + msleep(1000);
> + goto retry_rport_blocked;
> + }
>
> zfcp_dbf_scsi_eh("schrh_r", adapter, ~0, ret);
> return ret;
--
Mit freundlichen Gruessen / Kind regards
Steffen Maier
Linux on IBM Z and LinuxONE
https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschaeftsfuehrung: David Faller
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset
2022-05-03 17:21 ` Steffen Maier
@ 2022-05-03 18:42 ` Hannes Reinecke
0 siblings, 0 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-03 18:42 UTC (permalink / raw)
To: Steffen Maier, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Benjamin Block
On 5/3/22 10:21, Steffen Maier wrote:
> Hi Hannes,
>
> On 5/2/22 23:37, Hannes Reinecke wrote:
>> When issuing a host reset we should be waiting for all
>> ports to become unblocked; just waiting for one might
>> be resulting in host reset to return too early.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> Cc: Steffen Maier <maier@linux.ibm.com>
>> Cc: Benjamin Block <bblock@linux.ibm.com>
>> ---
>> drivers/s390/scsi/zfcp_scsi.c | 29 +++++++++++++++++++++++------
>> 1 file changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/s390/scsi/zfcp_scsi.c
>> b/drivers/s390/scsi/zfcp_scsi.c
>> index 526ac240d9fe..fb2b73fca381 100644
>> --- a/drivers/s390/scsi/zfcp_scsi.c
>> +++ b/drivers/s390/scsi/zfcp_scsi.c
>> @@ -373,9 +373,11 @@ static int
>> zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)
>> static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
>> {
>> - struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
>> - struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
>> - int ret = SUCCESS, fc_ret;
>> + struct Scsi_Host *host = scpnt->device->host;
>> + struct zfcp_adapter *adapter = (struct zfcp_adapter
>> *)host->hostdata[0];
>> + int ret = SUCCESS;
>> + unsigned long flags;
>> + struct zfcp_port *port;
>> if (!(adapter->connection_features & FSF_FEATURE_NPIV_MODE)) {
>> zfcp_erp_port_forced_reopen_all(adapter, 0, "schrh_p");
>> @@ -383,9 +385,24 @@ static int zfcp_scsi_eh_host_reset_handler(struct
>> scsi_cmnd *scpnt)
>> }
>> zfcp_erp_adapter_reopen(adapter, 0, "schrh_1");
>> zfcp_erp_wait(adapter);
>> - fc_ret = fc_block_scsi_eh(scpnt);
>> - if (fc_ret)
>> - ret = fc_ret;
>> +retry_rport_blocked:
>> + spin_lock_irqsave(host->host_lock, flags);
>> + list_for_each_entry(port, &adapter->port_list, list) {
>
> Reading adapter->port_list needs to be protected by
> read_lock_irq(&adapter->port_list_lock);
>
> Cf. Benjamin's last review
> https://lore.kernel.org/linux-scsi/YRujHScPbb1Aokrj@t480-pf1aa2c2.linux.ibm.com/
>
>
Ah. Sorry. Will be including it in the next round.
>> + struct fc_rport *rport = port->rport;
>
> While an rport is blocked, port->rport == NULL [zfcp_scsi_rport_block()
> and zfcp_scsi_rport_register()], so below could be a NULL pointer deref.
> Or is there evidence we would never have a blocked rport while iterating
> port_list here?
>
> See also my previous review comments
> https://lore.kernel.org/linux-scsi/a7950ea7-380c-bb01-7f31-5c555217ad2d@linux.vnet.ibm.com/
>
> It also alludes to lock ordering.
>
Right. Will be folding in the changes.
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: Felix Imendörffer
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 04/24] mptfc: simplify mpt_fc_block_error_handler()
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (2 preceding siblings ...)
2022-05-02 21:37 ` [PATCH 03/24] zfcp: open-code fc_block_scsi_eh() for host reset Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:07 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 05/24] mptfusion: correct definitions for mptscsih_dev_reset() Hannes Reinecke
` (19 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
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>
---
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] 75+ messages in thread
* [PATCH 05/24] mptfusion: correct definitions for mptscsih_dev_reset()
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (3 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 04/24] mptfc: simplify mpt_fc_block_error_handler() Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:07 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 06/24] mptfc: open-code mptfc_block_error_handler() for bus reset Hannes Reinecke
` (18 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
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>
---
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] 75+ messages in thread
* [PATCH 06/24] mptfc: open-code mptfc_block_error_handler() for bus reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (4 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 05/24] mptfusion: correct definitions for mptscsih_dev_reset() Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:08 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 07/24] qedf: use fc rport as argument for qedf_initiate_tmf() Hannes Reinecke
` (17 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
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>
---
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] 75+ messages in thread
* [PATCH 07/24] qedf: use fc rport as argument for qedf_initiate_tmf()
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (5 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 06/24] mptfc: open-code mptfc_block_error_handler() for bus reset Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:08 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 08/24] bnx2fc: Do not rely on a scsi command for lun or target reset Hannes Reinecke
` (16 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, 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>
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] 75+ messages in thread
* Re: [PATCH 07/24] qedf: use fc rport as argument for qedf_initiate_tmf()
2022-05-02 21:38 ` [PATCH 07/24] qedf: use fc rport as argument for qedf_initiate_tmf() Hannes Reinecke
@ 2022-05-03 14:08 ` Christoph Hellwig
0 siblings, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:08 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Saurav Kashyap
On Mon, May 02, 2022 at 11:38:03PM +0200, Hannes Reinecke 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().
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 08/24] bnx2fc: Do not rely on a scsi command for lun or target reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (6 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 07/24] qedf: use fc rport as argument for qedf_initiate_tmf() Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:08 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 09/24] ibmvfc: open-code reset loop for " Hannes Reinecke
` (15 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, 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>
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] 75+ messages in thread
* [PATCH 09/24] ibmvfc: open-code reset loop for target reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (7 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 08/24] bnx2fc: Do not rely on a scsi command for lun or target reset Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:09 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 10/24] ibmvfc: use fc_block_rport() Hannes Reinecke
` (14 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
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>
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] 75+ messages in thread
* [PATCH 10/24] ibmvfc: use fc_block_rport()
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (8 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 09/24] ibmvfc: open-code reset loop for " Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:09 ` Christoph Hellwig
` (2 more replies)
2022-05-02 21:38 ` [PATCH 11/24] fnic: use dedicated device reset command Hannes Reinecke
` (13 subsequent siblings)
23 siblings, 3 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
Use fc_block_rport() instead of fc_block_scsi_eh() as the latter
will be deprecated.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
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] 75+ messages in thread
* Re: [PATCH 10/24] ibmvfc: use fc_block_rport()
2022-05-02 21:38 ` [PATCH 10/24] ibmvfc: use fc_block_rport() Hannes Reinecke
@ 2022-05-03 14:09 ` Christoph Hellwig
2022-05-03 15:20 ` Johannes Thumshirn
2022-05-03 16:11 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:09 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley, linux-scsi
On Mon, May 02, 2022 at 11:38:06PM +0200, Hannes Reinecke wrote:
> Use fc_block_rport() instead of fc_block_scsi_eh() as the latter
> will be deprecated.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 10/24] ibmvfc: use fc_block_rport()
2022-05-02 21:38 ` [PATCH 10/24] ibmvfc: use fc_block_rport() Hannes Reinecke
2022-05-03 14:09 ` Christoph Hellwig
@ 2022-05-03 15:20 ` Johannes Thumshirn
2022-05-03 16:11 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 15:20 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 10/24] ibmvfc: use fc_block_rport()
2022-05-02 21:38 ` [PATCH 10/24] ibmvfc: use fc_block_rport() Hannes Reinecke
2022-05-03 14:09 ` Christoph Hellwig
2022-05-03 15:20 ` Johannes Thumshirn
@ 2022-05-03 16:11 ` Bart Van Assche
2 siblings, 0 replies; 75+ messages in thread
From: Bart Van Assche @ 2022-05-03 16:11 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
On 5/2/22 14:38, Hannes Reinecke wrote:
> Use fc_block_rport() instead of fc_block_scsi_eh() as the latter
> will be deprecated.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 11/24] fnic: use dedicated device reset command
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (9 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 10/24] ibmvfc: use fc_block_rport() Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:09 ` Christoph Hellwig
2022-05-03 17:12 ` kernel test robot
2022-05-02 21:38 ` [PATCH 12/24] fnic: use fc_block_rport() correctly Hannes Reinecke
` (12 subsequent siblings)
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
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>
---
drivers/scsi/fnic/fnic_scsi.c | 131 ++++++++++------------------------
1 file changed, 36 insertions(+), 95 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 3d64877bda8d..b2ba731b67de 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,28 @@ 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;
- }
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 +2277,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 +2320,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 +2358,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 +2387,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 +2617,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 +2627,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 +2671,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] 75+ messages in thread
* Re: [PATCH 11/24] fnic: use dedicated device reset command
2022-05-02 21:38 ` [PATCH 11/24] fnic: use dedicated device reset command Hannes Reinecke
@ 2022-05-03 14:09 ` Christoph Hellwig
2022-05-03 17:12 ` kernel test robot
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:09 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 11/24] fnic: use dedicated device reset command
2022-05-02 21:38 ` [PATCH 11/24] fnic: use dedicated device reset command Hannes Reinecke
2022-05-03 14:09 ` Christoph Hellwig
@ 2022-05-03 17:12 ` kernel test robot
1 sibling, 0 replies; 75+ messages in thread
From: kernel test robot @ 2022-05-03 17:12 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: llvm, kbuild-all
Hi Hannes,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next powerpc/next linus/master v5.18-rc5 next-20220503]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Hannes-Reinecke/scsi-EH-rework-prep-patches-part-1/20220503-054317
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20220504/202205040107.yOpQQQnY-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 363b3a645a1e30011cc8da624f13dac5fd915628)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/8691055a002b89e1dac5e431f039c5f3c6e8be1f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Hannes-Reinecke/scsi-EH-rework-prep-patches-part-1/20220503-054317
git checkout 8691055a002b89e1dac5e431f039c5f3c6e8be1f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/scsi/fnic/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/scsi/fnic/fnic_scsi.c:2310:5: warning: variable 'tag' is uninitialized when used here [-Wuninitialized]
tag | FNIC_TAG_DEV_RST,
^~~
drivers/scsi/fnic/fnic_scsi.c:2194:9: note: initialize the variable 'tag' to silence this warning
int tag;
^
= 0
1 warning generated.
vim +/tag +2310 drivers/scsi/fnic/fnic_scsi.c
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2173
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2174 /*
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2175 * SCSI Eh thread issues a Lun Reset when one or more commands on a LUN
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2176 * fail to get aborted. It calls driver's eh_device_reset with a SCSI command
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2177 * on the LUN.
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2178 */
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2179 int fnic_device_reset(struct scsi_cmnd *sc)
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2180 {
8691055a002b89 Hannes Reinecke 2022-05-02 2181 struct scsi_device *sdev = sc->device;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2182 struct fc_lport *lp;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2183 struct fnic *fnic;
4d7007b49d523d Hiral Patel 2013-02-12 2184 struct fnic_io_req *io_req = NULL;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2185 struct fc_rport *rport;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2186 int status;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2187 int ret = FAILED;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2188 spinlock_t *io_lock;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2189 unsigned long flags;
14eb5d905d16ec Hiral Patel 2013-02-12 2190 unsigned long start_time = 0;
03298552cba38f Hiral Patel 2013-02-12 2191 struct scsi_lun fc_lun;
67125b0287a9e6 Hiral Patel 2013-09-12 2192 struct fnic_stats *fnic_stats;
67125b0287a9e6 Hiral Patel 2013-09-12 2193 struct reset_stats *reset_stats;
8691055a002b89 Hannes Reinecke 2022-05-02 2194 int tag;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2195 DECLARE_COMPLETION_ONSTACK(tm_done);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2196
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2197 /* Wait for rport to unblock */
8691055a002b89 Hannes Reinecke 2022-05-02 2198 rport = starget_to_rport(scsi_target(sdev));
8691055a002b89 Hannes Reinecke 2022-05-02 2199 ret = fc_block_rport(rport);
8691055a002b89 Hannes Reinecke 2022-05-02 2200 if (ret)
8691055a002b89 Hannes Reinecke 2022-05-02 2201 return ret;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2202
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2203 /* Get local-port, check ready and link up */
8691055a002b89 Hannes Reinecke 2022-05-02 2204 lp = shost_priv(sdev->host);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2205
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2206 fnic = lport_priv(lp);
67125b0287a9e6 Hiral Patel 2013-09-12 2207 fnic_stats = &fnic->fnic_stats;
67125b0287a9e6 Hiral Patel 2013-09-12 2208 reset_stats = &fnic->fnic_stats.reset_stats;
67125b0287a9e6 Hiral Patel 2013-09-12 2209
67125b0287a9e6 Hiral Patel 2013-09-12 2210 atomic64_inc(&reset_stats->device_resets);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2211
0db6f4353d68c0 Roel Kluin 2010-06-11 2212 FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
8691055a002b89 Hannes Reinecke 2022-05-02 2213 "Device reset called FCID 0x%x, LUN 0x%llx\n",
8691055a002b89 Hannes Reinecke 2022-05-02 2214 rport->port_id, sdev->lun);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2215
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2216 if (lp->state != LPORT_ST_READY || !(lp->link_up))
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2217 goto fnic_device_reset_end;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2218
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2219 /* Check if remote port up */
67125b0287a9e6 Hiral Patel 2013-09-12 2220 if (fc_remote_port_chkready(rport)) {
67125b0287a9e6 Hiral Patel 2013-09-12 2221 atomic64_inc(&fnic_stats->misc_stats.rport_not_ready);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2222 goto fnic_device_reset_end;
67125b0287a9e6 Hiral Patel 2013-09-12 2223 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2224
8691055a002b89 Hannes Reinecke 2022-05-02 2225 /* The last tag is reserved for device reset */
8691055a002b89 Hannes Reinecke 2022-05-02 2226 sc = scsi_host_find_tag(sdev->host, fnic->fnic_max_tag_id - 1);
8691055a002b89 Hannes Reinecke 2022-05-02 2227 if (unlikely(!sc))
03298552cba38f Hiral Patel 2013-02-12 2228 goto fnic_device_reset_end;
8691055a002b89 Hannes Reinecke 2022-05-02 2229
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2230 io_lock = fnic_io_lock_hash(fnic, sc);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2231 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2232 io_req = fnic_priv(sc)->io_req;
8691055a002b89 Hannes Reinecke 2022-05-02 2233 if (io_req)
8691055a002b89 Hannes Reinecke 2022-05-02 2234 goto fnic_device_reset_end;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2235
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2236 io_req = mempool_alloc(fnic->io_req_pool, GFP_ATOMIC);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2237 if (!io_req) {
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2238 spin_unlock_irqrestore(io_lock, flags);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2239 goto fnic_device_reset_end;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2240 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2241 memset(io_req, 0, sizeof(*io_req));
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2242 io_req->port_id = rport->port_id;
924cb24df4fc4d Bart Van Assche 2022-02-18 2243 fnic_priv(sc)->io_req = io_req;
8691055a002b89 Hannes Reinecke 2022-05-02 2244
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2245 io_req->dr_done = &tm_done;
8691055a002b89 Hannes Reinecke 2022-05-02 2246 fnic_priv(sc)->flags = FNIC_DEVICE_RESET;
924cb24df4fc4d Bart Van Assche 2022-02-18 2247 fnic_priv(sc)->state = FNIC_IOREQ_CMD_PENDING;
924cb24df4fc4d Bart Van Assche 2022-02-18 2248 fnic_priv(sc)->lr_status = FCPIO_INVALID_CODE;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2249 spin_unlock_irqrestore(io_lock, flags);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2250
03298552cba38f Hiral Patel 2013-02-12 2251 FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host, "TAG %x\n", tag);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2252
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2253 /*
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2254 * issue the device reset, if enqueue failed, clean up the ioreq
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2255 * and break assoc with scsi cmd
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2256 */
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2257 if (fnic_queue_dr_io_req(fnic, sc, io_req)) {
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2258 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2259 io_req = fnic_priv(sc)->io_req;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2260 if (io_req)
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2261 io_req->dr_done = NULL;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2262 goto fnic_device_reset_clean;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2263 }
03298552cba38f Hiral Patel 2013-02-12 2264 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2265 fnic_priv(sc)->flags |= FNIC_DEV_RST_ISSUED;
03298552cba38f Hiral Patel 2013-02-12 2266 spin_unlock_irqrestore(io_lock, flags);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2267
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2268 /*
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2269 * Wait on the local completion for LUN reset. The io_req may be
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2270 * freed while we wait since we hold no lock.
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2271 */
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2272 wait_for_completion_timeout(&tm_done,
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2273 msecs_to_jiffies(FNIC_LUN_RESET_TIMEOUT));
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2274
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2275 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2276 io_req = fnic_priv(sc)->io_req;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2277 if (!io_req) {
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2278 spin_unlock_irqrestore(io_lock, flags);
03298552cba38f Hiral Patel 2013-02-12 2279 FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
8691055a002b89 Hannes Reinecke 2022-05-02 2280 "io_req is null tag 0x%x\n", tag);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2281 goto fnic_device_reset_end;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2282 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2283 io_req->dr_done = NULL;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2284
924cb24df4fc4d Bart Van Assche 2022-02-18 2285 status = fnic_priv(sc)->lr_status;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2286
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2287 /*
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2288 * If lun reset not completed, bail out with failed. io_req
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2289 * gets cleaned up during higher levels of EH
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2290 */
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2291 if (status == FCPIO_INVALID_CODE) {
67125b0287a9e6 Hiral Patel 2013-09-12 2292 atomic64_inc(&reset_stats->device_reset_timeouts);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2293 FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2294 "Device reset timed out\n");
924cb24df4fc4d Bart Van Assche 2022-02-18 2295 fnic_priv(sc)->flags |= FNIC_DEV_RST_TIMED_OUT;
03298552cba38f Hiral Patel 2013-02-12 2296 spin_unlock_irqrestore(io_lock, flags);
03298552cba38f Hiral Patel 2013-02-12 2297 int_to_scsilun(sc->device->lun, &fc_lun);
03298552cba38f Hiral Patel 2013-02-12 2298 /*
1259c5dc752474 Sesidhar Beddel 2013-09-09 2299 * Issue abort and terminate on device reset request.
1259c5dc752474 Sesidhar Beddel 2013-09-09 2300 * If q'ing of terminate fails, retry it after a delay.
03298552cba38f Hiral Patel 2013-02-12 2301 */
03298552cba38f Hiral Patel 2013-02-12 2302 while (1) {
03298552cba38f Hiral Patel 2013-02-12 2303 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2304 if (fnic_priv(sc)->flags & FNIC_DEV_RST_TERM_ISSUED) {
03298552cba38f Hiral Patel 2013-02-12 2305 spin_unlock_irqrestore(io_lock, flags);
03298552cba38f Hiral Patel 2013-02-12 2306 break;
03298552cba38f Hiral Patel 2013-02-12 2307 }
03298552cba38f Hiral Patel 2013-02-12 2308 spin_unlock_irqrestore(io_lock, flags);
03298552cba38f Hiral Patel 2013-02-12 2309 if (fnic_queue_abort_io_req(fnic,
03298552cba38f Hiral Patel 2013-02-12 @2310 tag | FNIC_TAG_DEV_RST,
03298552cba38f Hiral Patel 2013-02-12 2311 FCPIO_ITMF_ABT_TASK_TERM,
03298552cba38f Hiral Patel 2013-02-12 2312 fc_lun.scsi_lun, io_req)) {
03298552cba38f Hiral Patel 2013-02-12 2313 wait_for_completion_timeout(&tm_done,
03298552cba38f Hiral Patel 2013-02-12 2314 msecs_to_jiffies(FNIC_ABT_TERM_DELAY_TIMEOUT));
03298552cba38f Hiral Patel 2013-02-12 2315 } else {
03298552cba38f Hiral Patel 2013-02-12 2316 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2317 fnic_priv(sc)->flags |= FNIC_DEV_RST_TERM_ISSUED;
924cb24df4fc4d Bart Van Assche 2022-02-18 2318 fnic_priv(sc)->state = FNIC_IOREQ_ABTS_PENDING;
03298552cba38f Hiral Patel 2013-02-12 2319 io_req->abts_done = &tm_done;
03298552cba38f Hiral Patel 2013-02-12 2320 spin_unlock_irqrestore(io_lock, flags);
03298552cba38f Hiral Patel 2013-02-12 2321 FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
03298552cba38f Hiral Patel 2013-02-12 2322 "Abort and terminate issued on Device reset "
8691055a002b89 Hannes Reinecke 2022-05-02 2323 "tag 0x%x\n", tag);
03298552cba38f Hiral Patel 2013-02-12 2324 break;
03298552cba38f Hiral Patel 2013-02-12 2325 }
03298552cba38f Hiral Patel 2013-02-12 2326 }
03298552cba38f Hiral Patel 2013-02-12 2327 while (1) {
03298552cba38f Hiral Patel 2013-02-12 2328 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2329 if (!(fnic_priv(sc)->flags & FNIC_DEV_RST_DONE)) {
03298552cba38f Hiral Patel 2013-02-12 2330 spin_unlock_irqrestore(io_lock, flags);
03298552cba38f Hiral Patel 2013-02-12 2331 wait_for_completion_timeout(&tm_done,
03298552cba38f Hiral Patel 2013-02-12 2332 msecs_to_jiffies(FNIC_LUN_RESET_TIMEOUT));
03298552cba38f Hiral Patel 2013-02-12 2333 break;
03298552cba38f Hiral Patel 2013-02-12 2334 } else {
924cb24df4fc4d Bart Van Assche 2022-02-18 2335 io_req = fnic_priv(sc)->io_req;
03298552cba38f Hiral Patel 2013-02-12 2336 io_req->abts_done = NULL;
03298552cba38f Hiral Patel 2013-02-12 2337 goto fnic_device_reset_clean;
03298552cba38f Hiral Patel 2013-02-12 2338 }
03298552cba38f Hiral Patel 2013-02-12 2339 }
03298552cba38f Hiral Patel 2013-02-12 2340 } else {
03298552cba38f Hiral Patel 2013-02-12 2341 spin_unlock_irqrestore(io_lock, flags);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2342 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2343
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2344 /* Completed, but not successful, clean up the io_req, return fail */
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2345 if (status != FCPIO_SUCCESS) {
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2346 spin_lock_irqsave(io_lock, flags);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2347 FNIC_SCSI_DBG(KERN_DEBUG,
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2348 fnic->lport->host,
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2349 "Device reset completed - failed\n");
924cb24df4fc4d Bart Van Assche 2022-02-18 2350 io_req = fnic_priv(sc)->io_req;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2351 goto fnic_device_reset_clean;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2352 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2353
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2354 /*
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2355 * Clean up any aborts on this lun that have still not
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2356 * completed. If any of these fail, then LUN reset fails.
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2357 * clean_pending_aborts cleans all cmds on this lun except
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2358 * the lun reset cmd. If all cmds get cleaned, the lun reset
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2359 * succeeds
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2360 */
8691055a002b89 Hannes Reinecke 2022-05-02 2361 if (fnic_clean_pending_aborts(fnic, sc)) {
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2362 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2363 io_req = fnic_priv(sc)->io_req;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2364 FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2365 "Device reset failed"
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2366 " since could not abort all IOs\n");
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2367 goto fnic_device_reset_clean;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2368 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2369
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2370 /* Clean lun reset command */
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2371 spin_lock_irqsave(io_lock, flags);
924cb24df4fc4d Bart Van Assche 2022-02-18 2372 io_req = fnic_priv(sc)->io_req;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2373 if (io_req)
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2374 /* Completed, and successful */
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2375 ret = SUCCESS;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2376
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2377 fnic_device_reset_clean:
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2378 if (io_req)
924cb24df4fc4d Bart Van Assche 2022-02-18 2379 fnic_priv(sc)->io_req = NULL;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2380
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2381 spin_unlock_irqrestore(io_lock, flags);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2382
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2383 if (io_req) {
14eb5d905d16ec Hiral Patel 2013-02-12 2384 start_time = io_req->start_time;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2385 fnic_release_ioreq_buf(fnic, io_req, sc);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2386 mempool_free(io_req, fnic->io_req_pool);
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2387 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2388
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2389 fnic_device_reset_end:
8691055a002b89 Hannes Reinecke 2022-05-02 2390 FNIC_TRACE(fnic_device_reset, sc->device->host->host_no,
8691055a002b89 Hannes Reinecke 2022-05-02 2391 scsi_cmd_to_rq(sc)->tag, sc,
4d7007b49d523d Hiral Patel 2013-02-12 2392 jiffies_to_msecs(jiffies - start_time),
4d7007b49d523d Hiral Patel 2013-02-12 2393 0, ((u64)sc->cmnd[0] << 32 |
4d7007b49d523d Hiral Patel 2013-02-12 2394 (u64)sc->cmnd[2] << 24 | (u64)sc->cmnd[3] << 16 |
4d7007b49d523d Hiral Patel 2013-02-12 2395 (u64)sc->cmnd[4] << 8 | sc->cmnd[5]),
924cb24df4fc4d Bart Van Assche 2022-02-18 2396 fnic_flags_and_state(sc));
4d7007b49d523d Hiral Patel 2013-02-12 2397
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2398 FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2399 "Returning from device reset %s\n",
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2400 (ret == SUCCESS) ?
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2401 "SUCCESS" : "FAILED");
67125b0287a9e6 Hiral Patel 2013-09-12 2402
67125b0287a9e6 Hiral Patel 2013-09-12 2403 if (ret == FAILED)
67125b0287a9e6 Hiral Patel 2013-09-12 2404 atomic64_inc(&reset_stats->device_reset_failures);
67125b0287a9e6 Hiral Patel 2013-09-12 2405
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2406 return ret;
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2407 }
5df6d737dd4b0f Abhijeet Joglekar 2009-04-17 2408
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 12/24] fnic: use fc_block_rport() correctly
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (10 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 11/24] fnic: use dedicated device reset command Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:10 ` Christoph Hellwig
2022-05-03 15:22 ` Johannes Thumshirn
2022-05-02 21:38 ` [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function Hannes Reinecke
` (11 subsequent siblings)
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
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>
---
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 b2ba731b67de..fb6dc59a5a7e 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] 75+ messages in thread
* Re: [PATCH 12/24] fnic: use fc_block_rport() correctly
2022-05-02 21:38 ` [PATCH 12/24] fnic: use fc_block_rport() correctly Hannes Reinecke
@ 2022-05-03 14:10 ` Christoph Hellwig
2022-05-03 15:22 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:10 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley, linux-scsi
On Mon, May 02, 2022 at 11:38:08PM +0200, Hannes Reinecke wrote:
> 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.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 12/24] fnic: use fc_block_rport() correctly
2022-05-02 21:38 ` [PATCH 12/24] fnic: use fc_block_rport() correctly Hannes Reinecke
2022-05-03 14:10 ` Christoph Hellwig
@ 2022-05-03 15:22 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 15:22 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (11 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 12/24] fnic: use fc_block_rport() correctly Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 9:39 ` kernel test robot
2022-05-03 14:12 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 14/24] aic79xx: " Hannes Reinecke
` (10 subsequent siblings)
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
Convert BUILD_SCSIID() into a function and add a scsi_device argument.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/aic7xxx/aic7xxx_osm.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c b/drivers/scsi/aic7xxx/aic7xxx_osm.c
index d3b1082654d5..85fe8808399f 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
@@ -798,11 +798,16 @@ 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) |
+ ((sdev->channel == 0) ? ahc->our_id : ahc->our_id_b) |
+ (sdev->channel == 0) ? 0 : TWIN_CHNLB;
+ return scsiid;
+}
/******************************** Bus DMA *************************************/
int
@@ -1457,7 +1462,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] 75+ messages in thread
* Re: [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function
2022-05-02 21:38 ` [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function Hannes Reinecke
@ 2022-05-03 9:39 ` kernel test robot
2022-05-03 14:12 ` Christoph Hellwig
1 sibling, 0 replies; 75+ messages in thread
From: kernel test robot @ 2022-05-03 9:39 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: llvm, kbuild-all, Christoph Hellwig, James Bottomley, linux-scsi,
Hannes Reinecke
Hi Hannes,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next powerpc/next linus/master v5.18-rc5 next-20220502]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Hannes-Reinecke/scsi-EH-rework-prep-patches-part-1/20220503-054317
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: arm64-randconfig-r022-20220502 (https://download.01.org/0day-ci/archive/20220503/202205031715.Eiw7By3c-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 363b3a645a1e30011cc8da624f13dac5fd915628)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/5436bc0a0e277a5742af0b7b443b4591ce0e5b74
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Hannes-Reinecke/scsi-EH-rework-prep-patches-part-1/20220503-054317
git checkout 5436bc0a0e277a5742af0b7b443b4591ce0e5b74
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/scsi/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/scsi/aic7xxx/aic7xxx_osm.c:808:24: warning: operator '?:' has lower precedence than '|'; '|' will be evaluated first [-Wbitwise-conditional-parentheses]
(sdev->channel == 0) ? 0 : TWIN_CHNLB;
~~~~~~~~~~~~~~~~~~~~ ^
drivers/scsi/aic7xxx/aic7xxx_osm.c:808:24: note: place parentheses around the '|' expression to silence this warning
(sdev->channel == 0) ? 0 : TWIN_CHNLB;
^
)
drivers/scsi/aic7xxx/aic7xxx_osm.c:808:24: note: place parentheses around the '?:' expression to evaluate it first
(sdev->channel == 0) ? 0 : TWIN_CHNLB;
~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
1 warning generated.
vim +808 drivers/scsi/aic7xxx/aic7xxx_osm.c
800
801
802 static inline unsigned int ahc_build_scsiid(struct ahc_softc *ahc,
803 struct scsi_device *sdev)
804 {
805 unsigned int scsiid =
806 ((sdev->id << TID_SHIFT) & TID) |
807 ((sdev->channel == 0) ? ahc->our_id : ahc->our_id_b) |
> 808 (sdev->channel == 0) ? 0 : TWIN_CHNLB;
809 return scsiid;
810 }
811
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function
2022-05-02 21:38 ` [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function Hannes Reinecke
2022-05-03 9:39 ` kernel test robot
@ 2022-05-03 14:12 ` Christoph Hellwig
2022-05-03 14:18 ` Hannes Reinecke
1 sibling, 1 reply; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:12 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke
> +{
> + unsigned int scsiid =
> + ((sdev->id << TID_SHIFT) & TID) |
> + ((sdev->channel == 0) ? ahc->our_id : ahc->our_id_b) |
> + (sdev->channel == 0) ? 0 : TWIN_CHNLB;
> + return scsiid;
This still look weird. Why not:
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;
?
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function
2022-05-03 14:12 ` Christoph Hellwig
@ 2022-05-03 14:18 ` Hannes Reinecke
0 siblings, 0 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-03 14:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, James Bottomley, linux-scsi, Hannes Reinecke
On 5/3/22 07:12, Christoph Hellwig wrote:
>> +{
>> + unsigned int scsiid =
>> + ((sdev->id << TID_SHIFT) & TID) |
>> + ((sdev->channel == 0) ? ahc->our_id : ahc->our_id_b) |
>> + (sdev->channel == 0) ? 0 : TWIN_CHNLB;
>> + return scsiid;
>
> This still look weird. Why not:
>
> 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;
>
> ?
Yeah, kbuild had been complaining, too.
Will be fixing it up.
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: Felix Imendörffer
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 14/24] aic79xx: make BUILD_SCSIID() a function
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (12 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 13/24] aic7xxx: make BUILD_SCSIID() a function Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:13 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 15/24] aic7xxx: do not reference scsi command when resetting device Hannes Reinecke
` (9 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
Convert BUILD_SCSIID() into a function and add a scsi_device argument.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/aic7xxx/aic79xx_osm.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 928099163f0f..b9b214ef5a2a 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -541,8 +541,14 @@ 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)
+{
+ unsigned int scsiid =
+ ((sdev_id(sdev) << TID_SHIFT) & TID) | (ahd)->our_id;
+
+ return scsiid;
+}
/*
* Return a string describing the driver.
@@ -818,7 +824,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 +1583,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] 75+ messages in thread
* Re: [PATCH 14/24] aic79xx: make BUILD_SCSIID() a function
2022-05-02 21:38 ` [PATCH 14/24] aic79xx: " Hannes Reinecke
@ 2022-05-03 14:13 ` Christoph Hellwig
2022-05-03 14:19 ` Hannes Reinecke
0 siblings, 1 reply; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:13 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke
On Mon, May 02, 2022 at 11:38:10PM +0200, Hannes Reinecke wrote:
> +{
> + unsigned int scsiid =
> + ((sdev_id(sdev) << TID_SHIFT) & TID) | (ahd)->our_id;
> +
> + return scsiid;
return ((sdev_id(sdev) << TID_SHIFT) & TID) | ahd->our_id;
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 14/24] aic79xx: make BUILD_SCSIID() a function
2022-05-03 14:13 ` Christoph Hellwig
@ 2022-05-03 14:19 ` Hannes Reinecke
0 siblings, 0 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-03 14:19 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, James Bottomley, linux-scsi, Hannes Reinecke
On 5/3/22 07:13, Christoph Hellwig wrote:
> On Mon, May 02, 2022 at 11:38:10PM +0200, Hannes Reinecke wrote:
>> +{
>> + unsigned int scsiid =
>> + ((sdev_id(sdev) << TID_SHIFT) & TID) | (ahd)->our_id;
>> +
>> + return scsiid;
>
> return ((sdev_id(sdev) << TID_SHIFT) & TID) | ahd->our_id;
Yep.
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: Felix Imendörffer
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 15/24] aic7xxx: do not reference scsi command when resetting device
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (13 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 14/24] aic79xx: " Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:13 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 16/24] aic79xx: " Hannes Reinecke
` (8 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
When sending a device reset we should not take a reference to the
scsi command.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/aic7xxx/aic7xxx_osm.c | 102 +++++++++++++++--------------
1 file changed, 54 insertions(+), 48 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c b/drivers/scsi/aic7xxx/aic7xxx_osm.c
index 85fe8808399f..0ad429b7ebcf 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);
@@ -2034,11 +2035,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;
@@ -2051,18 +2053,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);
@@ -2073,7 +2076,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) {
/*
@@ -2081,13 +2084,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,
@@ -2102,25 +2104,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)
+ LIST_FOREACH(scb, &ahc->pending_scbs, pending_links) {
+ if (cmd && scb->io_ctx == cmd) {
+ pending_scb = scb;
break;
+ }
}
- if (pending_scb == NULL && flag == SCB_DEVICE_RESET) {
-
+ if (!cmd) {
/* 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;
}
@@ -2151,22 +2156,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) {
@@ -2179,7 +2184,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;
@@ -2199,18 +2204,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) {
@@ -2231,7 +2236,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
@@ -2240,9 +2246,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);
@@ -2265,9 +2271,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);
@@ -2276,7 +2282,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] 75+ messages in thread
* [PATCH 16/24] aic79xx: do not reference scsi command when resetting device
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (14 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 15/24] aic7xxx: do not reference scsi command when resetting device Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:17 ` Christoph Hellwig
2022-05-03 15:25 ` Johannes Thumshirn
2022-05-02 21:38 ` [PATCH 17/24] snic: reserve tag for TMF Hannes Reinecke
` (7 subsequent siblings)
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
When sending a device reset we should not take a reference to the
scsi command.
Signed-off-by: Hannes Reinecke <hare@suse.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 b9b214ef5a2a..c46e8152af28 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 **************************************/
@@ -817,7 +819,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);
@@ -1772,9 +1774,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] 75+ messages in thread
* Re: [PATCH 16/24] aic79xx: do not reference scsi command when resetting device
2022-05-02 21:38 ` [PATCH 16/24] aic79xx: " Hannes Reinecke
@ 2022-05-03 14:17 ` Christoph Hellwig
2022-05-03 15:25 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:17 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 16/24] aic79xx: do not reference scsi command when resetting device
2022-05-02 21:38 ` [PATCH 16/24] aic79xx: " Hannes Reinecke
2022-05-03 14:17 ` Christoph Hellwig
@ 2022-05-03 15:25 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 15:25 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 17/24] snic: reserve tag for TMF
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (15 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 16/24] aic79xx: " Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:18 ` Christoph Hellwig
` (3 more replies)
2022-05-02 21:38 ` [PATCH 18/24] snic: use dedicated device reset command Hannes Reinecke
` (6 subsequent siblings)
23 siblings, 4 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
Rather than re-using the failed command the snic driver should
reserve one command for TMFs.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/snic/snic.h | 3 ++-
drivers/scsi/snic/snic_main.c | 3 +++
drivers/scsi/snic/snic_scsi.c | 43 +++++++++++++++++------------------
3 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/snic/snic.h b/drivers/scsi/snic/snic.h
index 4ec7e30678e1..e40492d36031 100644
--- a/drivers/scsi/snic/snic.h
+++ b/drivers/scsi/snic/snic.h
@@ -310,6 +310,7 @@ struct snic {
struct list_head spl_cmd_list;
unsigned int max_tag_id;
+ unsigned int tmf_tag_id;
atomic_t ios_inflight; /* io in flight counter */
struct vnic_snic_config config;
@@ -380,7 +381,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..f344cbc27923 100644
--- a/drivers/scsi/snic/snic_main.c
+++ b/drivers/scsi/snic/snic_main.c
@@ -512,6 +512,9 @@ snic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
max_t(u32, SNIC_MIN_IO_REQ, max_ios));
snic->max_tag_id = shost->can_queue;
+ /* Reserve one reset command */
+ shost->can_queue--;
+ snic->tmf_tag_id = shost->can_queue;
shost->max_lun = snic->config.luns_per_tgt;
shost->max_id = SNIC_MAX_TARGET;
diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
index 5f17666f3e1d..e18c8c5e4b46 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,18 @@ snic_reset(struct Scsi_Host *shost, struct scsi_cmnd *sc)
while (atomic_read(&snic->ios_inflight))
schedule_timeout(msecs_to_jiffies(1));
+ sc = scsi_host_find_tag(shost, snic->tmf_tag_id);
+ 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 +2360,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 +2386,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] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-02 21:38 ` [PATCH 17/24] snic: reserve tag for TMF Hannes Reinecke
@ 2022-05-03 14:18 ` Christoph Hellwig
2022-05-03 15:41 ` Johannes Thumshirn
` (2 subsequent siblings)
3 siblings, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:18 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-02 21:38 ` [PATCH 17/24] snic: reserve tag for TMF Hannes Reinecke
2022-05-03 14:18 ` Christoph Hellwig
@ 2022-05-03 15:41 ` Johannes Thumshirn
2022-05-03 16:18 ` Bart Van Assche
2022-05-06 13:28 ` John Garry
3 siblings, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 15:41 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-02 21:38 ` [PATCH 17/24] snic: reserve tag for TMF Hannes Reinecke
2022-05-03 14:18 ` Christoph Hellwig
2022-05-03 15:41 ` Johannes Thumshirn
@ 2022-05-03 16:18 ` Bart Van Assche
2022-05-03 18:31 ` Hannes Reinecke
2022-05-06 13:28 ` John Garry
3 siblings, 1 reply; 75+ messages in thread
From: Bart Van Assche @ 2022-05-03 16:18 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
On 5/2/22 14:38, Hannes Reinecke wrote:
> diff --git a/drivers/scsi/snic/snic_main.c b/drivers/scsi/snic/snic_main.c
> index 29d56396058c..f344cbc27923 100644
> --- a/drivers/scsi/snic/snic_main.c
> +++ b/drivers/scsi/snic/snic_main.c
> @@ -512,6 +512,9 @@ snic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> max_t(u32, SNIC_MIN_IO_REQ, max_ios));
>
> snic->max_tag_id = shost->can_queue;
> + /* Reserve one reset command */
> + shost->can_queue--;
> + snic->tmf_tag_id = shost->can_queue;
>
> shost->max_lun = snic->config.luns_per_tgt;
> shost->max_id = SNIC_MAX_TARGET;
Hmm ... shouldn't cmd_per_lun also be decreased?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-03 16:18 ` Bart Van Assche
@ 2022-05-03 18:31 ` Hannes Reinecke
2022-05-03 20:09 ` Bart Van Assche
0 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-03 18:31 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
On 5/3/22 09:18, Bart Van Assche wrote:
> On 5/2/22 14:38, Hannes Reinecke wrote:
>> diff --git a/drivers/scsi/snic/snic_main.c
>> b/drivers/scsi/snic/snic_main.c
>> index 29d56396058c..f344cbc27923 100644
>> --- a/drivers/scsi/snic/snic_main.c
>> +++ b/drivers/scsi/snic/snic_main.c
>> @@ -512,6 +512,9 @@ snic_probe(struct pci_dev *pdev, const struct
>> pci_device_id *ent)
>> max_t(u32, SNIC_MIN_IO_REQ, max_ios));
>> snic->max_tag_id = shost->can_queue;
>> + /* Reserve one reset command */
>> + shost->can_queue--;
>> + snic->tmf_tag_id = shost->can_queue;
>> shost->max_lun = snic->config.luns_per_tgt;
>> shost->max_id = SNIC_MAX_TARGET;
>
> Hmm ... shouldn't cmd_per_lun also be decreased?
>
Shudder. No. Why?
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: Felix Imendörffer
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-03 18:31 ` Hannes Reinecke
@ 2022-05-03 20:09 ` Bart Van Assche
0 siblings, 0 replies; 75+ messages in thread
From: Bart Van Assche @ 2022-05-03 20:09 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
On 5/3/22 11:31, Hannes Reinecke wrote:
> On 5/3/22 09:18, Bart Van Assche wrote:
>> On 5/2/22 14:38, Hannes Reinecke wrote:
>>> diff --git a/drivers/scsi/snic/snic_main.c
>>> b/drivers/scsi/snic/snic_main.c
>>> index 29d56396058c..f344cbc27923 100644
>>> --- a/drivers/scsi/snic/snic_main.c
>>> +++ b/drivers/scsi/snic/snic_main.c
>>> @@ -512,6 +512,9 @@ snic_probe(struct pci_dev *pdev, const struct
>>> pci_device_id *ent)
>>> max_t(u32, SNIC_MIN_IO_REQ, max_ios));
>>> snic->max_tag_id = shost->can_queue;
>>> + /* Reserve one reset command */
>>> + shost->can_queue--;
>>> + snic->tmf_tag_id = shost->can_queue;
>>> shost->max_lun = snic->config.luns_per_tgt;
>>> shost->max_id = SNIC_MAX_TARGET;
>>
>> Hmm ... shouldn't cmd_per_lun also be decreased?
>>
> Shudder. No. Why?
I found the answer. The following code from scsi_add_host_with_dma()
reduces cmd_per_lun so it is not necessary to do that from inside
snic_probe():
shost->cmd_per_lun = min_t(int, shost->cmd_per_lun,
shost->can_queue);
Bart.
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-02 21:38 ` [PATCH 17/24] snic: reserve tag for TMF Hannes Reinecke
` (2 preceding siblings ...)
2022-05-03 16:18 ` Bart Van Assche
@ 2022-05-06 13:28 ` John Garry
2022-05-07 7:32 ` Hannes Reinecke
3 siblings, 1 reply; 75+ messages in thread
From: John Garry @ 2022-05-06 13:28 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
>
> diff --git a/drivers/scsi/snic/snic_main.c b/drivers/scsi/snic/snic_main.c
> index 29d56396058c..f344cbc27923 100644
> --- a/drivers/scsi/snic/snic_main.c
> +++ b/drivers/scsi/snic/snic_main.c
> @@ -512,6 +512,9 @@ snic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> max_t(u32, SNIC_MIN_IO_REQ, max_ios));
>
> snic->max_tag_id = shost->can_queue;
> + /* Reserve one reset command */
> + shost->can_queue--;
> + snic->tmf_tag_id = shost->can_queue;
>
> shost->max_lun = snic->config.luns_per_tgt;
> shost->max_id = SNIC_MAX_TARGET;
> diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c
> index 5f17666f3e1d..e18c8c5e4b46 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,18 @@ snic_reset(struct Scsi_Host *shost, struct scsi_cmnd *sc)
> while (atomic_read(&snic->ios_inflight))
> schedule_timeout(msecs_to_jiffies(1));
>
> + sc = scsi_host_find_tag(shost, snic->tmf_tag_id);
Maybe I am missing something but this does not seem right. As I see,
blk-mq has driver tags in range [0, snic->tmf_tag_id - 1], so we cannot
call scsi_host_find_tag() -> blk_mq_unique_tag_to_tag(snic->tmf_tag_id)
thanks,
John
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-06 13:28 ` John Garry
@ 2022-05-07 7:32 ` Hannes Reinecke
2022-05-09 6:44 ` John Garry
0 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-07 7:32 UTC (permalink / raw)
To: John Garry, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
On 5/6/22 06:28, John Garry wrote:
>> diff --git a/drivers/scsi/snic/snic_main.c
>> b/drivers/scsi/snic/snic_main.c
>> index 29d56396058c..f344cbc27923 100644
>> --- a/drivers/scsi/snic/snic_main.c
>> +++ b/drivers/scsi/snic/snic_main.c
>> @@ -512,6 +512,9 @@ snic_probe(struct pci_dev *pdev, const struct
>> pci_device_id *ent)
>> max_t(u32, SNIC_MIN_IO_REQ, max_ios));
>> snic->max_tag_id = shost->can_queue;
>> + /* Reserve one reset command */
>> + shost->can_queue--;
>> + snic->tmf_tag_id = shost->can_queue;
>> shost->max_lun = snic->config.luns_per_tgt;
>> shost->max_id = SNIC_MAX_TARGET;
>> diff --git a/drivers/scsi/snic/snic_scsi.c
>> b/drivers/scsi/snic/snic_scsi.c
>> index 5f17666f3e1d..e18c8c5e4b46 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,18 @@ snic_reset(struct Scsi_Host *shost, struct
>> scsi_cmnd *sc)
>> while (atomic_read(&snic->ios_inflight))
>> schedule_timeout(msecs_to_jiffies(1));
>> + sc = scsi_host_find_tag(shost, snic->tmf_tag_id);
>
> Maybe I am missing something but this does not seem right. As I see,
> blk-mq has driver tags in range [0, snic->tmf_tag_id - 1], so we cannot
> call scsi_host_find_tag() -> blk_mq_unique_tag_to_tag(snic->tmf_tag_id)
>
We do decrease 'can_queue' by '1' just at the start of this patch, hence
the last tag is always available for TMF.
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: Felix Imendörffer
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 17/24] snic: reserve tag for TMF
2022-05-07 7:32 ` Hannes Reinecke
@ 2022-05-09 6:44 ` John Garry
0 siblings, 0 replies; 75+ messages in thread
From: John Garry @ 2022-05-09 6:44 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
On 07/05/2022 08:32, Hannes Reinecke wrote:
> On 5/6/22 06:28, John Garry wrote:
>>> diff --git a/drivers/scsi/snic/snic_main.c
>>> b/drivers/scsi/snic/snic_main.c
>>> index 29d56396058c..f344cbc27923 100644
>>> --- a/drivers/scsi/snic/snic_main.c
>>> +++ b/drivers/scsi/snic/snic_main.c
>>> @@ -512,6 +512,9 @@ snic_probe(struct pci_dev *pdev, const struct
>>> pci_device_id *ent)
>>> max_t(u32, SNIC_MIN_IO_REQ, max_ios));
>>> snic->max_tag_id = shost->can_queue;
>>> + /* Reserve one reset command */
>>> + shost->can_queue--;
>>> + snic->tmf_tag_id = shost->can_queue;
>>> shost->max_lun = snic->config.luns_per_tgt;
>>> shost->max_id = SNIC_MAX_TARGET;
>>> diff --git a/drivers/scsi/snic/snic_scsi.c
>>> b/drivers/scsi/snic/snic_scsi.c
>>> index 5f17666f3e1d..e18c8c5e4b46 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,18 @@ snic_reset(struct Scsi_Host *shost, struct
>>> scsi_cmnd *sc)
>>> while (atomic_read(&snic->ios_inflight))
>>> schedule_timeout(msecs_to_jiffies(1));
>>> + sc = scsi_host_find_tag(shost, snic->tmf_tag_id);
>>
>> Maybe I am missing something but this does not seem right. As I see,
>> blk-mq has driver tags in range [0, snic->tmf_tag_id - 1], so we
>> cannot call scsi_host_find_tag() ->
>> blk_mq_unique_tag_to_tag(snic->tmf_tag_id)
>>
Hi Hannes,
> We do decrease 'can_queue' by '1' just at the start of this patch, hence
> the last tag is always available for TMF.
So has this code changed this here:
https://lore.kernel.org/linux-scsi/de4a7479-3dbf-0842-f8f7-5d82b8bd6ea6@suse.de/
At a glance it looks the same.
Thanks,
John
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 18/24] snic: use dedicated device reset command
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (16 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 17/24] snic: reserve tag for TMF Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:19 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 19/24] snic: Use scsi_host_busy_iter() to traverse commands Hannes Reinecke
` (5 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
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>
---
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 e18c8c5e4b46..0d6156085616 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 device reset */
+ sc = scsi_host_find_tag(snic->shost, snic->tmf_tag_id);
+ 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));
@@ -2332,7 +2328,7 @@ snic_reset(struct Scsi_Host *shost)
schedule_timeout(msecs_to_jiffies(1));
sc = scsi_host_find_tag(shost, snic->tmf_tag_id);
- 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] 75+ messages in thread
* [PATCH 19/24] snic: Use scsi_host_busy_iter() to traverse commands
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (17 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 18/24] snic: use dedicated device reset command Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:19 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 20/24] sym53c8xx_2: split off bus reset from host reset Hannes Reinecke
` (4 subsequent siblings)
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
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>
---
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 0d6156085616..635656ccf30a 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);
@@ -2420,53 +2420,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));
@@ -2481,24 +2464,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
@@ -2506,7 +2499,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 */
/*
@@ -2520,7 +2513,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);
@@ -2595,6 +2588,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
*/
@@ -2602,11 +2624,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;
@@ -2614,44 +2634,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] 75+ messages in thread
* [PATCH 20/24] sym53c8xx_2: split off bus reset from host reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (18 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 19/24] snic: Use scsi_host_busy_iter() to traverse commands Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:20 ` Christoph Hellwig
2022-05-03 15:42 ` Johannes Thumshirn
2022-05-02 21:38 ` [PATCH 21/24] ips: Do not try to abort command " Hannes Reinecke
` (3 subsequent siblings)
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, Matthew Wilcox
The current handler does both, bus reset and host reset.
So split them off into two distinct functions.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Cc: Matthew Wilcox <willy@infradead.org>
---
drivers/scsi/sym53c8xx_2/sym_glue.c | 107 +++++++++++++++++-----------
1 file changed, 66 insertions(+), 41 deletions(-)
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 2e2852bd5860..9166af69bbb4 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -559,8 +559,6 @@ static void sym53c8xx_timer(struct timer_list *t)
*/
#define SYM_EH_ABORT 0
#define SYM_EH_DEVICE_RESET 1
-#define SYM_EH_BUS_RESET 2
-#define SYM_EH_HOST_RESET 3
/*
* Generic method for our eh processing.
@@ -580,35 +578,11 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
scmd_printk(KERN_WARNING, cmd, "%s operation started\n", opname);
- /* We may be in an error condition because the PCI bus
- * went down. In this case, we need to wait until the
- * PCI bus is reset, the card is reset, and only then
- * proceed with the scsi error recovery. There's no
- * point in hurrying; take a leisurely wait.
+ /*
+ * Escalate to host reset if the PCI bus went down
*/
-#define WAIT_FOR_PCI_RECOVERY 35
- if (pci_channel_offline(pdev)) {
- int finished_reset = 0;
- init_completion(&eh_done);
- spin_lock_irq(shost->host_lock);
- /* Make sure we didn't race */
- if (pci_channel_offline(pdev)) {
- BUG_ON(sym_data->io_reset);
- sym_data->io_reset = &eh_done;
- } else {
- finished_reset = 1;
- }
- spin_unlock_irq(shost->host_lock);
- if (!finished_reset)
- finished_reset = wait_for_completion_timeout
- (sym_data->io_reset,
- WAIT_FOR_PCI_RECOVERY*HZ);
- spin_lock_irq(shost->host_lock);
- sym_data->io_reset = NULL;
- spin_unlock_irq(shost->host_lock);
- if (!finished_reset)
- return SCSI_FAILED;
- }
+ if (pci_channel_offline(pdev))
+ return SCSI_FAILED;
spin_lock_irq(shost->host_lock);
/* This one is queued in some place -> to wait for completion */
@@ -629,15 +603,6 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
case SYM_EH_DEVICE_RESET:
sts = sym_reset_scsi_target(np, cmd->device->id);
break;
- case SYM_EH_BUS_RESET:
- sym_reset_scsi_bus(np, 1);
- sts = 0;
- break;
- case SYM_EH_HOST_RESET:
- sym_reset_scsi_bus(np, 0);
- sym_start_up(shost, 1);
- sts = 0;
- break;
default:
break;
}
@@ -679,12 +644,72 @@ static int sym53c8xx_eh_device_reset_handler(struct scsi_cmnd *cmd)
static int sym53c8xx_eh_bus_reset_handler(struct scsi_cmnd *cmd)
{
- return sym_eh_handler(SYM_EH_BUS_RESET, "BUS RESET", cmd);
+ struct Scsi_Host *shost = cmd->device->host;
+ struct sym_data *sym_data = shost_priv(shost);
+ struct pci_dev *pdev = sym_data->pdev;
+ struct sym_hcb *np = sym_data->ncb;
+
+ scmd_printk(KERN_WARNING, cmd, "BUS RESET operation started\n");
+
+ /*
+ * Escalate to host reset if the PCI bus went down
+ */
+ if (pci_channel_offline(pdev))
+ return SCSI_FAILED;
+
+ spin_lock_irq(shost->host_lock);
+ sym_reset_scsi_bus(np, 1);
+ spin_unlock_irq(shost->host_lock);
+
+ dev_warn(&cmd->device->sdev_gendev, "BUS RESET operation complete.\n");
+ return SCSI_SUCCESS;
}
static int sym53c8xx_eh_host_reset_handler(struct scsi_cmnd *cmd)
{
- return sym_eh_handler(SYM_EH_HOST_RESET, "HOST RESET", cmd);
+ struct Scsi_Host *shost = cmd->device->host;
+ struct sym_data *sym_data = shost_priv(shost);
+ struct pci_dev *pdev = sym_data->pdev;
+ struct sym_hcb *np = sym_data->ncb;
+ struct completion eh_done;
+ int finished_reset = 1;
+
+ shost_printk(KERN_WARNING, shost, "HOST RESET operation started\n");
+
+ /* We may be in an error condition because the PCI bus
+ * went down. In this case, we need to wait until the
+ * PCI bus is reset, the card is reset, and only then
+ * proceed with the scsi error recovery. There's no
+ * point in hurrying; take a leisurely wait.
+ */
+#define WAIT_FOR_PCI_RECOVERY 35
+ if (pci_channel_offline(pdev)) {
+ init_completion(&eh_done);
+ spin_lock_irq(shost->host_lock);
+ /* Make sure we didn't race */
+ if (pci_channel_offline(pdev)) {
+ BUG_ON(sym_data->io_reset);
+ sym_data->io_reset = &eh_done;
+ finished_reset = 0;
+ }
+ spin_unlock_irq(shost->host_lock);
+ if (!finished_reset)
+ finished_reset = wait_for_completion_timeout
+ (sym_data->io_reset,
+ WAIT_FOR_PCI_RECOVERY*HZ);
+ spin_lock_irq(shost->host_lock);
+ sym_data->io_reset = NULL;
+ spin_unlock_irq(shost->host_lock);
+ }
+
+ if (finished_reset) {
+ sym_reset_scsi_bus(np, 0);
+ sym_start_up(shost, 1);
+ }
+
+ shost_printk(KERN_WARNING, shost, "HOST RESET operation %s.\n",
+ finished_reset==1 ? "complete" : "failed");
+ return finished_reset ? SCSI_SUCCESS : SCSI_FAILED;
}
/*
--
2.29.2
^ permalink raw reply related [flat|nested] 75+ messages in thread
* Re: [PATCH 20/24] sym53c8xx_2: split off bus reset from host reset
2022-05-02 21:38 ` [PATCH 20/24] sym53c8xx_2: split off bus reset from host reset Hannes Reinecke
@ 2022-05-03 14:20 ` Christoph Hellwig
2022-05-03 15:42 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:20 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Matthew Wilcox
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 20/24] sym53c8xx_2: split off bus reset from host reset
2022-05-02 21:38 ` [PATCH 20/24] sym53c8xx_2: split off bus reset from host reset Hannes Reinecke
2022-05-03 14:20 ` Christoph Hellwig
@ 2022-05-03 15:42 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 15:42 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Matthew Wilcox
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 21/24] ips: Do not try to abort command from host reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (19 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 20/24] sym53c8xx_2: split off bus reset from host reset Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:20 ` Christoph Hellwig
2022-05-03 15:43 ` Johannes Thumshirn
2022-05-02 21:38 ` [PATCH 22/24] qla1280: separate out host reset function from qla1280_error_action() Hannes Reinecke
` (2 subsequent siblings)
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke, 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>
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] 75+ messages in thread
* Re: [PATCH 21/24] ips: Do not try to abort command from host reset
2022-05-02 21:38 ` [PATCH 21/24] ips: Do not try to abort command " Hannes Reinecke
@ 2022-05-03 14:20 ` Christoph Hellwig
2022-05-03 15:43 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:20 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke, Adaptec OEM Raid Solutions
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 21/24] ips: Do not try to abort command from host reset
2022-05-02 21:38 ` [PATCH 21/24] ips: Do not try to abort command " Hannes Reinecke
2022-05-03 14:20 ` Christoph Hellwig
@ 2022-05-03 15:43 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 15:43 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Adaptec OEM Raid Solutions
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 22/24] qla1280: separate out host reset function from qla1280_error_action()
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (20 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 21/24] ips: Do not try to abort command " Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 13:32 ` kernel test robot
2022-05-03 14:20 ` Christoph Hellwig
2022-05-02 21:38 ` [PATCH 23/24] megaraid: pass in NULL scb for host reset Hannes Reinecke
2022-05-02 21:38 ` [PATCH 24/24] mpi3mr: split off bus_reset function from host_reset Hannes Reinecke
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
There's not much in common between host reset and all other error
handlers, so use a separate function here.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
drivers/scsi/qla1280.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index 0ab595c0870a..c4466e4e692f 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -716,7 +716,6 @@ enum action {
ABORT_COMMAND,
DEVICE_RESET,
BUS_RESET,
- ADAPTER_RESET,
};
@@ -898,22 +897,9 @@ qla1280_error_action(struct scsi_cmnd *cmd, enum action action)
}
break;
- case ADAPTER_RESET:
default:
- if (qla1280_verbose) {
- printk(KERN_INFO
- "scsi(%ld): Issued ADAPTER RESET\n",
- ha->host_no);
- printk(KERN_INFO "scsi(%ld): I/O processing will "
- "continue automatically\n", ha->host_no);
- }
- ha->flags.reset_active = 1;
-
- if (qla1280_abort_isp(ha) != 0) { /* it's dead */
- result = FAILED;
- }
-
- ha->flags.reset_active = 0;
+ dprintk(1, "RESET invalid action %d\n", action);
+ return FAILED;
}
/*
@@ -1012,10 +998,26 @@ static int
qla1280_eh_adapter_reset(struct scsi_cmnd *cmd)
{
int rc;
+ struct Scsi_Host *shost = cmd->device->host;
+ struct scsi_qla_host *ha = (struct scsi_qla_host *)shost->hostdata;
- spin_lock_irq(cmd->device->host->host_lock);
- rc = qla1280_error_action(cmd, ADAPTER_RESET);
- spin_unlock_irq(cmd->device->host->host_lock);
+ spin_lock_irq(shost->host_lock);
+ if (qla1280_verbose) {
+ printk(KERN_INFO
+ "scsi(%ld): Issued ADAPTER RESET\n",
+ ha->host_no);
+ printk(KERN_INFO "scsi(%ld): I/O processing will "
+ "continue automatically\n", ha->host_no);
+ }
+ ha->flags.reset_active = 1;
+
+ if (qla1280_abort_isp(ha) != 0) { /* it's dead */
+ rc = FAILED;
+ }
+
+ ha->flags.reset_active = 0;
+
+ spin_unlock_irq(shost->host_lock);
return rc;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 75+ messages in thread
* Re: [PATCH 22/24] qla1280: separate out host reset function from qla1280_error_action()
2022-05-02 21:38 ` [PATCH 22/24] qla1280: separate out host reset function from qla1280_error_action() Hannes Reinecke
@ 2022-05-03 13:32 ` kernel test robot
2022-05-03 14:20 ` Christoph Hellwig
1 sibling, 0 replies; 75+ messages in thread
From: kernel test robot @ 2022-05-03 13:32 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: llvm, kbuild-all
Hi Hannes,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next powerpc/next linus/master v5.18-rc5 next-20220503]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Hannes-Reinecke/scsi-EH-rework-prep-patches-part-1/20220503-054317
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: arm64-randconfig-r022-20220502 (https://download.01.org/0day-ci/archive/20220503/202205032113.60oVJQpe-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 363b3a645a1e30011cc8da624f13dac5fd915628)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/c29ab2b7bfc02579580a4a887b6e5d7fe29807cc
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Hannes-Reinecke/scsi-EH-rework-prep-patches-part-1/20220503-054317
git checkout c29ab2b7bfc02579580a4a887b6e5d7fe29807cc
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/scsi/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/scsi/qla1280.c:1014:6: warning: variable 'rc' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (qla1280_abort_isp(ha) != 0) { /* it's dead */
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/qla1280.c:1022:9: note: uninitialized use occurs here
return rc;
^~
drivers/scsi/qla1280.c:1014:2: note: remove the 'if' if its condition is always true
if (qla1280_abort_isp(ha) != 0) { /* it's dead */
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/qla1280.c:1000:8: note: initialize the variable 'rc' to silence this warning
int rc;
^
= 0
drivers/scsi/qla1280.c:1712:15: warning: variable 'num' set but not used [-Wunused-but-set-variable]
int err = 0, num, i;
^
2 warnings generated.
vim +1014 drivers/scsi/qla1280.c
992
993 /**************************************************************************
994 * qla1280_adapter_reset
995 * Reset the specified adapter (both channels)
996 **************************************************************************/
997 static int
998 qla1280_eh_adapter_reset(struct scsi_cmnd *cmd)
999 {
1000 int rc;
1001 struct Scsi_Host *shost = cmd->device->host;
1002 struct scsi_qla_host *ha = (struct scsi_qla_host *)shost->hostdata;
1003
1004 spin_lock_irq(shost->host_lock);
1005 if (qla1280_verbose) {
1006 printk(KERN_INFO
1007 "scsi(%ld): Issued ADAPTER RESET\n",
1008 ha->host_no);
1009 printk(KERN_INFO "scsi(%ld): I/O processing will "
1010 "continue automatically\n", ha->host_no);
1011 }
1012 ha->flags.reset_active = 1;
1013
> 1014 if (qla1280_abort_isp(ha) != 0) { /* it's dead */
1015 rc = FAILED;
1016 }
1017
1018 ha->flags.reset_active = 0;
1019
1020 spin_unlock_irq(shost->host_lock);
1021
1022 return rc;
1023 }
1024
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 22/24] qla1280: separate out host reset function from qla1280_error_action()
2022-05-02 21:38 ` [PATCH 22/24] qla1280: separate out host reset function from qla1280_error_action() Hannes Reinecke
2022-05-03 13:32 ` kernel test robot
@ 2022-05-03 14:20 ` Christoph Hellwig
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:20 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 23/24] megaraid: pass in NULL scb for host reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (21 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 22/24] qla1280: separate out host reset function from qla1280_error_action() Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:20 ` Christoph Hellwig
2022-05-03 16:09 ` Johannes Thumshirn
2022-05-02 21:38 ` [PATCH 24/24] mpi3mr: split off bus_reset function from host_reset Hannes Reinecke
23 siblings, 2 replies; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Hannes Reinecke
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>
---
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] 75+ messages in thread
* Re: [PATCH 23/24] megaraid: pass in NULL scb for host reset
2022-05-02 21:38 ` [PATCH 23/24] megaraid: pass in NULL scb for host reset Hannes Reinecke
@ 2022-05-03 14:20 ` Christoph Hellwig
2022-05-03 16:09 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Christoph Hellwig @ 2022-05-03 14:20 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
linux-scsi, Hannes Reinecke
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 75+ messages in thread
* Re: [PATCH 23/24] megaraid: pass in NULL scb for host reset
2022-05-02 21:38 ` [PATCH 23/24] megaraid: pass in NULL scb for host reset Hannes Reinecke
2022-05-03 14:20 ` Christoph Hellwig
@ 2022-05-03 16:09 ` Johannes Thumshirn
1 sibling, 0 replies; 75+ messages in thread
From: Johannes Thumshirn @ 2022-05-03 16:09 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 24/24] mpi3mr: split off bus_reset function from host_reset
2022-05-02 21:37 [PATCH 00/24] scsi: EH rework prep patches, part 1 Hannes Reinecke
` (22 preceding siblings ...)
2022-05-02 21:38 ` [PATCH 23/24] megaraid: pass in NULL scb for host reset Hannes Reinecke
@ 2022-05-02 21:38 ` Hannes Reinecke
2022-05-03 14:21 ` Christoph Hellwig
23 siblings, 1 reply; 75+ messages in thread
From: Hannes Reinecke @ 2022-05-02 21:38 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke,
Kashyap Desai, Sathya Prakash Veerichetty, Sumit Saxena,
Sreekanth Reddy
SCSI EH host reset is the final callback in the escalation chain;
once we reach this we need to reset the controller.
As such it defeats the purpose to skip controller reset if no
I/Os are pending and the RAID device is to be reset; especially
after kexec there might be stale commands pending in firmware
for which we have no reference whatsoever.
So this patch splits off the check for pending I/O into a
'bus_reset' function, and leaves the actual controller reset
to the host reset.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>
Cc: Sumit Saxena <sumit.saxena@broadcom.com>
Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
---
drivers/scsi/mpi3mr/mpi3mr_os.c | 57 +++++++++++++++++++++------------
1 file changed, 37 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index f7cd70a15ea6..31f05e9f5ee0 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -3337,20 +3337,45 @@ void mpi3mr_wait_for_host_io(struct mpi3mr_ioc *mrioc, u32 timeout)
* mpi3mr_eh_host_reset - Host reset error handling callback
* @scmd: SCSI command reference
*
- * Issue controller reset if the scmd is for a Physical Device,
- * if the scmd is for RAID volume, then wait for
- * MPI3MR_RAID_ERRREC_RESET_TIMEOUT and checke whether any
- * pending I/Os prior to issuing reset to the controller.
+ * Issue controller reset
*
* Return: SUCCESS of successful reset else FAILED
*/
static int mpi3mr_eh_host_reset(struct scsi_cmnd *scmd)
+{
+ struct mpi3mr_ioc *mrioc = shost_priv(scmd->device->host);
+ int retval = FAILED, ret;
+
+ ret = mpi3mr_soft_reset_handler(mrioc,
+ MPI3MR_RESET_FROM_EH_HOS, 1);
+ if (ret)
+ goto out;
+
+ retval = SUCCESS;
+out:
+ sdev_printk(KERN_INFO, scmd->device,
+ "Host reset is %s for scmd(%p)\n",
+ ((retval == SUCCESS) ? "SUCCESS" : "FAILED"), scmd);
+
+ return retval;
+}
+
+/**
+ * mpi3mr_eh_bus_reset - Bus reset error handling callback
+ * @scmd: SCSI command reference
+ *
+ * Checks whether pending I/Os are present for the RAID volume;
+ * if not there's no need to reset the adapter.
+ *
+ * Return: SUCCESS of successful reset else FAILED
+ */
+static int mpi3mr_eh_bus_reset(struct scsi_cmnd *scmd)
{
struct mpi3mr_ioc *mrioc = shost_priv(scmd->device->host);
struct mpi3mr_stgt_priv_data *stgt_priv_data;
struct mpi3mr_sdev_priv_data *sdev_priv_data;
u8 dev_type = MPI3_DEVICE_DEVFORM_VD;
- int retval = FAILED, ret;
+ int retval = FAILED;
sdev_priv_data = scmd->device->hostdata;
if (sdev_priv_data && sdev_priv_data->tgt_priv_data) {
@@ -3360,25 +3385,16 @@ static int mpi3mr_eh_host_reset(struct scsi_cmnd *scmd)
if (dev_type == MPI3_DEVICE_DEVFORM_VD) {
mpi3mr_wait_for_host_io(mrioc,
- MPI3MR_RAID_ERRREC_RESET_TIMEOUT);
- if (!mpi3mr_get_fw_pending_ios(mrioc)) {
+ MPI3MR_RAID_ERRREC_RESET_TIMEOUT);
+ if (!mpi3mr_get_fw_pending_ios(mrioc))
retval = SUCCESS;
- goto out;
- }
}
+ if (retval == FAILED)
+ mpi3mr_print_pending_host_io(mrioc);
- mpi3mr_print_pending_host_io(mrioc);
- ret = mpi3mr_soft_reset_handler(mrioc,
- MPI3MR_RESET_FROM_EH_HOS, 1);
- if (ret)
- goto out;
-
- retval = SUCCESS;
-out:
sdev_printk(KERN_INFO, scmd->device,
- "Host reset is %s for scmd(%p)\n",
- ((retval == SUCCESS) ? "SUCCESS" : "FAILED"), scmd);
-
+ "Bus reset is %s for scmd(%p)\n",
+ ((retval == SUCCESS) ? "SUCCESS" : "FAILED"), scmd);
return retval;
}
@@ -4094,6 +4110,7 @@ static struct scsi_host_template mpi3mr_driver_template = {
.change_queue_depth = mpi3mr_change_queue_depth,
.eh_device_reset_handler = mpi3mr_eh_dev_reset,
.eh_target_reset_handler = mpi3mr_eh_target_reset,
+ .eh_bus_reset_handler = mpi3mr_eh_bus_reset,
.eh_host_reset_handler = mpi3mr_eh_host_reset,
.bios_param = mpi3mr_bios_param,
.map_queues = mpi3mr_map_queues,
--
2.29.2
^ permalink raw reply related [flat|nested] 75+ messages in thread