scsi_debug: deadlock between completions and surprise module removal

A deadlock has been reported when the completion
of SCSI commands (simulated by a timer) was surprised
by a module removal. This patch removes one half of
the offending locks around timer deletions. This fix
is applied both to stop_all_queued() which is were
the deadlock was discovered and stop_queued_cmnd()
which has very similar logic.

This patch should be applied both to the lk 3.17 tree
and Christoph's drivers-for-3.18 tree.

Tested-and-reported-by: Milan Broz <gmazyland@gmail.com>
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 1c475e9a46aba4ec822f6251877d3df44c42f480..2b6d447ad6d641723169cb45af42efdab5cb16be 100644 (file)
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2743,6 +2743,13 @@ static int stop_queued_cmnd(struct scsi_cmnd *cmnd)
                if (test_bit(k, queued_in_use_bm)) {
                        sqcp = &queued_arr[k];
                        if (cmnd == sqcp->a_cmnd) {
+                               devip = (struct sdebug_dev_info *)
+                                       cmnd->device->hostdata;
+                               if (devip)
+                                       atomic_dec(&devip->num_in_q);
+                               sqcp->a_cmnd = NULL;
+                               spin_unlock_irqrestore(&queued_arr_lock,
+                                                      iflags);
                                if (scsi_debug_ndelay > 0) {
                                        if (sqcp->sd_hrtp)
                                                hrtimer_cancel(
@@ -2755,18 +2762,13 @@ static int stop_queued_cmnd(struct scsi_cmnd *cmnd)
                                        if (sqcp->tletp)
                                                tasklet_kill(sqcp->tletp);
                                }
-                               __clear_bit(k, queued_in_use_bm);
-                               devip = (struct sdebug_dev_info *)
-                                       cmnd->device->hostdata;
-                               if (devip)
-                                       atomic_dec(&devip->num_in_q);
-                               sqcp->a_cmnd = NULL;
-                               break;
+                               clear_bit(k, queued_in_use_bm);
+                               return 1;
                        }
                }
        }
        spin_unlock_irqrestore(&queued_arr_lock, iflags);
-       return (k < qmax) ? 1 : 0;
+       return 0;
 }
 
 /* Deletes (stops) timers or tasklets of all queued commands */
@@ -2782,6 +2784,13 @@ static void stop_all_queued(void)
                if (test_bit(k, queued_in_use_bm)) {
                        sqcp = &queued_arr[k];
                        if (sqcp->a_cmnd) {
+                               devip = (struct sdebug_dev_info *)
+                                       sqcp->a_cmnd->device->hostdata;
+                               if (devip)
+                                       atomic_dec(&devip->num_in_q);
+                               sqcp->a_cmnd = NULL;
+                               spin_unlock_irqrestore(&queued_arr_lock,
+                                                      iflags);
                                if (scsi_debug_ndelay > 0) {
                                        if (sqcp->sd_hrtp)
                                                hrtimer_cancel(
@@ -2794,12 +2803,8 @@ static void stop_all_queued(void)
                                        if (sqcp->tletp)
                                                tasklet_kill(sqcp->tletp);
                                }
-                               __clear_bit(k, queued_in_use_bm);
-                               devip = (struct sdebug_dev_info *)
-                                       sqcp->a_cmnd->device->hostdata;
-                               if (devip)
-                                       atomic_dec(&devip->num_in_q);
-                               sqcp->a_cmnd = NULL;
+                               clear_bit(k, queued_in_use_bm);
+                               spin_lock_irqsave(&queued_arr_lock, iflags);
                        }
                }
        }