* [PATCH AUTOSEL 6.0 14/73] blk-mq: avoid double ->queue_rq() because of early timeout
[not found] <20221218160741.927862-1-sashal@kernel.org>
@ 2022-12-18 16:06 ` Sasha Levin
2022-12-18 16:07 ` [PATCH AUTOSEL 6.0 54/73] blk-mq: fix possible memleak when register 'hctx' failed Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2022-12-18 16:06 UTC (permalink / raw
To: linux-kernel, stable
Cc: David Jeffery, Stefan Hajnoczi, Keith Busch, virtualization,
Bart Van Assche, Ming Lei, Jens Axboe, Sasha Levin, linux-block
From: David Jeffery <djeffery@redhat.com>
[ Upstream commit 82c229476b8f6afd7e09bc4dc77d89dc19ff7688 ]
David Jeffery found one double ->queue_rq() issue, so far it can
be triggered in VM use case because of long vmexit latency or preempt
latency of vCPU pthread or long page fault in vCPU pthread, then block
IO req could be timed out before queuing the request to hardware but after
calling blk_mq_start_request() during ->queue_rq(), then timeout handler
may handle it by requeue, then double ->queue_rq() is caused, and kernel
panic.
So far, it is driver's responsibility to cover the race between timeout
and completion, so it seems supposed to be solved in driver in theory,
given driver has enough knowledge.
But it is really one common problem, lots of driver could have similar
issue, and could be hard to fix all affected drivers, even it isn't easy
for driver to handle the race. So David suggests this patch by draining
in-progress ->queue_rq() for solving this issue.
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Keith Busch <kbusch@kernel.org>
Cc: virtualization@lists.linux-foundation.org
Cc: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: David Jeffery <djeffery@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/20221026051957.358818-1-ming.lei@redhat.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
block/blk-mq.c | 56 +++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 44 insertions(+), 12 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3f1f5e3e0951..1a30f6580274 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1442,7 +1442,13 @@ static void blk_mq_rq_timed_out(struct request *req)
blk_add_timer(req);
}
-static bool blk_mq_req_expired(struct request *rq, unsigned long *next)
+struct blk_expired_data {
+ bool has_timedout_rq;
+ unsigned long next;
+ unsigned long timeout_start;
+};
+
+static bool blk_mq_req_expired(struct request *rq, struct blk_expired_data *expired)
{
unsigned long deadline;
@@ -1452,13 +1458,13 @@ static bool blk_mq_req_expired(struct request *rq, unsigned long *next)
return false;
deadline = READ_ONCE(rq->deadline);
- if (time_after_eq(jiffies, deadline))
+ if (time_after_eq(expired->timeout_start, deadline))
return true;
- if (*next == 0)
- *next = deadline;
- else if (time_after(*next, deadline))
- *next = deadline;
+ if (expired->next == 0)
+ expired->next = deadline;
+ else if (time_after(expired->next, deadline))
+ expired->next = deadline;
return false;
}
@@ -1472,7 +1478,7 @@ void blk_mq_put_rq_ref(struct request *rq)
static bool blk_mq_check_expired(struct request *rq, void *priv)
{
- unsigned long *next = priv;
+ struct blk_expired_data *expired = priv;
/*
* blk_mq_queue_tag_busy_iter() has locked the request, so it cannot
@@ -1481,7 +1487,18 @@ static bool blk_mq_check_expired(struct request *rq, void *priv)
* it was completed and reallocated as a new request after returning
* from blk_mq_check_expired().
*/
- if (blk_mq_req_expired(rq, next))
+ if (blk_mq_req_expired(rq, expired)) {
+ expired->has_timedout_rq = true;
+ return false;
+ }
+ return true;
+}
+
+static bool blk_mq_handle_expired(struct request *rq, void *priv)
+{
+ struct blk_expired_data *expired = priv;
+
+ if (blk_mq_req_expired(rq, expired))
blk_mq_rq_timed_out(rq);
return true;
}
@@ -1490,7 +1507,9 @@ static void blk_mq_timeout_work(struct work_struct *work)
{
struct request_queue *q =
container_of(work, struct request_queue, timeout_work);
- unsigned long next = 0;
+ struct blk_expired_data expired = {
+ .timeout_start = jiffies,
+ };
struct blk_mq_hw_ctx *hctx;
unsigned long i;
@@ -1510,10 +1529,23 @@ static void blk_mq_timeout_work(struct work_struct *work)
if (!percpu_ref_tryget(&q->q_usage_counter))
return;
- blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &next);
+ /* check if there is any timed-out request */
+ blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &expired);
+ if (expired.has_timedout_rq) {
+ /*
+ * Before walking tags, we must ensure any submit started
+ * before the current time has finished. Since the submit
+ * uses srcu or rcu, wait for a synchronization point to
+ * ensure all running submits have finished
+ */
+ blk_mq_wait_quiesce_done(q);
+
+ expired.next = 0;
+ blk_mq_queue_tag_busy_iter(q, blk_mq_handle_expired, &expired);
+ }
- if (next != 0) {
- mod_timer(&q->timeout, next);
+ if (expired.next != 0) {
+ mod_timer(&q->timeout, expired.next);
} else {
/*
* Request timeouts are handled as a forward rolling timer. If
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH AUTOSEL 6.0 54/73] blk-mq: fix possible memleak when register 'hctx' failed
[not found] <20221218160741.927862-1-sashal@kernel.org>
2022-12-18 16:06 ` [PATCH AUTOSEL 6.0 14/73] blk-mq: avoid double ->queue_rq() because of early timeout Sasha Levin
@ 2022-12-18 16:07 ` Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2022-12-18 16:07 UTC (permalink / raw
To: linux-kernel, stable
Cc: Ye Bin, Ming Lei, Jens Axboe, Sasha Levin, linux-block
From: Ye Bin <yebin10@huawei.com>
[ Upstream commit 4b7a21c57b14fbcd0e1729150189e5933f5088e9 ]
There's issue as follows when do fault injection test:
unreferenced object 0xffff888132a9f400 (size 512):
comm "insmod", pid 308021, jiffies 4324277909 (age 509.733s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 08 f4 a9 32 81 88 ff ff ...........2....
08 f4 a9 32 81 88 ff ff 00 00 00 00 00 00 00 00 ...2............
backtrace:
[<00000000e8952bb4>] kmalloc_node_trace+0x22/0xa0
[<00000000f9980e0f>] blk_mq_alloc_and_init_hctx+0x3f1/0x7e0
[<000000002e719efa>] blk_mq_realloc_hw_ctxs+0x1e6/0x230
[<000000004f1fda40>] blk_mq_init_allocated_queue+0x27e/0x910
[<00000000287123ec>] __blk_mq_alloc_disk+0x67/0xf0
[<00000000a2a34657>] 0xffffffffa2ad310f
[<00000000b173f718>] 0xffffffffa2af824a
[<0000000095a1dabb>] do_one_initcall+0x87/0x2a0
[<00000000f32fdf93>] do_init_module+0xdf/0x320
[<00000000cbe8541e>] load_module+0x3006/0x3390
[<0000000069ed1bdb>] __do_sys_finit_module+0x113/0x1b0
[<00000000a1a29ae8>] do_syscall_64+0x35/0x80
[<000000009cd878b0>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
Fault injection context as follows:
kobject_add
blk_mq_register_hctx
blk_mq_sysfs_register
blk_register_queue
device_add_disk
null_add_dev.part.0 [null_blk]
As 'blk_mq_register_hctx' may already add some objects when failed halfway,
but there isn't do fallback, caller don't know which objects add failed.
To solve above issue just do fallback when add objects failed halfway in
'blk_mq_register_hctx'.
Signed-off-by: Ye Bin <yebin10@huawei.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Link: https://lore.kernel.org/r/20221117022940.873959-1-yebin@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
block/blk-mq-sysfs.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 93997d297d42..4515288fbe35 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -185,7 +185,7 @@ static int blk_mq_register_hctx(struct blk_mq_hw_ctx *hctx)
{
struct request_queue *q = hctx->queue;
struct blk_mq_ctx *ctx;
- int i, ret;
+ int i, j, ret;
if (!hctx->nr_ctx)
return 0;
@@ -197,9 +197,16 @@ static int blk_mq_register_hctx(struct blk_mq_hw_ctx *hctx)
hctx_for_each_ctx(hctx, ctx, i) {
ret = kobject_add(&ctx->kobj, &hctx->kobj, "cpu%u", ctx->cpu);
if (ret)
- break;
+ goto out;
}
+ return 0;
+out:
+ hctx_for_each_ctx(hctx, ctx, j) {
+ if (j < i)
+ kobject_del(&ctx->kobj);
+ }
+ kobject_del(&hctx->kobj);
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-12-18 16:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20221218160741.927862-1-sashal@kernel.org>
2022-12-18 16:06 ` [PATCH AUTOSEL 6.0 14/73] blk-mq: avoid double ->queue_rq() because of early timeout Sasha Levin
2022-12-18 16:07 ` [PATCH AUTOSEL 6.0 54/73] blk-mq: fix possible memleak when register 'hctx' failed Sasha Levin
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 read-only IMAP folder(s) and NNTP newsgroup(s).