On Mon, Dec 06, 2021 at 11:31:21AM +0100, Marek Szyprowski wrote:
> Hi,
>
> On 03.12.2021 14:15, Ming Lei wrote:
> > Remove hctx_lock and hctx_unlock, and add one helper of
> > blk_mq_run_dispatch_ops() to run code block defined in dispatch_ops
> > with rcu/srcu read held.
> >
> > Compared with hctx_lock()/hctx_unlock():
> >
> > 1) remove 2 branch to 1, so we just need to check
> > (hctx->flags & BLK_MQ_F_BLOCKING) once when running one dispatch_ops
> >
> > 2) srcu_idx needn't to be touched in case of non-blocking
> >
> > 3) might_sleep_if() can be moved to the blocking branch
> >
> > Also put the added blk_mq_run_dispatch_ops() in private header, so that
> > the following patch can use it out of blk-mq.c.
> >
> > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
>
> This patch landed in linux next-20211206 as commit 2a904d00855f
> ("blk-mq: remove hctx_lock and hctx_unlock"). It introduces a following
> 'BUG' warning on my test systems (ARM/ARM64-based boards with rootfs on
> SD card or eMMC):
>
> BUG: sleeping function called from invalid context at block/blk-mq.c:2060
> in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 249, name:
> kworker/0:3H
> preempt_count: 1, expected: 0
> RCU nest depth: 0, expected: 0
> 4 locks held by kworker/0:3H/249:
> #0: c1d782a8 ((wq_completion)mmc_complete){+.+.}-{0:0}, at:
> process_one_work+0x21c/0x7ec
> #1: c3b59f18 ((work_completion)(&mq->complete_work)){+.+.}-{0:0}, at:
> process_one_work+0x21c/0x7ec
> #2: c1d7858c (&mq->complete_lock){+.+.}-{3:3}, at:
> mmc_blk_mq_complete_prev_req.part.3+0x2c/0x234
> #3: c1f7a1b4 (&fq->mq_flush_lock){....}-{2:2}, at:
> mq_flush_data_end_io+0x68/0x124
It should be fixed by the attached patch.
>From bce4d1bf7ab4ac4c04a65eca67705567e9d5f0c0 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@xxxxxxxxxx>
Date: Mon, 6 Dec 2021 15:54:11 +0800
Subject: [PATCH] blk-mq: don't run might_sleep() if the operation needn't
blocking
The operation protected via blk_mq_run_dispatch_ops() in blk_mq_run_hw_queue
won't sleep, so don't run might_sleep() for it.
Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
---
block/blk-mq.c | 2 +-
block/blk-mq.h | 7 +++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 537295f6e0e9..0bf3523dd1f5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2048,7 +2048,7 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
* And queue will be rerun in blk_mq_unquiesce_queue() if it is
* quiesced.
*/
- blk_mq_run_dispatch_ops(hctx->queue,
+ __blk_mq_run_dispatch_ops(hctx->queue, false,
need_run = !blk_queue_quiesced(hctx->queue) &&
blk_mq_hctx_has_pending(hctx));
diff --git a/block/blk-mq.h b/block/blk-mq.h
index d62004e2d531..948791ea2a3e 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -375,7 +375,7 @@ static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,
}
/* run the code block in @dispatch_ops with rcu/srcu read lock held */
-#define blk_mq_run_dispatch_ops(q, dispatch_ops) \
+#define __blk_mq_run_dispatch_ops(q, check_sleep, dispatch_ops) \
do { \
if (!blk_queue_has_srcu(q)) { \
rcu_read_lock(); \
@@ -384,11 +384,14 @@ do { \
} else { \
int srcu_idx; \
\
- might_sleep(); \
+ might_sleep_if(check_sleep); \
srcu_idx = srcu_read_lock((q)->srcu); \
(dispatch_ops); \
srcu_read_unlock((q)->srcu, srcu_idx); \
} \
} while (0)
+#define blk_mq_run_dispatch_ops(q, dispatch_ops) \
+ __blk_mq_run_dispatch_ops(q, true, dispatch_ops) \
+
#endif
--
2.31.1
Thanks,
Ming
