Skip to content

Commit ac0fb4a

Browse files
bvanasschemartinkpetersen
authored andcommitted
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 <[email protected]> jpg: return FAILED from scsi_debug_abort() when possible callback running Signed-off-by: John Garry <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Martin K. Petersen <[email protected]>
1 parent b441eaf commit ac0fb4a

File tree

1 file changed

+10
-6
lines changed

1 file changed

+10
-6
lines changed

drivers/scsi/scsi_debug.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6655,7 +6655,7 @@ static void scsi_debug_sdev_destroy(struct scsi_device *sdp)
66556655
sdp->hostdata = NULL;
66566656
}
66576657

6658-
/* Returns true if it is safe to complete @cmnd. */
6658+
/* Returns true if cancelled or not running callback. */
66596659
static bool scsi_debug_stop_cmnd(struct scsi_cmnd *cmnd)
66606660
{
66616661
struct sdebug_scsi_cmd *sdsc = scsi_cmd_priv(cmnd);
@@ -6668,18 +6668,18 @@ static bool scsi_debug_stop_cmnd(struct scsi_cmnd *cmnd)
66686668
int res = hrtimer_try_to_cancel(&sd_dp->hrt);
66696669

66706670
switch (res) {
6671-
case 0: /* Not active, it must have already run */
66726671
case -1: /* -1 It's executing the CB */
66736672
return false;
6673+
case 0: /* Not active, it must have already run */
66746674
case 1: /* Was active, we've now cancelled */
66756675
default:
66766676
return true;
66776677
}
66786678
} else if (defer_t == SDEB_DEFER_WQ) {
66796679
/* Cancel if pending */
6680-
if (cancel_work_sync(&sd_dp->ew.work))
6680+
if (cancel_work(&sd_dp->ew.work))
66816681
return true;
6682-
/* Was not pending, so it must have run */
6682+
/* callback may be running, so return false */
66836683
return false;
66846684
} else if (defer_t == SDEB_DEFER_POLL) {
66856685
return true;
@@ -6759,7 +6759,7 @@ static int sdebug_fail_abort(struct scsi_cmnd *cmnd)
67596759

67606760
static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
67616761
{
6762-
bool ok = scsi_debug_abort_cmnd(SCpnt);
6762+
bool aborted = scsi_debug_abort_cmnd(SCpnt);
67636763
u8 *cmd = SCpnt->cmnd;
67646764
u8 opcode = cmd[0];
67656765

@@ -6768,14 +6768,18 @@ static int scsi_debug_abort(struct scsi_cmnd *SCpnt)
67686768
if (SDEBUG_OPT_ALL_NOISE & sdebug_opts)
67696769
sdev_printk(KERN_INFO, SCpnt->device,
67706770
"%s: command%s found\n", __func__,
6771-
ok ? "" : " not");
6771+
aborted ? "" : " not");
6772+
67726773

67736774
if (sdebug_fail_abort(SCpnt)) {
67746775
scmd_printk(KERN_INFO, SCpnt, "fail abort command 0x%x\n",
67756776
opcode);
67766777
return FAILED;
67776778
}
67786779

6780+
if (aborted == false)
6781+
return FAILED;
6782+
67796783
return SUCCESS;
67806784
}
67816785

0 commit comments

Comments
 (0)