Re: [PATCH 3/7] scsi: Use scsi_target as argument for eh_target_reset_handler()

Steffen Maier
2022-05-06T10:18:43Z

On 5/2/22 23:59, Hannes Reinecke wrote:
> The target reset function should only depend on the scsi target,
> not the scsi command.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: James Smart <jsmart2021@gmail.com>
> ---
> Documentation/scsi/scsi_eh.rst | 10 ++++
> Documentation/scsi/scsi_mid_low_api.rst | 18 +++++++

> drivers/s390/scsi/zfcp_scsi.c | 7 ++-

> drivers/scsi/scsi_debug.c | 21 +++-----
> drivers/scsi/scsi_error.c | 5 +-

> include/scsi/scsi_host.h | 2 +-
> 34 files changed, 216 insertions(+), 190 deletions(-)

> diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
> index 873ccc60e4d6..10bbe56189b3 100644
> --- a/drivers/s390/scsi/zfcp_scsi.c
> +++ b/drivers/s390/scsi/zfcp_scsi.c
> @@ -340,9 +340,12 @@ static int zfcp_scsi_eh_device_reset_handler(struct scsi_cmnd *scpnt)
> return zfcp_scsi_task_mgmt_function(sdev, FCP_TMF_LUN_RESET);
> }
>
> -static int zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)

> +/*
> + * Note: We need to select a LUN as the storage array doesn't
> + * necessarily supports LUN 0 and might refuse the target reset.
> + */

That's not the reason and this patch does not change anything regarding LUN
handling within target reset handler, so I wonder if we should just drop this
new comment.

The reason was given in the description of:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=674595d8519fdf4f78e1a4cceb2130ffa46bcdeb
"zfcp_scsi_eh_target_reset_handler() is special: The FCP channel requires a
valid LUN handle so we try to find ourselves a stand-in scsi_device"

I think that's sufficient. If you think we also need a copy in the code, you
can copy&paste the description.

> +static int zfcp_scsi_eh_target_reset_handler(struct scsi_target *starget)
> {
> - struct scsi_target *starget = scsi_target(scpnt->device);
> struct fc_rport *rport = starget_to_rport(starget);
> struct Scsi_Host *shost = rport_to_shost(rport);
> struct scsi_device *sdev = NULL, *tmp_sdev;

the other change to zfcp looks good

--
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