[PATCH] scsi: scsi_debug: Abort commands from scsi_debug_device_reset()

[PATCH] scsi: scsi_debug: Abort commands from scsi_debug_device_reset()

[John Garry]

Currently scsi_debug_device_reset() does not do much apart from setting
the SDEBUG_UA_POR ("Power on, reset, or bus device reset") flag, which is
eventually passed back to the SCSI midlayer later for a "unit attention"
command.

There is a report that blktest scsi/007 test fails due to commit
1107c7b24ee3 ("scsi: scsi_debug: Dynamically allocate sdebug_queued_cmd").
The problem there is that there are dangling scsi_debug queued commands
when we attempt to remove the driver.

scsi/007 test triggers SCSI EH and attempts to abort a timed-out command.
Function scsi_debug_device_reset() is called as part of the EH, but does
not deal with outstanding erroneous command. Prior to the named commit,
removing the driver caused all dangling queued commands to be stopped -
this should have not been necessary.

Fix by aborting outstanding commands on a scsi_device basis from
scsi_debug_device_reset().

Fixes: 1107c7b24ee3 ("scsi: scsi_debug: Dynamically allocate sdebug_queued_cmd")
Reported-by: kernel test robot <yujie.liu@intel.com>
Link: https://lore.kernel.org/oe-lkp/202304071111.e762fcbd-yujie.liu@intel.com
Signed-off-by: John Garry <john.g.garry@oracle.com>

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index fd1c28352df0..c2d2ed3a2db1 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5291,6 +5291,26 @@ static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
 	return SUCCESS;
 }
 
+static bool scsi_debug_stop_all_queued_iter(struct request *rq, void *data)
+{
+	struct scsi_device *sdp = data;
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
+
+	if (scmd->device == sdp)
+		scsi_debug_abort_cmnd(scmd);
+
+	return true;
+}
+
+/* Deletes (stops) timers or work queues of all queued commands per sdev */
+static void scsi_debug_stop_all_queued(struct scsi_device *sdp)
+{
+	struct Scsi_Host *shost = sdp->host;
+
+	blk_mq_tagset_busy_iter(&shost->tag_set,
+				scsi_debug_stop_all_queued_iter, sdp);
+}
+
 static int scsi_debug_device_reset(struct scsi_cmnd *SCpnt)
 {
 	struct scsi_device *sdp = SCpnt->device;
@@ -5300,6 +5320,8 @@ static int scsi_debug_device_reset(struct scsi_cmnd *SCpnt)
 
 	if (SDEBUG_OPT_ALL_NOISE & sdebug_opts)
 		sdev_printk(KERN_INFO, sdp, "%s\n", __func__);
+
+	scsi_debug_stop_all_queued(sdp);
 	if (devip)
 		set_bit(SDEBUG_UA_POR, devip->uas_bm);
 
-- 
2.35.3

Re: [PATCH] scsi: scsi_debug: Abort commands from scsi_debug_device_reset()

[Bart Van Assche]

On 4/16/23 10:56, John Garry wrote:
> Currently scsi_debug_device_reset() does not do much apart from setting
> the SDEBUG_UA_POR ("Power on, reset, or bus device reset") flag, which is
> eventually passed back to the SCSI midlayer later for a "unit attention"
> command.
> 
> There is a report that blktest scsi/007 test fails due to commit
> 1107c7b24ee3 ("scsi: scsi_debug: Dynamically allocate sdebug_queued_cmd").
> The problem there is that there are dangling scsi_debug queued commands
> when we attempt to remove the driver.
> 
> scsi/007 test triggers SCSI EH and attempts to abort a timed-out command.
> Function scsi_debug_device_reset() is called as part of the EH, but does
> not deal with outstanding erroneous command. Prior to the named commit,
> removing the driver caused all dangling queued commands to be stopped -
> this should have not been necessary.
> 
> Fix by aborting outstanding commands on a scsi_device basis from
> scsi_debug_device_reset().
> 
> Fixes: 1107c7b24ee3 ("scsi: scsi_debug: Dynamically allocate sdebug_queued_cmd")
> Reported-by: kernel test robot <yujie.liu@intel.com>
> Link: https://lore.kernel.org/oe-lkp/202304071111.e762fcbd-yujie.liu@intel.com
> Signed-off-by: John Garry <john.g.garry@oracle.com>

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH] scsi: scsi_debug: Abort commands from scsi_debug_device_reset()

[Martin K. Petersen]

John,

> Currently scsi_debug_device_reset() does not do much apart from
> setting the SDEBUG_UA_POR ("Power on, reset, or bus device reset")
> flag, which is eventually passed back to the SCSI midlayer later for a
> "unit attention" command.

Applied to 6.4/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering