aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTejun Heo2011-10-19 07:42:16 -0500
committerJens Axboe2011-10-19 07:42:16 -0500
commitc9a929dde3913780b5c416f4bb9d9ed804f509ce (patch)
tree1acadc374d8f1faebdf07f08fae0993a38a8fd0d /block/blk.h
parentbd87b5898a72b1aef6acf3705c61c9f6372adf0c (diff)
downloadkernel-common-c9a929dde3913780b5c416f4bb9d9ed804f509ce.tar.gz
kernel-common-c9a929dde3913780b5c416f4bb9d9ed804f509ce.tar.xz
kernel-common-c9a929dde3913780b5c416f4bb9d9ed804f509ce.zip
block: fix request_queue lifetime handling by making blk_queue_cleanup() properly shutdown
request_queue is refcounted but actually depdends on lifetime management from the queue owner - on blk_cleanup_queue(), block layer expects that there's no request passing through request_queue and no new one will. This is fundamentally broken. The queue owner (e.g. SCSI layer) doesn't have a way to know whether there are other active users before calling blk_cleanup_queue() and other users (e.g. bsg) don't have any guarantee that the queue is and would stay valid while it's holding a reference. With delay added in blk_queue_bio() before queue_lock is grabbed, the following oops can be easily triggered when a device is removed with in-flight IOs. sd 0:0:1:0: [sdb] Stopping disk ata1.01: disabled general protection fault: 0000 [#1] PREEMPT SMP CPU 2 Modules linked in: Pid: 648, comm: test_rawio Not tainted 3.1.0-rc3-work+ #56 Bochs Bochs RIP: 0010:[<ffffffff8137d651>] [<ffffffff8137d651>] elv_rqhash_find+0x61/0x100 ... Process test_rawio (pid: 648, threadinfo ffff880019efa000, task ffff880019ef8a80) ... Call Trace: [<ffffffff8137d774>] elv_merge+0x84/0xe0 [<ffffffff81385b54>] blk_queue_bio+0xf4/0x400 [<ffffffff813838ea>] generic_make_request+0xca/0x100 [<ffffffff81383994>] submit_bio+0x74/0x100 [<ffffffff811c53ec>] dio_bio_submit+0xbc/0xc0 [<ffffffff811c610e>] __blockdev_direct_IO+0x92e/0xb40 [<ffffffff811c39f7>] blkdev_direct_IO+0x57/0x60 [<ffffffff8113b1c5>] generic_file_aio_read+0x6d5/0x760 [<ffffffff8118c1ca>] do_sync_read+0xda/0x120 [<ffffffff8118ce55>] vfs_read+0xc5/0x180 [<ffffffff8118cfaa>] sys_pread64+0x9a/0xb0 [<ffffffff81afaf6b>] system_call_fastpath+0x16/0x1b This happens because blk_queue_cleanup() destroys the queue and elevator whether IOs are in progress or not and DEAD tests are sprinkled in the request processing path without proper synchronization. Similar problem exists for blk-throtl. On queue cleanup, blk-throtl is shutdown whether it has requests in it or not. Depending on timing, it either oopses or throttled bios are lost putting tasks which are waiting for bio completion into eternal D state. The way it should work is having the usual clear distinction between shutdown and release. Shutdown drains all currently pending requests, marks the queue dead, and performs partial teardown of the now unnecessary part of the queue. Even after shutdown is complete, reference holders are still allowed to issue requests to the queue although they will be immmediately failed. The rest of teardown happens on release. This patch makes the following changes to make blk_queue_cleanup() behave as proper shutdown. * QUEUE_FLAG_DEAD is now set while holding both q->exit_mutex and queue_lock. * Unsynchronized DEAD check in generic_make_request_checks() removed. This couldn't make any meaningful difference as the queue could die after the check. * blk_drain_queue() updated such that it can drain all requests and is now called during cleanup. * blk_throtl updated such that it checks DEAD on grabbing queue_lock, drains all throttled bios during cleanup and free td when queue is released. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Vivek Goyal <vgoyal@redhat.com> Signed-off-by: Jens Axboe <axboe@kernel.dk>
Diffstat (limited to 'block/blk.h')
-rw-r--r--block/blk.h6
1 files changed, 5 insertions, 1 deletions
diff --git a/block/blk.h b/block/blk.h
index c018dba4e33..3f6551b3c92 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -15,7 +15,7 @@ void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
15 struct bio *bio); 15 struct bio *bio);
16int blk_rq_append_bio(struct request_queue *q, struct request *rq, 16int blk_rq_append_bio(struct request_queue *q, struct request *rq,
17 struct bio *bio); 17 struct bio *bio);
18void blk_drain_queue(struct request_queue *q); 18void blk_drain_queue(struct request_queue *q, bool drain_all);
19void blk_dequeue_request(struct request *rq); 19void blk_dequeue_request(struct request *rq);
20void __blk_queue_free_tags(struct request_queue *q); 20void __blk_queue_free_tags(struct request_queue *q);
21bool __blk_end_bidi_request(struct request *rq, int error, 21bool __blk_end_bidi_request(struct request *rq, int error,
@@ -191,15 +191,19 @@ static inline int blk_do_io_stat(struct request *rq)
191 191
192#ifdef CONFIG_BLK_DEV_THROTTLING 192#ifdef CONFIG_BLK_DEV_THROTTLING
193extern bool blk_throtl_bio(struct request_queue *q, struct bio *bio); 193extern bool blk_throtl_bio(struct request_queue *q, struct bio *bio);
194extern void blk_throtl_drain(struct request_queue *q);
194extern int blk_throtl_init(struct request_queue *q); 195extern int blk_throtl_init(struct request_queue *q);
195extern void blk_throtl_exit(struct request_queue *q); 196extern void blk_throtl_exit(struct request_queue *q);
197extern void blk_throtl_release(struct request_queue *q);
196#else /* CONFIG_BLK_DEV_THROTTLING */ 198#else /* CONFIG_BLK_DEV_THROTTLING */
197static inline bool blk_throtl_bio(struct request_queue *q, struct bio *bio) 199static inline bool blk_throtl_bio(struct request_queue *q, struct bio *bio)
198{ 200{
199 return false; 201 return false;
200} 202}
203static inline void blk_throtl_drain(struct request_queue *q) { }
201static inline int blk_throtl_init(struct request_queue *q) { return 0; } 204static inline int blk_throtl_init(struct request_queue *q) { return 0; }
202static inline void blk_throtl_exit(struct request_queue *q) { } 205static inline void blk_throtl_exit(struct request_queue *q) { }
206static inline void blk_throtl_release(struct request_queue *q) { }
203#endif /* CONFIG_BLK_DEV_THROTTLING */ 207#endif /* CONFIG_BLK_DEV_THROTTLING */
204 208
205#endif /* BLK_INTERNAL_H */ 209#endif /* BLK_INTERNAL_H */