summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJens Axboe <axboe@fb.com>2017-02-03 09:48:28 -0700
committerJens Axboe <axboe@fb.com>2017-02-03 09:48:28 -0700
commite4d750c97794ea2bab793d4c518b1f4006364588 (patch)
tree8b351d08b1d81986402964a243268e70bb31a6a9
parentb973cb7e89fe3dcc2bc72c5b3aa7a3bfd9d0e6d5 (diff)
downloadlinux-0-day-e4d750c97794ea2bab793d4c518b1f4006364588.tar.gz
block: free merged request in the caller
If we end up doing a request-to-request merge when we have completed a bio-to-request merge, we free the request from deep down in that path. For blk-mq-sched, the merge path has to hold the appropriate lock, but we don't need it for freeing the request. And in fact holding the lock is problematic, since we are now calling the mq sched put_rq_private() hook with the lock held. Other call paths do not hold this lock. Fix this inconsistency by ensuring that the caller frees a merged request. Then we can do it outside of the lock, making it both more efficient and fixing the blk-mq-sched problem of invoking parts of the scheduler with an unknown lock state. Reported-by: Paolo Valente <paolo.valente@linaro.org> Signed-off-by: Jens Axboe <axboe@fb.com> Reviewed-by: Omar Sandoval <osandov@fb.com>
-rw-r--r--block/blk-core.c12
-rw-r--r--block/blk-merge.c15
-rw-r--r--block/blk-mq-sched.c9
-rw-r--r--block/blk-mq-sched.h3
-rw-r--r--block/mq-deadline.c8
5 files changed, 35 insertions, 12 deletions
diff --git a/block/blk-core.c b/block/blk-core.c
index 76c42f5..1d26331 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1596,7 +1596,7 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
{
struct blk_plug *plug;
int el_ret, where = ELEVATOR_INSERT_SORT;
- struct request *req;
+ struct request *req, *free;
unsigned int request_count = 0;
unsigned int wb_acct;
@@ -1637,15 +1637,21 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
if (el_ret == ELEVATOR_BACK_MERGE) {
if (bio_attempt_back_merge(q, req, bio)) {
elv_bio_merged(q, req, bio);
- if (!attempt_back_merge(q, req))
+ free = attempt_back_merge(q, req);
+ if (!free)
elv_merged_request(q, req, el_ret);
+ else
+ __blk_put_request(q, free);
goto out_unlock;
}
} else if (el_ret == ELEVATOR_FRONT_MERGE) {
if (bio_attempt_front_merge(q, req, bio)) {
elv_bio_merged(q, req, bio);
- if (!attempt_front_merge(q, req))
+ free = attempt_front_merge(q, req);
+ if (!free)
elv_merged_request(q, req, el_ret);
+ else
+ __blk_put_request(q, free);
goto out_unlock;
}
}
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 3826fc3..a373416 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -733,9 +733,11 @@ static struct request *attempt_merge(struct request_queue *q,
if (blk_rq_cpu_valid(next))
req->cpu = next->cpu;
- /* owner-ship of bio passed from next to req */
+ /*
+ * ownership of bio passed from next to req, return 'next' for
+ * the caller to free
+ */
next->bio = NULL;
- __blk_put_request(q, next);
return next;
}
@@ -763,12 +765,19 @@ int blk_attempt_req_merge(struct request_queue *q, struct request *rq,
struct request *next)
{
struct elevator_queue *e = q->elevator;
+ struct request *free;
if (!e->uses_mq && e->type->ops.sq.elevator_allow_rq_merge_fn)
if (!e->type->ops.sq.elevator_allow_rq_merge_fn(q, rq, next))
return 0;
- return attempt_merge(q, rq, next) != NULL;
+ free = attempt_merge(q, rq, next);
+ if (free) {
+ __blk_put_request(q, free);
+ return 1;
+ }
+
+ return 0;
}
bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 3ec52f4..ee455e7 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -234,7 +234,8 @@ void blk_mq_sched_move_to_dispatch(struct blk_mq_hw_ctx *hctx,
}
EXPORT_SYMBOL_GPL(blk_mq_sched_move_to_dispatch);
-bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio)
+bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
+ struct request **merged_request)
{
struct request *rq;
int ret;
@@ -244,7 +245,8 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio)
if (!blk_mq_sched_allow_merge(q, rq, bio))
return false;
if (bio_attempt_back_merge(q, rq, bio)) {
- if (!attempt_back_merge(q, rq))
+ *merged_request = attempt_back_merge(q, rq);
+ if (!*merged_request)
elv_merged_request(q, rq, ret);
return true;
}
@@ -252,7 +254,8 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio)
if (!blk_mq_sched_allow_merge(q, rq, bio))
return false;
if (bio_attempt_front_merge(q, rq, bio)) {
- if (!attempt_front_merge(q, rq))
+ *merged_request = attempt_front_merge(q, rq);
+ if (!*merged_request)
elv_merged_request(q, rq, ret);
return true;
}
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index add5f09..5954859 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -15,7 +15,8 @@ struct request *blk_mq_sched_get_request(struct request_queue *q, struct bio *bi
void blk_mq_sched_put_request(struct request *rq);
void blk_mq_sched_request_inserted(struct request *rq);
-bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio);
+bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
+ struct request **merged_request);
bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio);
bool blk_mq_sched_try_insert_merge(struct request_queue *q, struct request *rq);
void blk_mq_sched_restart_queues(struct blk_mq_hw_ctx *hctx);
diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 8f91f21..d68d9c2 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -371,12 +371,16 @@ static bool dd_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio)
{
struct request_queue *q = hctx->queue;
struct deadline_data *dd = q->elevator->elevator_data;
- int ret;
+ struct request *free = NULL;
+ bool ret;
spin_lock(&dd->lock);
- ret = blk_mq_sched_try_merge(q, bio);
+ ret = blk_mq_sched_try_merge(q, bio, &free);
spin_unlock(&dd->lock);
+ if (free)
+ blk_mq_free_request(free);
+
return ret;
}