Re: [PATCH] scsi_transport_srp: sanitize scsi_target_block/unblock sequences

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/28/20 9:50 PM, Laurence Oberman wrote:
On Tue, 2020-07-28 at 14:43 -0400, Laurence Oberman wrote:
On Tue, 2020-07-28 at 15:48 +0200, Hannes Reinecke wrote:
The SCSI midlayer does not allow state transitions from SDEV_BLOCK
to SDEV_BLOCK, so calling scsi_target_block() from
__rport_fast_io_fail()
is wrong as the port is already blocked.
Similarly we don't need to call scsi_target_unblock() afterwards as
the
function has already done this.

Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
---
  drivers/scsi/scsi_transport_srp.c | 12 +++++-------
  1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_transport_srp.c
b/drivers/scsi/scsi_transport_srp.c
index d4d1104fac99..cba1cf6a1c12 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -395,6 +395,10 @@ static void srp_reconnect_work(struct
work_struct *work)
  	}
  }
+/*
+ * scsi_target_block() must have been called before this function
is
+ * called to guarantee that no .queuecommand() calls are in
progress.
+ */
  static void __rport_fail_io_fast(struct srp_rport *rport)
  {
  	struct Scsi_Host *shost = rport_to_shost(rport);
@@ -404,11 +408,7 @@ static void __rport_fail_io_fast(struct
srp_rport *rport)
if (srp_rport_set_state(rport, SRP_RPORT_FAIL_FAST))
  		return;
-	/*
-	 * Call scsi_target_block() to wait for ongoing shost-
queuecommand()

-	 * calls before invoking i->f->terminate_rport_io().
-	 */
-	scsi_target_block(rport->dev.parent);
+
  	scsi_target_unblock(rport->dev.parent, SDEV_TRANSPORT_OFFLINE);
/* Involve the LLD if possible to terminate all I/O on the
rport. */
@@ -570,8 +570,6 @@ int srp_reconnect_rport(struct srp_rport
*rport)
  		 * failure timers if these had not yet been started.
  		 */
  		__rport_fail_io_fast(rport);
-		scsi_target_unblock(&shost->shost_gendev,
-				    SDEV_TRANSPORT_OFFLINE);
  		__srp_start_tl_fail_timers(rport);
  	} else if (rport->state != SRP_RPORT_BLOCKED) {
  		scsi_target_unblock(&shost->shost_gendev,

This looks OK to me but I guess its alwasy worked by just ignoring it
being called or IU would have seenm issues.
I etest that stuff pretty heavily.
Reviewed-by: Laurence Oberman <loberman@xxxxxxxxxx>


Ouch, my last message saw my bad touch typing creep in. Let me try
again.

This looks OK to me but I guess its always worked by just ignoring it
being called or I would have seen issues already.
I test that stuff pretty heavily and regularly.

Reviewed-by: Laurence Oberman <loberman@xxxxxxxxxx>


Well, we've had a customer running into it.
And they do _heavy_ tests, too.

I haven't said it's a regular occurrence :-)

Cheers,

Hannes
--
Dr. Hannes Reinecke            Teamlead Storage & Networking
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



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux