This patch looks like it will call ibmvfc_reset_device() w/IBMVFC_TARGET_RESET
before it has called ibmvfc_cancel_all() on all the devices, the
existing code calls
ibmvfc_reset_device() w/IBMVFC_TARGET_RESET after the iterator.
Since you have the starget, why change to use shost_for_each_device()
and check the
sdev->channel and sdev->id ? That's what starget_for_each_device() does.
-Ewan
On Thu, May 12, 2022 at 7:13 AM Hannes Reinecke <hare@xxxxxxx> wrote:
>
> From: Hannes Reinecke <hare@xxxxxxxx>
>
> 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@xxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> Cc: Tyrel Datwyler <tyreld@xxxxxxxxxxxxx>
> ---
> 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
>
