Hi,
On 06.12.2021 12:12, Ming Lei wrote:
> On Mon, Dec 06, 2021 at 11:31:21AM +0100, Marek Szyprowski wrote:
>> 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>
Confirmed, this fixed the issue.
Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> ---
> 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
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
