From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <20180523174012.GB12533@vader> References: <20180519074406.6045-1-ming.lei@redhat.com> <20180523174012.GB12533@vader> From: Ming Lei Date: Thu, 24 May 2018 10:20:01 +0800 Message-ID: Subject: Re: [PATCH V2] blk-mq: avoid to starve tag allocation after allocation process migrates To: Omar Sandoval Cc: Ming Lei , Jens Axboe , linux-block , stable , Omar Sandoval Content-Type: text/plain; charset="UTF-8" List-ID: On Thu, May 24, 2018 at 1:40 AM, Omar Sandoval wrote: > On Sat, May 19, 2018 at 03:44:06PM +0800, Ming Lei wrote: >> When the allocation process is scheduled back and the mapped hw queue is >> changed, do one extra wake up on orignal queue for compensating wake up >> miss, so other allocations on the orignal queue won't be starved. >> >> This patch fixes one request allocation hang issue, which can be >> triggered easily in case of very low nr_request. >> >> Cc: >> Cc: Omar Sandoval >> Signed-off-by: Ming Lei >> --- >> >> V2: >> fix build failure >> >> block/blk-mq-tag.c | 13 +++++++++++++ >> include/linux/sbitmap.h | 7 +++++++ >> lib/sbitmap.c | 6 ++++++ >> 3 files changed, 26 insertions(+) >> >> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c >> index 336dde07b230..77607f89d205 100644 >> --- a/block/blk-mq-tag.c >> +++ b/block/blk-mq-tag.c >> @@ -134,6 +134,8 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) >> ws = bt_wait_ptr(bt, data->hctx); >> drop_ctx = data->ctx == NULL; >> do { >> + struct sbitmap_queue *bt_orig; >> + >> /* >> * We're out of tags on this hardware queue, kick any >> * pending IO submits before going to sleep waiting for >> @@ -159,6 +161,7 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) >> if (data->ctx) >> blk_mq_put_ctx(data->ctx); >> >> + bt_orig = bt; >> io_schedule(); >> >> data->ctx = blk_mq_get_ctx(data->q); >> @@ -170,6 +173,16 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) >> bt = &tags->bitmap_tags; >> >> finish_wait(&ws->wait, &wait); >> + >> + /* >> + * If destination hw queue is changed, wake up original >> + * queue one extra time for compensating the wake up >> + * miss, so other allocations on original queue won't >> + * be starved. >> + */ >> + if (bt != bt_orig) >> + sbitmap_queue_wake_up(bt_orig); >> + >> ws = bt_wait_ptr(bt, data->hctx); >> } while (1); >> >> diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h >> index 841585f6e5f2..b23f50355281 100644 >> --- a/include/linux/sbitmap.h >> +++ b/include/linux/sbitmap.h >> @@ -484,6 +484,13 @@ static inline struct sbq_wait_state *sbq_wait_ptr(struct sbitmap_queue *sbq, >> void sbitmap_queue_wake_all(struct sbitmap_queue *sbq); >> >> /** >> + * sbitmap_wake_up() - Do a regular wake up compensation if the queue >> + * allocated from is changed after scheduling back. >> + * @sbq: Bitmap queue to wake up. >> + */ >> +void sbitmap_queue_wake_up(struct sbitmap_queue *sbq); >> + >> +/** >> * sbitmap_queue_show() - Dump &struct sbitmap_queue information to a &struct >> * seq_file. >> * @sbq: Bitmap queue to show. >> diff --git a/lib/sbitmap.c b/lib/sbitmap.c >> index e6a9c06ec70c..c6ae4206bcb1 100644 >> --- a/lib/sbitmap.c >> +++ b/lib/sbitmap.c >> @@ -466,6 +466,12 @@ static void sbq_wake_up(struct sbitmap_queue *sbq) >> } >> } >> >> +void sbitmap_queue_wake_up(struct sbitmap_queue *sbq) >> +{ >> + sbq_wake_up(sbq); >> +} >> +EXPORT_SYMBOL_GPL(sbitmap_queue_wake_up); > > There's no reason to wrap an internal helper with a function taking the > same arguments, just rename sbq_wake_up() and export it. This way may keep sbq_wake_up() inlined to sbitmap_queue_clear(), so we won't introduce any cost to fast path. Thanks, Ming Lei