scsi: scsi_debug: Do not sleep in atomic sections

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>
Link: https://lore.kernel.org/r/20250224115517.495899-5-john.g.garry@oracle.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c1a217b5aac2682e5953758bb7235a1f3cd5dd7f..2208dcba346edc41a6be58ea41cec30e81d295cb 100644 (file)
--- 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;
 }