| Message ID | 20230416175654.159163-1-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: scsi_debug: Abort commands from scsi_debug_device_reset() | expand |
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>
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!
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);
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>