Re: Strange block/scsi/workqueue issue

[Steven Whitehouse]

Hi,

On Tue, 2011-04-12 at 14:56 -0500, James Bottomley wrote:
> On Tue, 2011-04-12 at 19:33 +0100, Steven Whitehouse wrote:
> > Hi,
> > 
> > On Tue, 2011-04-12 at 12:41 -0500, James Bottomley wrote:
> > > On Tue, 2011-04-12 at 17:51 +0100, Steven Whitehouse wrote:
> > > > Still not quite there, but looking more hopeful now,
> > > 
> > > Not sure I share your optimism; but this one
> > > 
> > Neither do I any more :-) Looks like we are back in blk_peek_request()
> > again. 
> [...]
> >                 if (!q->elevator->ops || !q->elevator->ops->elevator_dispatch_fn
> > (q, 0))
> >     6d62:       49 8b 44 24 18          mov    0x18(%r12),%rax
> >     6d67:       48 8b 00                mov    (%rax),%rax
> >     6d6a:       48 85 c0                test   %rax,%rax
> >     6d6d:       74 0c                   je     6d7b <blk_peek_request+0x11b>
> >     6d6f:       31 f6                   xor    %esi,%esi
> >     6d71:       4c 89 e7                mov    %r12,%rdi                                <----- here
> >     6d74:       ff 50 28                callq  *0x28(%rax)
> >     6d77:       85 c0                   test   %eax,%eax
> >     6d79:       75 da                   jne    6d55 <blk_peek_request+0xf5>
> >     6d7b:       45 31 ed                xor    %r13d,%r13d
> 
> Hmm, wrong signal for no elevator then.  How about this?
> 
> James
> 
That seems to do the trick... the box has been booted for several
minutes now and no sign of anything untoward so far :-)

Below is the cumulative patch which I now have applied to the kernel.
Many thanks for all your help in debugging this, its greatly
appreciated :-)

Steve.

diff --git a/block/blk-core.c b/block/blk-core.c
index 90f22cc..7f15eb7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -219,6 +219,7 @@ static void blk_delay_work(struct work_struct *work)
 	spin_lock_irq(q->queue_lock);
 	__blk_run_queue(q, false);
 	spin_unlock_irq(q->queue_lock);
+	blk_put_queue(q);
 }
 
 /**
@@ -233,7 +234,8 @@ static void blk_delay_work(struct work_struct *work)
  */
 void blk_delay_queue(struct request_queue *q, unsigned long msecs)
 {
-	schedule_delayed_work(&q->delay_work, msecs_to_jiffies(msecs));
+	if (!blk_get_queue(q))
+		schedule_delayed_work(&q->delay_work, msecs_to_jiffies(msecs));
 }
 EXPORT_SYMBOL(blk_delay_queue);
 
@@ -271,7 +273,8 @@ EXPORT_SYMBOL(blk_start_queue);
  **/
 void blk_stop_queue(struct request_queue *q)
 {
-	__cancel_delayed_work(&q->delay_work);
+	if (__cancel_delayed_work(&q->delay_work))
+		blk_put_queue(q);
 	queue_flag_set(QUEUE_FLAG_STOPPED, q);
 }
 EXPORT_SYMBOL(blk_stop_queue);
@@ -297,7 +300,8 @@ EXPORT_SYMBOL(blk_stop_queue);
 void blk_sync_queue(struct request_queue *q)
 {
 	del_timer_sync(&q->timeout);
-	cancel_delayed_work_sync(&q->delay_work);
+	if (__cancel_delayed_work(&q->delay_work))
+		blk_put_queue(q);
 	queue_sync_plugs(q);
 }
 EXPORT_SYMBOL(blk_sync_queue);
@@ -324,7 +328,7 @@ void __blk_run_queue(struct request_queue *q, bool force_kblockd)
 	if (!force_kblockd && !queue_flag_test_and_set(QUEUE_FLAG_REENTER, q)) {
 		q->request_fn(q);
 		queue_flag_clear(QUEUE_FLAG_REENTER, q);
-	} else
+	} else if (!blk_get_queue(q))
 		queue_delayed_work(kblockd_workqueue, &q->delay_work, 0);
 }
 EXPORT_SYMBOL(__blk_run_queue);
diff --git a/block/blk.h b/block/blk.h
index 6126346..4df474d 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -62,7 +62,8 @@ static inline struct request *__elv_next_request(struct request_queue *q)
 			return rq;
 		}
 
-		if (!q->elevator->ops->elevator_dispatch_fn(q, 0))
+		if (test_bit(QUEUE_FLAG_DEAD, &q->queue_flags) ||
+		    !q->elevator->ops->elevator_dispatch_fn(q, 0))
 			return NULL;
 	}
 }
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index e44ff64..2e85668 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -323,7 +323,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	}
 
 	if (sdev->request_queue) {
-		sdev->request_queue->queuedata = NULL;
 		/* user context needed to free queue */
 		scsi_free_queue(sdev->request_queue);
 		/* temporary expedient, try to catch use of queue lock
@@ -937,6 +936,7 @@ void __scsi_remove_device(struct scsi_device *sdev)
 	if (sdev->host->hostt->slave_destroy)
 		sdev->host->hostt->slave_destroy(sdev);
 	transport_destroy_device(dev);
+	sdev->request_queue->queuedata = NULL;
 	put_device(dev);
 }
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/