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@xxxxxxxx>
Cc: Steffen Maier <maier@xxxxxxxxxxxxx>
Cc: Benjamin Block <bblock@xxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
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@xxxxxxxxxxxxxxxxxx/
It also alludes to lock ordering.
Right. Will be folding in the changes.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
