On 8/17/21 1:53 PM, Benjamin Block wrote:

On Tue, Aug 17, 2021 at 11:14:13AM +0200, Hannes Reinecke wrote:

quoted

@@ -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) {

You need to take the `adapter->port_list_lock` to iterate over the `port_list`.

i.e.: read_lock_irqsave(&adapter->port_list_lock, flags);

quoted

+		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;
+	}

I really can't say I like this open coded FC code in the driver at all.

Is there a reason we can't use `fc_block_rport()` for all the rports of
the adapter?

We already do use it for other EH callbacks in the same file, and you
already look up the rports in the adapters rport-list; so using that on
the rports in the loop, instead of open-coding it doesn't seem bad? Or
is there a locking problem? 

We might waste a few cycles with that, but frankly, this is all in EH
and after adapter reset.. all performance concerns went our of the
window with that already.

Question would be why we need to call fc_block_rport() at all in host reset.
To my understanding a host reset is expected to do a full resync of the
SAN topology, so the expectation is that after zfcp_erp_wait() the port
list is stable (ie the HBA has finished processing all RSCNs related to
the SAN resync).
So can't we just drop the fc_block_rport() call here?
All the other FC drivers do fine without that ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer