diff mbox series

[01/16] zfcp: do not wait for rports to become unblocked after host reset

Message ID 20231017100729.123506-2-hare@suse.de
State Superseded
Headers show
Series scsi: EH rework prep patches, part 2 | expand

Commit Message

Hannes Reinecke Oct. 17, 2023, 10:07 a.m. UTC
zfcp_scsi_eh_host_reset_handler() would call fc_block_rport() to
wait for all rports to become unblocked after host reset.
But after host reset it might happen that the port is gone, hence
fc_block_rport() might fail due to a missing port.
But that's a perfectly legal operation; on FC remote ports might
come and go.
So this patch removes the call to fc_block_rport() after host
reset. But with that rports may still be in blocked state after
host reset, so we need to return FAST_IO_FAIL from host reset
to avoid SCSI EH to fail commands prematurely if the rports
are still blocked.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Steffen Maier <maier@linux.ibm.com
Cc: Benjamin Block <bblock@linux.ibm.com>
---
 drivers/s390/scsi/zfcp_scsi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Hannes Reinecke Oct. 20, 2023, 6:18 a.m. UTC | #1
On 10/19/23 19:44, Benjamin Block wrote:
> Hey Hannes,
> 
> On Tue, Oct 17, 2023 at 12:07:14PM +0200, Hannes Reinecke wrote:
>> zfcp_scsi_eh_host_reset_handler() would call fc_block_rport() to
>> wait for all rports to become unblocked after host reset.
>> But after host reset it might happen that the port is gone, hence
>> fc_block_rport() might fail due to a missing port.
>> But that's a perfectly legal operation; on FC remote ports might
>> come and go.
>> So this patch removes the call to fc_block_rport() after host
>> reset. But with that rports may still be in blocked state after
>> host reset, so we need to return FAST_IO_FAIL from host reset
>> to avoid SCSI EH to fail commands prematurely if the rports
>> are still blocked.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> Cc: Steffen Maier <maier@linux.ibm.com
>> Cc: Benjamin Block <bblock@linux.ibm.com>
>> ---
>>   drivers/s390/scsi/zfcp_scsi.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
>> index b2a8cd792266..b1df853e6f66 100644
>> --- a/drivers/s390/scsi/zfcp_scsi.c
>> +++ b/drivers/s390/scsi/zfcp_scsi.c
>> @@ -375,7 +375,7 @@ 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;
>> +	int ret = FAST_IO_FAIL;
> 
> One thing I noticed, `scsi_ioctl_reset()` doesn't handle FAST_IO_FAIL at all;
> it just considers that a failure, and returns EIO to userspace. I wonder if
> this is appropriate after what we've discussed. In our case FAST_IO_FAIL
> doesn't mean the host reset failed.
> 
Hmm. Indeed, that is not quite correct. EIO indicates an IO error, 
whereas FAST_IO_FAIL signal more something like 'operation in progress, 
couldn't complete in time'. So I guess EAGAIN is more appropriate here.

> I don't think we need to adapt this patch for that - couldn't really - but
> maybe there should be an extra change for that. Not exactly sure what even
> uses that IOCTL interface, and how it'd react to an EIO.
> 
Yes, we should be fixing that one. After all, FAST_IO_FAIL is a 
perfectly legit return value for the EH functions, and we should
be handling it properly.

Will be adding a patch for it.
Thanks for the heads-up.

Cheers,

Hannes
diff mbox series

Patch

diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index b2a8cd792266..b1df853e6f66 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -375,7 +375,7 @@  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;
+	int ret = FAST_IO_FAIL;
 
 	if (!(adapter->connection_features & FSF_FEATURE_NPIV_MODE)) {
 		zfcp_erp_port_forced_reopen_all(adapter, 0, "schrh_p");
@@ -383,10 +383,6 @@  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;
-
 	zfcp_dbf_scsi_eh("schrh_r", adapter, ~0, ret);
 	return ret;
 }