Commit 5815839b authored by Ming Lei's avatar Ming Lei Committed by Jens Axboe

blk-mq: introduce new lock for protecting hctx->dispatch_wait

Now hctx->lock is only acquired when adding hctx->dispatch_wait to
one wait queue, but not held when removing it from the wait queue.

IO hang can be observed easily if SCHED RESTART is disabled, that means
now RESTART exits just for fixing the issue in blk_mq_mark_tag_wait().

This patch fixes the issue by introducing hctx->dispatch_wait_lock and
holding it for removing hctx->dispatch_wait in blk_mq_dispatch_wake(),
since we need to avoid acquiring hctx->lock in irq context.

Fixes: eb619fdb ("blk-mq: fix issue with shared tag queue re-running")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Bart Van Assche <bart.vanassche@wdc.com>
Tested-by: default avatarAndrew Jones <drjones@redhat.com>
Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 2278d69f
...@@ -998,7 +998,10 @@ static int blk_mq_dispatch_wake(wait_queue_entry_t *wait, unsigned mode, ...@@ -998,7 +998,10 @@ static int blk_mq_dispatch_wake(wait_queue_entry_t *wait, unsigned mode,
hctx = container_of(wait, struct blk_mq_hw_ctx, dispatch_wait); hctx = container_of(wait, struct blk_mq_hw_ctx, dispatch_wait);
spin_lock(&hctx->dispatch_wait_lock);
list_del_init(&wait->entry); list_del_init(&wait->entry);
spin_unlock(&hctx->dispatch_wait_lock);
blk_mq_run_hw_queue(hctx, true); blk_mq_run_hw_queue(hctx, true);
return 1; return 1;
} }
...@@ -1012,7 +1015,7 @@ static int blk_mq_dispatch_wake(wait_queue_entry_t *wait, unsigned mode, ...@@ -1012,7 +1015,7 @@ static int blk_mq_dispatch_wake(wait_queue_entry_t *wait, unsigned mode,
static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx, static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx,
struct request *rq) struct request *rq)
{ {
struct sbq_wait_state *ws; struct wait_queue_head *wq;
wait_queue_entry_t *wait; wait_queue_entry_t *wait;
bool ret; bool ret;
...@@ -1035,14 +1038,18 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx, ...@@ -1035,14 +1038,18 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx,
if (!list_empty_careful(&wait->entry)) if (!list_empty_careful(&wait->entry))
return false; return false;
spin_lock(&hctx->lock); wq = &bt_wait_ptr(&hctx->tags->bitmap_tags, hctx)->wait;
spin_lock_irq(&wq->lock);
spin_lock(&hctx->dispatch_wait_lock);
if (!list_empty(&wait->entry)) { if (!list_empty(&wait->entry)) {
spin_unlock(&hctx->lock); spin_unlock(&hctx->dispatch_wait_lock);
spin_unlock_irq(&wq->lock);
return false; return false;
} }
ws = bt_wait_ptr(&hctx->tags->bitmap_tags, hctx); wait->flags &= ~WQ_FLAG_EXCLUSIVE;
add_wait_queue(&ws->wait, wait); __add_wait_queue(wq, wait);
/* /*
* It's possible that a tag was freed in the window between the * It's possible that a tag was freed in the window between the
...@@ -1051,7 +1058,8 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx, ...@@ -1051,7 +1058,8 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx,
*/ */
ret = blk_mq_get_driver_tag(rq); ret = blk_mq_get_driver_tag(rq);
if (!ret) { if (!ret) {
spin_unlock(&hctx->lock); spin_unlock(&hctx->dispatch_wait_lock);
spin_unlock_irq(&wq->lock);
return false; return false;
} }
...@@ -1059,10 +1067,9 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx, ...@@ -1059,10 +1067,9 @@ static bool blk_mq_mark_tag_wait(struct blk_mq_hw_ctx *hctx,
* We got a tag, remove ourselves from the wait queue to ensure * We got a tag, remove ourselves from the wait queue to ensure
* someone else gets the wakeup. * someone else gets the wakeup.
*/ */
spin_lock_irq(&ws->wait.lock);
list_del_init(&wait->entry); list_del_init(&wait->entry);
spin_unlock_irq(&ws->wait.lock); spin_unlock(&hctx->dispatch_wait_lock);
spin_unlock(&hctx->lock); spin_unlock_irq(&wq->lock);
return true; return true;
} }
...@@ -2142,6 +2149,7 @@ static int blk_mq_init_hctx(struct request_queue *q, ...@@ -2142,6 +2149,7 @@ static int blk_mq_init_hctx(struct request_queue *q,
hctx->nr_ctx = 0; hctx->nr_ctx = 0;
spin_lock_init(&hctx->dispatch_wait_lock);
init_waitqueue_func_entry(&hctx->dispatch_wait, blk_mq_dispatch_wake); init_waitqueue_func_entry(&hctx->dispatch_wait, blk_mq_dispatch_wake);
INIT_LIST_HEAD(&hctx->dispatch_wait.entry); INIT_LIST_HEAD(&hctx->dispatch_wait.entry);
......
...@@ -39,6 +39,7 @@ struct blk_mq_hw_ctx { ...@@ -39,6 +39,7 @@ struct blk_mq_hw_ctx {
struct blk_mq_ctx **ctxs; struct blk_mq_ctx **ctxs;
unsigned int nr_ctx; unsigned int nr_ctx;
spinlock_t dispatch_wait_lock;
wait_queue_entry_t dispatch_wait; wait_queue_entry_t dispatch_wait;
atomic_t wait_index; atomic_t wait_index;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment