diff options
| author | Ross Lagerwall | 2023-07-14 11:11:06 +0100 |
|---|---|---|
| committer | Greg Kroah-Hartman | 2023-08-03 10:23:48 +0200 |
| commit | 557ea2ff05193c2a04cf05233102a774d9e57e52 (patch) | |
| tree | ed7c52f64a3e2783ccb0181c81302c0717278649 /block | |
| parent | 0935bbbf6e5aa55b7ebb3bde7ae1df44905121df (diff) | |
blk-mq: Fix stall due to recursive flush plug
[ Upstream commit 70904263512a74a3b8941dd9e6e515ca6fc57821 ]
We have seen rare IO stalls as follows:
* blk_mq_plug_issue_direct() is entered with an mq_list containing two
requests.
* For the first request, it sets last == false and enters the driver's
queue_rq callback.
* The driver queue_rq callback indirectly calls schedule() which calls
blk_flush_plug(). This may happen if the driver has the
BLK_MQ_F_BLOCKING flag set and is allowed to sleep in ->queue_rq.
* blk_flush_plug() handles the remaining request in the mq_list. mq_list
is now empty.
* The original call to queue_rq resumes (with last == false).
* The loop in blk_mq_plug_issue_direct() terminates because there are no
remaining requests in mq_list.
The IO is now stalled because the last request submitted to the driver
had last == false and there was no subsequent call to commit_rqs().
Fix this by returning early in blk_mq_flush_plug_list() if rq_count is 0
which it will be in the recursive case, rather than checking if the
mq_list is empty. At the same time, adjust one of the callers to skip
the mq_list empty check as it is not necessary.
Fixes: dc5fc361d891 ("block: attempt direct issue of plug list")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/20230714101106.3635611-1-ross.lagerwall@citrix.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Diffstat (limited to 'block')
| -rw-r--r-- | block/blk-core.c | 3 | ||||
| -rw-r--r-- | block/blk-mq.c | 9 |
2 files changed, 9 insertions, 3 deletions
diff --git a/block/blk-core.c b/block/blk-core.c index 24ee7785a5ad..ebb7a1689b26 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1140,8 +1140,7 @@ void __blk_flush_plug(struct blk_plug *plug, bool from_schedule) { if (!list_empty(&plug->cb_list)) flush_plug_callbacks(plug, from_schedule); - if (!rq_list_empty(plug->mq_list)) - blk_mq_flush_plug_list(plug, from_schedule); + blk_mq_flush_plug_list(plug, from_schedule); /* * Unconditionally flush out cached requests, even if the unplug * event came from schedule. Since we know hold references to the diff --git a/block/blk-mq.c b/block/blk-mq.c index add013d5bbda..100fb0c3114f 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2749,7 +2749,14 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) { struct request *rq; - if (rq_list_empty(plug->mq_list)) + /* + * We may have been called recursively midway through handling + * plug->mq_list via a schedule() in the driver's queue_rq() callback. + * To avoid mq_list changing under our feet, clear rq_count early and + * bail out specifically if rq_count is 0 rather than checking + * whether the mq_list is empty. + */ + if (plug->rq_count == 0) return; plug->rq_count = 0; |