Messages in this thread | | | Date | Sun, 1 Nov 2015 08:32:46 +0800 | Subject | Re: [patch, v2] blk-mq: avoid excessive boot delays with large lun counts | From | Ming Lei <> |
| |
On Sat, Oct 31, 2015 at 1:22 AM, Jeff Moyer <jmoyer@redhat.com> wrote: > Hi, > > Zhangqing Luo reported long boot times on a system with thousands of > LUNs when scsi-mq was enabled. He narrowed the problem down to > blk_mq_add_queue_tag_set, where every queue is frozen in order to set > the BLK_MQ_F_TAG_SHARED flag. Each added device will freeze all queues > added before it in sequence, which involves waiting for an RCU grace > period for each one. We don't need to do this. After the second queue > is added, only new queues need to be initialized with the shared tag. > We can do that by percolating the flag up to the blk_mq_tag_set, and > updating the newly added queue's hctxs if the flag is set. > > This problem was introduced by commit 0d2602ca30e41 (blk-mq: improve > support for shared tags maps). > > Reported-and-tested-by: Jason Luo <zhangqing.luo@oracle.com> > Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
You can add Reviewed-by: Ming Lei <ming.lei@canonical.com> if the following trivial issues(especially the 2nd one) are addressed.
> > --- > Changes from v1: > - addressed review comments from Ming, which simplified the patch > > Jason, if you could sanity test this patch to make sure it still solves > your problem, that would be greatly appreciated. > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 85f0143..12f79af 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -1673,7 +1673,7 @@ static int blk_mq_init_hctx(struct request_queue *q, > INIT_LIST_HEAD(&hctx->dispatch); > hctx->queue = q; > hctx->queue_num = hctx_idx; > - hctx->flags = set->flags; > + hctx->flags = set->flags & ~BLK_MQ_F_TAG_SHARED; > > blk_mq_init_cpu_notifier(&hctx->cpu_notifier, > blk_mq_hctx_notify, hctx); > @@ -1860,27 +1860,26 @@ static void blk_mq_map_swqueue(struct request_queue *q, > } > } > > -static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set) > +static void queue_set_hctx_shared(struct request_queue *q, bool shared) > { > struct blk_mq_hw_ctx *hctx; > - struct request_queue *q; > - bool shared; > int i; > > - if (set->tag_list.next == set->tag_list.prev) > - shared = false; > - else > - shared = true; > + queue_for_each_hw_ctx(q, hctx, i) { > + if (shared) > + hctx->flags |= BLK_MQ_F_TAG_SHARED; > + else > + hctx->flags &= ~BLK_MQ_F_TAG_SHARED; > + } > +} > + > +static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, bool shared) > +{ > + struct request_queue *q; > > list_for_each_entry(q, &set->tag_list, tag_set_list) { > blk_mq_freeze_queue(q); > - > - queue_for_each_hw_ctx(q, hctx, i) { > - if (shared) > - hctx->flags |= BLK_MQ_F_TAG_SHARED; > - else > - hctx->flags &= ~BLK_MQ_F_TAG_SHARED; > - } > + queue_set_hctx_shared(q, shared); > blk_mq_unfreeze_queue(q); > } > } > @@ -1891,7 +1890,12 @@ static void blk_mq_del_queue_tag_set(struct request_queue *q) > > mutex_lock(&set->tag_list_lock); > list_del_init(&q->tag_set_list); > - blk_mq_update_tag_set_depth(set); > + if (set->tag_list.next == set->tag_list.prev) {
list_is_singular() should be better.
> + /* just transitioned to unshared */ > + set->flags &= ~BLK_MQ_F_TAG_SHARED; > + /* update existing queue */ > + blk_mq_update_tag_set_depth(set, false); > + } > mutex_unlock(&set->tag_list_lock); > } > > @@ -1901,8 +1905,17 @@ static void blk_mq_add_queue_tag_set(struct blk_mq_tag_set *set, > q->tag_set = set; > > mutex_lock(&set->tag_list_lock); > + > + /* Check to see if we're transitioning to shared (from 1 to 2 queues). */ > + if (!list_empty(&set->tag_list) && !(set->flags & BLK_MQ_F_TAG_SHARED)) { > + set->flags |= BLK_MQ_F_TAG_SHARED; > + /* update existing queue */ > + blk_mq_update_tag_set_depth(set, true); > + } > + if (set->flags & BLK_MQ_F_TAG_SHARED)
The above should be 'else if', otherwise the current queue will be set twice.
> + queue_set_hctx_shared(q, true); > list_add_tail(&q->tag_set_list, &set->tag_list); > - blk_mq_update_tag_set_depth(set); > + > mutex_unlock(&set->tag_list_lock); > } >
-- Ming Lei
| |