Re: [BUG] Oops when SCSI device under multipath is removed
From: Jun'ichi Nomura
Date: Wed Aug 10 2011 - 23:12:20 EST
On 08/11/11 09:24, Jun'ichi Nomura wrote:
> On 08/11/11 04:52, James Bottomley wrote:
>> On Wed, 2011-08-10 at 13:29 +0900, Jun'ichi Nomura wrote:
>>> 2) SCSI to call blk_cleanup_queue() from device's ->release() callback
>>> (before 2.6.39, it used to work like this)
>> Well, they both have documented objections. I asked why we destroy the
>> elevator in the del case and didn't get any traction, so let me show the
>> actual patch which should fix all of these issues.
>> Is there a good reason for not doing this as a bug fix now?
> I think it doesn't work because elevator_exit() and
> blk_throtl_exit() take &q->queue_lock, which may be freed
> by LLD after blk_cleanup_queue, before blk_release_queue.
If the reason you moved scsi_free_queue into scsi_remove_device
is marking the queue dead, how about the following patch?
Do you think it's acceptable?
Jun'ichi Nomura, NEC Corporation
Add blk_kill_queue() for drivers which want to mark the queue dead early.
blk_cleanup_queue() is an interface for LLD to notify block layer
that LLD no longer needs the queue.
Since q->queue_lock may point to a structure in LLD which is freed
after blk_cleanup_queue, blk_cleanup_queue() frees subordinate structures
like elevator, which uses q->queue_lock, to avoid invalid reference.
OTOH, LLD like SCSI wants to just mark the queue dead earlier in tear
So this patch factors out the early part of blk_cleanup_queue into
blk_kill_queue for such drivers.
--- linux-3.1-rc1/include/linux/blkdev.h.orig 2011-08-11 11:19:52.585280223 +0900
+++ linux-3.1-rc1/include/linux/blkdev.h 2011-08-11 11:20:09.482279763 +0900
@@ -804,6 +804,7 @@ extern struct request_queue *blk_init_al
extern struct request_queue *blk_init_queue(request_fn_proc *, spinlock_t *);
extern struct request_queue *blk_init_allocated_queue(struct request_queue *,
request_fn_proc *, spinlock_t *);
+extern void blk_kill_queue(struct request_queue *);
extern void blk_cleanup_queue(struct request_queue *);
extern void blk_queue_make_request(struct request_queue *, make_request_fn *);
extern void blk_queue_bounce_limit(struct request_queue *, u64);
--- linux-3.1-rc1/block/blk-core.c.orig 2011-08-10 09:46:06.014043123 +0900
+++ linux-3.1-rc1/block/blk-core.c 2011-08-11 11:19:34.551280697 +0900
@@ -347,6 +347,17 @@ void blk_put_queue(struct request_queue
+void blk_kill_queue(struct request_queue *q)
+ queue_flag_set_unlocked(QUEUE_FLAG_DEAD, q);
* Note: If a driver supplied the queue lock, it should not zap that lock
* unexpectedly as some queue cleanup components like elevator_exit() and
@@ -360,12 +371,7 @@ void blk_cleanup_queue(struct request_qu
* are done before moving on. Going into this function, we should
* not have processes doing IO to this device.
- queue_flag_set_unlocked(QUEUE_FLAG_DEAD, q);
--- linux-3.1-rc1/drivers/scsi/scsi_sysfs.c.orig 2011-08-09 18:48:13.676485115 +0900
+++ linux-3.1-rc1/drivers/scsi/scsi_sysfs.c 2011-08-11 11:21:07.923277456 +0900
@@ -322,6 +322,7 @@ static void scsi_device_dev_release_user
/* NULL queue means the device can't be used */
sdev->request_queue = NULL;
@@ -937,7 +938,7 @@ void __scsi_remove_device(struct scsi_de
sdev->request_queue->queuedata = NULL;
/* Freeing the queue signals to block that we're done */
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/