* [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
@ 2017-02-27 17:47 Omar Sandoval
2017-02-27 17:47 ` [PATCH 2/3] blk-mq: kill blk_mq_set_alloc_data() Omar Sandoval
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Omar Sandoval @ 2017-02-27 17:47 UTC (permalink / raw)
To: Jens Axboe, Sagi Grimberg, linux-block; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
its blk_mq_alloc_request() counterpart. It also crashes because it
doesn't update the tags->rqs map.
Fix it by making it allocate a scheduler request.
Reported-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
I think this should do it. Verified that normal operation still works
for me, but I'm not sure how to test the actual _hctx() alloc path.
Sagi, could you please test this series out?
block/blk-mq-sched.c | 11 +++++------
block/blk-mq.c | 34 +++++++++++++++-------------------
2 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 46ca965fff5c..5697b23412a1 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -110,15 +110,14 @@ struct request *blk_mq_sched_get_request(struct request_queue *q,
struct blk_mq_alloc_data *data)
{
struct elevator_queue *e = q->elevator;
- struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
struct request *rq;
blk_queue_enter_live(q);
- ctx = blk_mq_get_ctx(q);
- hctx = blk_mq_map_queue(q, ctx->cpu);
-
- blk_mq_set_alloc_data(data, q, data->flags, ctx, hctx);
+ data->q = q;
+ if (likely(!data->ctx))
+ data->ctx = blk_mq_get_ctx(q);
+ if (likely(!data->hctx))
+ data->hctx = blk_mq_map_queue(q, data->ctx->cpu);
if (e) {
data->flags |= BLK_MQ_REQ_INTERNAL;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9611cd9920e9..cc9713f574a5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -273,10 +273,9 @@ EXPORT_SYMBOL(blk_mq_alloc_request);
struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
unsigned int flags, unsigned int hctx_idx)
{
- struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
+ struct blk_mq_alloc_data alloc_data = { .flags = flags };
struct request *rq;
- struct blk_mq_alloc_data alloc_data;
+ unsigned int cpu;
int ret;
/*
@@ -299,26 +298,23 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
* Check if the hardware context is actually mapped to anything.
* If not tell the caller that it should skip this queue.
*/
- hctx = q->queue_hw_ctx[hctx_idx];
- if (!blk_mq_hw_queue_mapped(hctx)) {
- ret = -EXDEV;
- goto out_queue_exit;
+ alloc_data.hctx = q->queue_hw_ctx[hctx_idx];
+ if (!blk_mq_hw_queue_mapped(alloc_data.hctx)) {
+ blk_queue_exit(q);
+ return ERR_PTR(-EXDEV);
}
- ctx = __blk_mq_get_ctx(q, cpumask_first(hctx->cpumask));
+ cpu = cpumask_first(alloc_data.hctx->cpumask);
+ alloc_data.ctx = __blk_mq_get_ctx(q, cpu);
- blk_mq_set_alloc_data(&alloc_data, q, flags, ctx, hctx);
- rq = __blk_mq_alloc_request(&alloc_data, rw);
- if (!rq) {
- ret = -EWOULDBLOCK;
- goto out_queue_exit;
- }
- alloc_data.hctx->tags->rqs[rq->tag] = rq;
-
- return rq;
+ rq = blk_mq_sched_get_request(q, NULL, rw, &alloc_data);
-out_queue_exit:
+ blk_mq_put_ctx(alloc_data.ctx);
blk_queue_exit(q);
- return ERR_PTR(ret);
+
+ if (!rq)
+ return ERR_PTR(-EWOULDBLOCK);
+
+ return rq;
}
EXPORT_SYMBOL_GPL(blk_mq_alloc_request_hctx);
--
2.12.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] blk-mq: kill blk_mq_set_alloc_data()
2017-02-27 17:47 [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
@ 2017-02-27 17:47 ` Omar Sandoval
2017-02-27 17:47 ` [PATCH 3/3] blk-mq: move update of tags->rqs to __blk_mq_alloc_request() Omar Sandoval
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Omar Sandoval @ 2017-02-27 17:47 UTC (permalink / raw)
To: Jens Axboe, Sagi Grimberg, linux-block; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
Nothing is using it anymore.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
block/blk-mq.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 24b2256186f3..088ced003c13 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -146,16 +146,6 @@ struct blk_mq_alloc_data {
struct blk_mq_hw_ctx *hctx;
};
-static inline void blk_mq_set_alloc_data(struct blk_mq_alloc_data *data,
- struct request_queue *q, unsigned int flags,
- struct blk_mq_ctx *ctx, struct blk_mq_hw_ctx *hctx)
-{
- data->q = q;
- data->flags = flags;
- data->ctx = ctx;
- data->hctx = hctx;
-}
-
static inline struct blk_mq_tags *blk_mq_tags_from_data(struct blk_mq_alloc_data *data)
{
if (data->flags & BLK_MQ_REQ_INTERNAL)
--
2.12.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] blk-mq: move update of tags->rqs to __blk_mq_alloc_request()
2017-02-27 17:47 [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
2017-02-27 17:47 ` [PATCH 2/3] blk-mq: kill blk_mq_set_alloc_data() Omar Sandoval
@ 2017-02-27 17:47 ` Omar Sandoval
2017-02-27 17:53 ` [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Omar Sandoval @ 2017-02-27 17:47 UTC (permalink / raw)
To: Jens Axboe, Sagi Grimberg, linux-block; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
No functional difference, it just makes a little more sense to update
the tag map where we actually allocate the tag.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
Optional cleanup, since there's only one place that does this.
block/blk-mq-sched.c | 2 --
block/blk-mq.c | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 5697b23412a1..09af8ff18719 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -134,8 +134,6 @@ struct request *blk_mq_sched_get_request(struct request_queue *q,
rq = __blk_mq_alloc_request(data, op);
} else {
rq = __blk_mq_alloc_request(data, op);
- if (rq)
- data->hctx->tags->rqs[rq->tag] = rq;
}
if (rq) {
diff --git a/block/blk-mq.c b/block/blk-mq.c
index cc9713f574a5..452c1caf978f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -234,6 +234,7 @@ struct request *__blk_mq_alloc_request(struct blk_mq_alloc_data *data,
}
rq->tag = tag;
rq->internal_tag = -1;
+ data->hctx->tags->rqs[rq->tag] = rq;
}
blk_mq_rq_ctx_init(data->q, data->ctx, rq, op);
--
2.12.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 17:47 [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
2017-02-27 17:47 ` [PATCH 2/3] blk-mq: kill blk_mq_set_alloc_data() Omar Sandoval
2017-02-27 17:47 ` [PATCH 3/3] blk-mq: move update of tags->rqs to __blk_mq_alloc_request() Omar Sandoval
@ 2017-02-27 17:53 ` Omar Sandoval
2017-02-27 18:02 ` Sagi Grimberg
2017-02-27 18:07 ` Jens Axboe
2017-02-27 18:28 ` [PATCH v2] " Omar Sandoval
4 siblings, 1 reply; 11+ messages in thread
From: Omar Sandoval @ 2017-02-27 17:53 UTC (permalink / raw)
To: Jens Axboe, Sagi Grimberg, linux-block; +Cc: kernel-team
[-- Attachment #1: Type: text/plain, Size: 809 bytes --]
On Mon, Feb 27, 2017 at 09:47:53AM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
> its blk_mq_alloc_request() counterpart. It also crashes because it
> doesn't update the tags->rqs map.
>
> Fix it by making it allocate a scheduler request.
>
> Reported-by: Sagi Grimberg <sagi@grimberg.me>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> I think this should do it. Verified that normal operation still works
> for me, but I'm not sure how to test the actual _hctx() alloc path.
> Sagi, could you please test this series out?
Hm, Sagi, your mail server seems to have rejected patches 2 and 3
because git-send-email duplicated the in-reply-to header for some
reason. I've attached the patches instead.
[-- Attachment #2: 0001-blk-mq-make-blk_mq_alloc_request_hctx-allocate-a-sch.patch --]
[-- Type: text/x-diff, Size: 3435 bytes --]
>From dcfe49f597dc1eec3b326024c86a6ad3afb82fa8 Mon Sep 17 00:00:00 2001
Message-Id: <dcfe49f597dc1eec3b326024c86a6ad3afb82fa8.1488217516.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Date: Mon, 27 Feb 2017 09:34:20 -0800
Subject: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a
scheduler request
blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
its blk_mq_alloc_request() counterpart. It also crashes because it
doesn't update the tags->rqs map.
Fix it by making it allocate a scheduler request.
Reported-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
I think this should do it. Verified that normal operation still works
for me, but I'm not sure how to test the actual _hctx() alloc path.
Sagi, could you please test this series out?
block/blk-mq-sched.c | 11 +++++------
block/blk-mq.c | 34 +++++++++++++++-------------------
2 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 46ca965fff5c..5697b23412a1 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -110,15 +110,14 @@ struct request *blk_mq_sched_get_request(struct request_queue *q,
struct blk_mq_alloc_data *data)
{
struct elevator_queue *e = q->elevator;
- struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
struct request *rq;
blk_queue_enter_live(q);
- ctx = blk_mq_get_ctx(q);
- hctx = blk_mq_map_queue(q, ctx->cpu);
-
- blk_mq_set_alloc_data(data, q, data->flags, ctx, hctx);
+ data->q = q;
+ if (likely(!data->ctx))
+ data->ctx = blk_mq_get_ctx(q);
+ if (likely(!data->hctx))
+ data->hctx = blk_mq_map_queue(q, data->ctx->cpu);
if (e) {
data->flags |= BLK_MQ_REQ_INTERNAL;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9611cd9920e9..cc9713f574a5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -273,10 +273,9 @@ EXPORT_SYMBOL(blk_mq_alloc_request);
struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
unsigned int flags, unsigned int hctx_idx)
{
- struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
+ struct blk_mq_alloc_data alloc_data = { .flags = flags };
struct request *rq;
- struct blk_mq_alloc_data alloc_data;
+ unsigned int cpu;
int ret;
/*
@@ -299,26 +298,23 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
* Check if the hardware context is actually mapped to anything.
* If not tell the caller that it should skip this queue.
*/
- hctx = q->queue_hw_ctx[hctx_idx];
- if (!blk_mq_hw_queue_mapped(hctx)) {
- ret = -EXDEV;
- goto out_queue_exit;
+ alloc_data.hctx = q->queue_hw_ctx[hctx_idx];
+ if (!blk_mq_hw_queue_mapped(alloc_data.hctx)) {
+ blk_queue_exit(q);
+ return ERR_PTR(-EXDEV);
}
- ctx = __blk_mq_get_ctx(q, cpumask_first(hctx->cpumask));
+ cpu = cpumask_first(alloc_data.hctx->cpumask);
+ alloc_data.ctx = __blk_mq_get_ctx(q, cpu);
- blk_mq_set_alloc_data(&alloc_data, q, flags, ctx, hctx);
- rq = __blk_mq_alloc_request(&alloc_data, rw);
- if (!rq) {
- ret = -EWOULDBLOCK;
- goto out_queue_exit;
- }
- alloc_data.hctx->tags->rqs[rq->tag] = rq;
-
- return rq;
+ rq = blk_mq_sched_get_request(q, NULL, rw, &alloc_data);
-out_queue_exit:
+ blk_mq_put_ctx(alloc_data.ctx);
blk_queue_exit(q);
- return ERR_PTR(ret);
+
+ if (!rq)
+ return ERR_PTR(-EWOULDBLOCK);
+
+ return rq;
}
EXPORT_SYMBOL_GPL(blk_mq_alloc_request_hctx);
--
2.12.0
[-- Attachment #3: 0002-blk-mq-kill-blk_mq_set_alloc_data.patch --]
[-- Type: text/x-diff, Size: 1227 bytes --]
>From 87aa35aa12f2edd4a9c56c382bf85ee580edf156 Mon Sep 17 00:00:00 2001
Message-Id: <87aa35aa12f2edd4a9c56c382bf85ee580edf156.1488217516.git.osandov@fb.com>
In-Reply-To: <dcfe49f597dc1eec3b326024c86a6ad3afb82fa8.1488217516.git.osandov@fb.com>
References: <dcfe49f597dc1eec3b326024c86a6ad3afb82fa8.1488217516.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Date: Mon, 27 Feb 2017 09:38:58 -0800
Subject: [PATCH 2/3] blk-mq: kill blk_mq_set_alloc_data()
Nothing is using it anymore.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
block/blk-mq.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 24b2256186f3..088ced003c13 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -146,16 +146,6 @@ struct blk_mq_alloc_data {
struct blk_mq_hw_ctx *hctx;
};
-static inline void blk_mq_set_alloc_data(struct blk_mq_alloc_data *data,
- struct request_queue *q, unsigned int flags,
- struct blk_mq_ctx *ctx, struct blk_mq_hw_ctx *hctx)
-{
- data->q = q;
- data->flags = flags;
- data->ctx = ctx;
- data->hctx = hctx;
-}
-
static inline struct blk_mq_tags *blk_mq_tags_from_data(struct blk_mq_alloc_data *data)
{
if (data->flags & BLK_MQ_REQ_INTERNAL)
--
2.12.0
[-- Attachment #4: 0003-blk-mq-move-update-of-tags-rqs-to-__blk_mq_alloc_req.patch --]
[-- Type: text/x-diff, Size: 1611 bytes --]
>From 4715cce107df8b579734a074093a34001efd01d0 Mon Sep 17 00:00:00 2001
Message-Id: <4715cce107df8b579734a074093a34001efd01d0.1488217516.git.osandov@fb.com>
In-Reply-To: <dcfe49f597dc1eec3b326024c86a6ad3afb82fa8.1488217516.git.osandov@fb.com>
References: <dcfe49f597dc1eec3b326024c86a6ad3afb82fa8.1488217516.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Date: Mon, 27 Feb 2017 09:40:34 -0800
Subject: [PATCH 3/3] blk-mq: move update of tags->rqs to
__blk_mq_alloc_request()
No functional difference, it just makes a little more sense to update
the tag map where we actually allocate the tag.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
Optional cleanup, since there's only one place that does this.
block/blk-mq-sched.c | 2 --
block/blk-mq.c | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 5697b23412a1..09af8ff18719 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -134,8 +134,6 @@ struct request *blk_mq_sched_get_request(struct request_queue *q,
rq = __blk_mq_alloc_request(data, op);
} else {
rq = __blk_mq_alloc_request(data, op);
- if (rq)
- data->hctx->tags->rqs[rq->tag] = rq;
}
if (rq) {
diff --git a/block/blk-mq.c b/block/blk-mq.c
index cc9713f574a5..452c1caf978f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -234,6 +234,7 @@ struct request *__blk_mq_alloc_request(struct blk_mq_alloc_data *data,
}
rq->tag = tag;
rq->internal_tag = -1;
+ data->hctx->tags->rqs[rq->tag] = rq;
}
blk_mq_rq_ctx_init(data->q, data->ctx, rq, op);
--
2.12.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 17:53 ` [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
@ 2017-02-27 18:02 ` Sagi Grimberg
0 siblings, 0 replies; 11+ messages in thread
From: Sagi Grimberg @ 2017-02-27 18:02 UTC (permalink / raw)
To: Omar Sandoval, Jens Axboe, linux-block; +Cc: kernel-team
>> blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
>> its blk_mq_alloc_request() counterpart. It also crashes because it
>> doesn't update the tags->rqs map.
>>
>> Fix it by making it allocate a scheduler request.
>>
>> Reported-by: Sagi Grimberg <sagi@grimberg.me>
>> Signed-off-by: Omar Sandoval <osandov@fb.com>
>> ---
>> I think this should do it. Verified that normal operation still works
>> for me, but I'm not sure how to test the actual _hctx() alloc path.
>> Sagi, could you please test this series out?
>
> Hm, Sagi, your mail server seems to have rejected patches 2 and 3
> because git-send-email duplicated the in-reply-to header for some
> reason. I've attached the patches instead.
Strange, I got the them.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 17:47 [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
` (2 preceding siblings ...)
2017-02-27 17:53 ` [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
@ 2017-02-27 18:07 ` Jens Axboe
2017-02-27 18:30 ` Omar Sandoval
2017-02-27 18:28 ` [PATCH v2] " Omar Sandoval
4 siblings, 1 reply; 11+ messages in thread
From: Jens Axboe @ 2017-02-27 18:07 UTC (permalink / raw)
To: Omar Sandoval, Sagi Grimberg, linux-block; +Cc: kernel-team
On 02/27/2017 10:47 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
> its blk_mq_alloc_request() counterpart. It also crashes because it
> doesn't update the tags->rqs map.
>
> Fix it by making it allocate a scheduler request.
All three look good to me. Would you mind respinning #1, it doesn't
apply on top of the reserved tag patch.
--
Jens Axboe
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 17:47 [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
` (3 preceding siblings ...)
2017-02-27 18:07 ` Jens Axboe
@ 2017-02-27 18:28 ` Omar Sandoval
4 siblings, 0 replies; 11+ messages in thread
From: Omar Sandoval @ 2017-02-27 18:28 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
its blk_mq_alloc_request() counterpart. It also crashes because it
doesn't update the tags->rqs map.
Fix it by making it allocate a scheduler request.
Reported-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
block/blk-mq-sched.c | 11 +++++------
block/blk-mq.c | 33 +++++++++++++++------------------
2 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 46ca965fff5c..5697b23412a1 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -110,15 +110,14 @@ struct request *blk_mq_sched_get_request(struct request_queue *q,
struct blk_mq_alloc_data *data)
{
struct elevator_queue *e = q->elevator;
- struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
struct request *rq;
blk_queue_enter_live(q);
- ctx = blk_mq_get_ctx(q);
- hctx = blk_mq_map_queue(q, ctx->cpu);
-
- blk_mq_set_alloc_data(data, q, data->flags, ctx, hctx);
+ data->q = q;
+ if (likely(!data->ctx))
+ data->ctx = blk_mq_get_ctx(q);
+ if (likely(!data->hctx))
+ data->hctx = blk_mq_map_queue(q, data->ctx->cpu);
if (e) {
data->flags |= BLK_MQ_REQ_INTERNAL;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4df5fb42c74f..85a7047d8b03 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -273,10 +273,9 @@ EXPORT_SYMBOL(blk_mq_alloc_request);
struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
unsigned int flags, unsigned int hctx_idx)
{
- struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
+ struct blk_mq_alloc_data alloc_data = { .flags = flags };
struct request *rq;
- struct blk_mq_alloc_data alloc_data;
+ unsigned int cpu;
int ret;
/*
@@ -299,25 +298,23 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
* Check if the hardware context is actually mapped to anything.
* If not tell the caller that it should skip this queue.
*/
- hctx = q->queue_hw_ctx[hctx_idx];
- if (!blk_mq_hw_queue_mapped(hctx)) {
- ret = -EXDEV;
- goto out_queue_exit;
+ alloc_data.hctx = q->queue_hw_ctx[hctx_idx];
+ if (!blk_mq_hw_queue_mapped(alloc_data.hctx)) {
+ blk_queue_exit(q);
+ return ERR_PTR(-EXDEV);
}
- ctx = __blk_mq_get_ctx(q, cpumask_first(hctx->cpumask));
+ cpu = cpumask_first(alloc_data.hctx->cpumask);
+ alloc_data.ctx = __blk_mq_get_ctx(q, cpu);
- blk_mq_set_alloc_data(&alloc_data, q, flags, ctx, hctx);
- rq = __blk_mq_alloc_request(&alloc_data, rw);
- if (!rq) {
- ret = -EWOULDBLOCK;
- goto out_queue_exit;
- }
-
- return rq;
+ rq = blk_mq_sched_get_request(q, NULL, rw, &alloc_data);
-out_queue_exit:
+ blk_mq_put_ctx(alloc_data.ctx);
blk_queue_exit(q);
- return ERR_PTR(ret);
+
+ if (!rq)
+ return ERR_PTR(-EWOULDBLOCK);
+
+ return rq;
}
EXPORT_SYMBOL_GPL(blk_mq_alloc_request_hctx);
--
2.12.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 18:07 ` Jens Axboe
@ 2017-02-27 18:30 ` Omar Sandoval
2017-02-27 18:34 ` Jens Axboe
0 siblings, 1 reply; 11+ messages in thread
From: Omar Sandoval @ 2017-02-27 18:30 UTC (permalink / raw)
To: Jens Axboe; +Cc: Sagi Grimberg, linux-block, kernel-team
On Mon, Feb 27, 2017 at 11:07:27AM -0700, Jens Axboe wrote:
> On 02/27/2017 10:47 AM, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
> > its blk_mq_alloc_request() counterpart. It also crashes because it
> > doesn't update the tags->rqs map.
> >
> > Fix it by making it allocate a scheduler request.
>
> All three look good to me. Would you mind respinning #1, it doesn't
> apply on top of the reserved tag patch.
Yup, I had those based on Sagi's original patches for some reason. I
fat-fingered send-email, sent as a reply to the original patch 1 instead
of this email.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 18:30 ` Omar Sandoval
@ 2017-02-27 18:34 ` Jens Axboe
2017-02-27 18:41 ` Sagi Grimberg
0 siblings, 1 reply; 11+ messages in thread
From: Jens Axboe @ 2017-02-27 18:34 UTC (permalink / raw)
To: Omar Sandoval; +Cc: Sagi Grimberg, linux-block, kernel-team
On 02/27/2017 11:30 AM, Omar Sandoval wrote:
> On Mon, Feb 27, 2017 at 11:07:27AM -0700, Jens Axboe wrote:
>> On 02/27/2017 10:47 AM, Omar Sandoval wrote:
>>> From: Omar Sandoval <osandov@fb.com>
>>>
>>> blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
>>> its blk_mq_alloc_request() counterpart. It also crashes because it
>>> doesn't update the tags->rqs map.
>>>
>>> Fix it by making it allocate a scheduler request.
>>
>> All three look good to me. Would you mind respinning #1, it doesn't
>> apply on top of the reserved tag patch.
>
> Yup, I had those based on Sagi's original patches for some reason. I
> fat-fingered send-email, sent as a reply to the original patch 1 instead
> of this email.
I got it, applied all 3, thanks Omar!
--
Jens Axboe
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 18:34 ` Jens Axboe
@ 2017-02-27 18:41 ` Sagi Grimberg
2017-02-27 18:44 ` Jens Axboe
0 siblings, 1 reply; 11+ messages in thread
From: Sagi Grimberg @ 2017-02-27 18:41 UTC (permalink / raw)
To: Jens Axboe, Omar Sandoval; +Cc: linux-block, kernel-team
>>>> blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
>>>> its blk_mq_alloc_request() counterpart. It also crashes because it
>>>> doesn't update the tags->rqs map.
>>>>
>>>> Fix it by making it allocate a scheduler request.
>>>
>>> All three look good to me. Would you mind respinning #1, it doesn't
>>> apply on top of the reserved tag patch.
>>
>> Yup, I had those based on Sagi's original patches for some reason. I
>> fat-fingered send-email, sent as a reply to the original patch 1 instead
>> of this email.
>
> I got it, applied all 3, thanks Omar!
FWIW, you can add my:
Tested-by: Sagi Grimberg <sagi@grmberg.me>
Although I didn't try to trigger reserved vs. normal tag
competition which will never ever happen for nvmf btw, but
I assume it can happen in drivers who use reserved for error
handling.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request
2017-02-27 18:41 ` Sagi Grimberg
@ 2017-02-27 18:44 ` Jens Axboe
0 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2017-02-27 18:44 UTC (permalink / raw)
To: Sagi Grimberg, Omar Sandoval; +Cc: linux-block, kernel-team
On 02/27/2017 11:41 AM, Sagi Grimberg wrote:
>
>>>>> blk_mq_alloc_request_hctx() allocates a driver request directly, unlike
>>>>> its blk_mq_alloc_request() counterpart. It also crashes because it
>>>>> doesn't update the tags->rqs map.
>>>>>
>>>>> Fix it by making it allocate a scheduler request.
>>>>
>>>> All three look good to me. Would you mind respinning #1, it doesn't
>>>> apply on top of the reserved tag patch.
>>>
>>> Yup, I had those based on Sagi's original patches for some reason. I
>>> fat-fingered send-email, sent as a reply to the original patch 1 instead
>>> of this email.
>>
>> I got it, applied all 3, thanks Omar!
>
> FWIW, you can add my:
>
> Tested-by: Sagi Grimberg <sagi@grmberg.me>
Done, I fixed up your typo, though :-)
> Although I didn't try to trigger reserved vs. normal tag
> competition which will never ever happen for nvmf btw, but
> I assume it can happen in drivers who use reserved for error
> handling.
As long as we verify they work independently, there should not
be a need to check that edge case separately. We should have
identical behavior there now to previously.
--
Jens Axboe
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-02-27 22:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-27 17:47 [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
2017-02-27 17:47 ` [PATCH 2/3] blk-mq: kill blk_mq_set_alloc_data() Omar Sandoval
2017-02-27 17:47 ` [PATCH 3/3] blk-mq: move update of tags->rqs to __blk_mq_alloc_request() Omar Sandoval
2017-02-27 17:53 ` [PATCH 1/3] blk-mq: make blk_mq_alloc_request_hctx() allocate a scheduler request Omar Sandoval
2017-02-27 18:02 ` Sagi Grimberg
2017-02-27 18:07 ` Jens Axboe
2017-02-27 18:30 ` Omar Sandoval
2017-02-27 18:34 ` Jens Axboe
2017-02-27 18:41 ` Sagi Grimberg
2017-02-27 18:44 ` Jens Axboe
2017-02-27 18:28 ` [PATCH v2] " Omar Sandoval
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).