diff mbox series

[4/4] scsi: scsi_debug: Do not sleep in atomic sections

Message ID 20250224115517.495899-5-john.g.garry@oracle.com
State New
Headers show
Series None | expand

Commit Message

John Garry Feb. 24, 2025, 11:55 a.m. UTC 
From: Bart Van Assche <bvanassche@acm.org>

Function stop_qc_helper() is called while the debug_scsi_cmd lock is held,
and from here we may call cancel_work_sync(), which may sleep.

Sleeping in atomic sections is not allowed.

Hence change the cancel_work_sync() call into a cancel_work() call.

However now it is not possible to know if the work callback is running
when we return. This is relevant for eh_abort_handler handling, as the
semantics of that callback are that success means that we do not keep a
reference to the scsi_cmnd - now this is not possible. So return FAIL
when we are unsure if the callback still running.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
jpg: return FAILED from scsi_debug_abort() when possible callback running
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/scsi/scsi_debug.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c1a217b5aac2..2208dcba346e 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6655,7 +6655,7 @@  static void scsi_debug_sdev_destroy(struct scsi_device *sdp)
 	sdp->hostdata = NULL;
 }
 
-/* Returns true if it is safe to complete @cmnd. */
+/* Returns true if cancelled or not running callback. */
 static bool scsi_debug_stop_cmnd(struct scsi_cmnd *cmnd)
 {
 	struct sdebug_scsi_cmd *sdsc = scsi_cmd_priv(cmnd);
@@ -6668,18 +6668,18 @@  static bool scsi_debug_stop_cmnd(struct scsi_cmnd *cmnd)
 		int res = hrtimer_try_to_cancel(&sd_dp->hrt);
 
 		switch (res) {
-		case 0: /* Not active, it must have already run */
 		case -1: /* -1 It's executing the CB */
 			return false;
+		case 0: /* Not active, it must have already run */
 		case 1: /* Was active, we've now cancelled */
 		default:
 			return true;
 		}
 	} else if (defer_t == SDEB_DEFER_WQ) {
 		/* Cancel if pending */
-		if (cancel_work_sync(&sd_dp->ew.work))
+		if (cancel_work(&sd_dp->ew.work))
 			return true;
-		/* Was not pending, so it must have run */
+		/* callback may be running, so return false */
 		return false;
 	} else if (defer_t == SDEB_DEFER_POLL) {
 		return true;
@@ -6759,7 +6759,7 @@  static int sdebug_fail_abort(struct scsi_cmnd *cmnd)
 
 static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
 {
-	bool ok = scsi_debug_abort_cmnd(SCpnt);
+	bool aborted = scsi_debug_abort_cmnd(SCpnt);
 	u8 *cmd = SCpnt->cmnd;
 	u8 opcode = cmd[0];
 
@@ -6768,7 +6768,8 @@  static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
 	if (SDEBUG_OPT_ALL_NOISE & sdebug_opts)
 		sdev_printk(KERN_INFO, SCpnt->device,
 			    "%s: command%s found\n", __func__,
-			    ok ? "" : " not");
+			    aborted ? "" : " not");
+
 
 	if (sdebug_fail_abort(SCpnt)) {
 		scmd_printk(KERN_INFO, SCpnt, "fail abort command 0x%x\n",
@@ -6776,6 +6777,9 @@  static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
 		return FAILED;
 	}
 
+	if (aborted == false)
+		return FAILED;
+
 	return SUCCESS;
 }