* [PATCH v2] blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash
@ 2018-04-10 20:45 Bart Van Assche
2018-04-10 22:35 ` Jens Axboe
2018-04-11 2:12 ` Ming Lei
0 siblings, 2 replies; 4+ messages in thread
From: Bart Van Assche @ 2018-04-10 20:45 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, Ming Lei, Joseph Qi
Because blkcg_exit_queue() is now called from inside blk_cleanup_queue()
it is no longer safe to access cgroup information during or after the
blk_cleanup_queue() call. Hence protect the generic_make_request_checks()
call with blk_queue_enter() / blk_queue_exit().
Reported-by: Ming Lei <ming.lei@redhat.com>
Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Joseph Qi <joseph.qi@linux.alibaba.com>
---
Changes compared to v1: changed the blk_queue_exit() inside the loop with "if (q)".
block/blk-core.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 34e2f2227fd9..181b1a688a5b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2386,8 +2386,19 @@ blk_qc_t generic_make_request(struct bio *bio)
* yet.
*/
struct bio_list bio_list_on_stack[2];
+ blk_mq_req_flags_t flags = bio->bi_opf & REQ_NOWAIT ?
+ BLK_MQ_REQ_NOWAIT : 0;
+ struct request_queue *q = bio->bi_disk->queue;
blk_qc_t ret = BLK_QC_T_NONE;
+ if (blk_queue_enter(q, flags) < 0) {
+ if (!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT))
+ bio_wouldblock_error(bio);
+ else
+ bio_io_error(bio);
+ return ret;
+ }
+
if (!generic_make_request_checks(bio))
goto out;
@@ -2424,11 +2435,21 @@ blk_qc_t generic_make_request(struct bio *bio)
bio_list_init(&bio_list_on_stack[0]);
current->bio_list = bio_list_on_stack;
do {
- struct request_queue *q = bio->bi_disk->queue;
- blk_mq_req_flags_t flags = bio->bi_opf & REQ_NOWAIT ?
- BLK_MQ_REQ_NOWAIT : 0;
+ bool enter_succeeded = true;
+
+ if (unlikely(q != bio->bi_disk->queue)) {
+ if (q)
+ blk_queue_exit(q);
+ q = bio->bi_disk->queue;
+ flags = bio->bi_opf & REQ_NOWAIT ? BLK_MQ_REQ_NOWAIT :
+ 0;
+ if (blk_queue_enter(q, flags) < 0) {
+ enter_succeeded = false;
+ q = NULL;
+ }
+ }
- if (likely(blk_queue_enter(q, flags) == 0)) {
+ if (enter_succeeded) {
struct bio_list lower, same;
/* Create a fresh bio_list for all subordinate requests */
@@ -2436,8 +2457,6 @@ blk_qc_t generic_make_request(struct bio *bio)
bio_list_init(&bio_list_on_stack[0]);
ret = q->make_request_fn(q, bio);
- blk_queue_exit(q);
-
/* sort new bios into those for a lower level
* and those for the same level
*/
@@ -2464,6 +2483,8 @@ blk_qc_t generic_make_request(struct bio *bio)
current->bio_list = NULL; /* deactivate */
out:
+ if (q)
+ blk_queue_exit(q);
return ret;
}
EXPORT_SYMBOL(generic_make_request);
--
2.16.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash
2018-04-10 20:45 [PATCH v2] blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash Bart Van Assche
@ 2018-04-10 22:35 ` Jens Axboe
2018-04-11 2:12 ` Ming Lei
1 sibling, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2018-04-10 22:35 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-block, Christoph Hellwig, Ming Lei, Joseph Qi
On 4/10/18 2:45 PM, Bart Van Assche wrote:
> Because blkcg_exit_queue() is now called from inside blk_cleanup_queue()
> it is no longer safe to access cgroup information during or after the
> blk_cleanup_queue() call. Hence protect the generic_make_request_checks()
> call with blk_queue_enter() / blk_queue_exit().
This looks better. But can we please get rid of the ternary? I hate it
with a vengeance, especially when it ends up spanning multiple lines.
This:
flags = 0;
if (bio->bi_opf & REQ_NOWAIT)
flags = BLK_MQ_REQ_NOWAIT;
is so much more readable.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash
2018-04-10 20:45 [PATCH v2] blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash Bart Van Assche
2018-04-10 22:35 ` Jens Axboe
@ 2018-04-11 2:12 ` Ming Lei
2018-04-11 2:22 ` Bart Van Assche
1 sibling, 1 reply; 4+ messages in thread
From: Ming Lei @ 2018-04-11 2:12 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Christoph Hellwig, Joseph Qi
On Tue, Apr 10, 2018 at 02:45:54PM -0600, Bart Van Assche wrote:
> Because blkcg_exit_queue() is now called from inside blk_cleanup_queue()
> it is no longer safe to access cgroup information during or after the
> blk_cleanup_queue() call. Hence protect the generic_make_request_checks()
> call with blk_queue_enter() / blk_queue_exit().
>
> Reported-by: Ming Lei <ming.lei@redhat.com>
> Fixes: a063057d7c73 ("block: Fix a race between request queue removal and the block cgroup controller")
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>
> Changes compared to v1: changed the blk_queue_exit() inside the loop with "if (q)".
>
> block/blk-core.c | 33 +++++++++++++++++++++++++++------
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 34e2f2227fd9..181b1a688a5b 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2386,8 +2386,19 @@ blk_qc_t generic_make_request(struct bio *bio)
> * yet.
> */
> struct bio_list bio_list_on_stack[2];
> + blk_mq_req_flags_t flags = bio->bi_opf & REQ_NOWAIT ?
> + BLK_MQ_REQ_NOWAIT : 0;
> + struct request_queue *q = bio->bi_disk->queue;
> blk_qc_t ret = BLK_QC_T_NONE;
>
> + if (blk_queue_enter(q, flags) < 0) {
Same issue with V1, the queue pointer has to be checked before calling
blk_queue_enter().
--
Ming
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash
2018-04-11 2:12 ` Ming Lei
@ 2018-04-11 2:22 ` Bart Van Assche
0 siblings, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2018-04-11 2:22 UTC (permalink / raw)
To: ming.lei; +Cc: hch, linux-block, axboe, joseph.qi
T24gV2VkLCAyMDE4LTA0LTExIGF0IDEwOjEyICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gT24g
VHVlLCBBcHIgMTAsIDIwMTggYXQgMDI6NDU6NTRQTSAtMDYwMCwgQmFydCBWYW4gQXNzY2hlIHdy
b3RlOg0KPiA+ICsJc3RydWN0IHJlcXVlc3RfcXVldWUgKnEgPSBiaW8tPmJpX2Rpc2stPnF1ZXVl
Ow0KPiA+ICAJYmxrX3FjX3QgcmV0ID0gQkxLX1FDX1RfTk9ORTsNCj4gPiAgDQo+ID4gKwlpZiAo
YmxrX3F1ZXVlX2VudGVyKHEsIGZsYWdzKSA8IDApIHsNCj4gDQo+IFNhbWUgaXNzdWUgd2l0aCBW
MSwgdGhlIHF1ZXVlIHBvaW50ZXIgaGFzIHRvIGJlIGNoZWNrZWQgYmVmb3JlIGNhbGxpbmcNCj4g
YmxrX3F1ZXVlX2VudGVyKCkuDQoNCkkgdGhpbmsgaXQncyB0aGUgcmVzcG9uc2liaWxpdHkgb2Yg
dGhlIGNhbGxlciB0byBrZWVwIGEgcmVmZXJlbmNlIG9uIHRoZQ0KcmVxdWVzdCBxdWV1ZS4NCg0K
QmFydC4NCg0KDQo=
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-11 2:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-10 20:45 [PATCH v2] blk-mq: Avoid that submitting a bio concurrently with device removal triggers a crash Bart Van Assche
2018-04-10 22:35 ` Jens Axboe
2018-04-11 2:12 ` Ming Lei
2018-04-11 2:22 ` Bart Van Assche
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.