From: Omar Sandoval <osandov@osandov.com> To: Jens Axboe <axboe@kernel.dk> Cc: linux-block@vger.kernel.org, linux-nvme@lists.infradead.org Subject: Re: [PATCH 2/8] block: improve logic around when to sort a plug list Date: Tue, 27 Nov 2018 15:31:42 -0800 [thread overview] Message-ID: <20181127233142.GB846@vader> (raw) In-Reply-To: <20181126163556.5181-3-axboe@kernel.dk> On Mon, Nov 26, 2018 at 09:35:50AM -0700, Jens Axboe wrote: > Do it for the nr_hw_queues == 1 case, but only do it for the multi queue > case if we have requests for multiple devices in the plug. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > block/blk-core.c | 1 + > block/blk-mq.c | 7 +++++-- > include/linux/blkdev.h | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index be9233400314..c9758d185357 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1780,6 +1780,7 @@ void blk_start_plug(struct blk_plug *plug) > INIT_LIST_HEAD(&plug->mq_list); > INIT_LIST_HEAD(&plug->cb_list); > plug->rq_count = 0; > + plug->do_sort = false; > > /* > * Store ordering should not be needed here, since a potential > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 99c66823d52f..6a249bf6ed00 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -1678,7 +1678,8 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) > list_splice_init(&plug->mq_list, &list); > plug->rq_count = 0; > > - list_sort(NULL, &list, plug_rq_cmp); > + if (plug->do_sort) > + list_sort(NULL, &list, plug_rq_cmp); > > this_q = NULL; > this_hctx = NULL; > @@ -1935,6 +1936,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio) > > list_add_tail(&rq->queuelist, &plug->mq_list); > plug->rq_count++; > + plug->do_sort = true; > } else if (plug && !blk_queue_nomerges(q)) { > blk_mq_bio_to_request(rq, bio); > > @@ -1958,7 +1960,8 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio) > data.hctx = same_queue_rq->mq_hctx; > blk_mq_try_issue_directly(data.hctx, same_queue_rq, > &cookie); > - } > + } else if (plug->rq_count > 1) > + plug->do_sort = true; If plug->rq_count == 2, there's no benefit to sorting, either. The nr_hw_queues == 1 case could also avoid sorting in that case. So maybe this whole patch could just be replaced with: diff --git a/block/blk-mq.c b/block/blk-mq.c index 7b7dff85cf6c..de93c14e4700 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1675,9 +1675,10 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) unsigned int depth; list_splice_init(&plug->mq_list, &list); - plug->rq_count = 0; - list_sort(NULL, &list, plug_rq_cmp); + if (plug->rq_count > 2) + list_sort(NULL, &list, plug_rq_cmp); + plug->rq_count = 0; this_q = NULL; this_hctx = NULL; That will also handle the multi-queue case since we only plug one request per multi-queue device. Thoughts?
WARNING: multiple messages have this Message-ID (diff)
From: osandov@osandov.com (Omar Sandoval) Subject: [PATCH 2/8] block: improve logic around when to sort a plug list Date: Tue, 27 Nov 2018 15:31:42 -0800 [thread overview] Message-ID: <20181127233142.GB846@vader> (raw) In-Reply-To: <20181126163556.5181-3-axboe@kernel.dk> On Mon, Nov 26, 2018@09:35:50AM -0700, Jens Axboe wrote: > Do it for the nr_hw_queues == 1 case, but only do it for the multi queue > case if we have requests for multiple devices in the plug. > > Signed-off-by: Jens Axboe <axboe at kernel.dk> > --- > block/blk-core.c | 1 + > block/blk-mq.c | 7 +++++-- > include/linux/blkdev.h | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index be9233400314..c9758d185357 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1780,6 +1780,7 @@ void blk_start_plug(struct blk_plug *plug) > INIT_LIST_HEAD(&plug->mq_list); > INIT_LIST_HEAD(&plug->cb_list); > plug->rq_count = 0; > + plug->do_sort = false; > > /* > * Store ordering should not be needed here, since a potential > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 99c66823d52f..6a249bf6ed00 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -1678,7 +1678,8 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) > list_splice_init(&plug->mq_list, &list); > plug->rq_count = 0; > > - list_sort(NULL, &list, plug_rq_cmp); > + if (plug->do_sort) > + list_sort(NULL, &list, plug_rq_cmp); > > this_q = NULL; > this_hctx = NULL; > @@ -1935,6 +1936,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio) > > list_add_tail(&rq->queuelist, &plug->mq_list); > plug->rq_count++; > + plug->do_sort = true; > } else if (plug && !blk_queue_nomerges(q)) { > blk_mq_bio_to_request(rq, bio); > > @@ -1958,7 +1960,8 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio) > data.hctx = same_queue_rq->mq_hctx; > blk_mq_try_issue_directly(data.hctx, same_queue_rq, > &cookie); > - } > + } else if (plug->rq_count > 1) > + plug->do_sort = true; If plug->rq_count == 2, there's no benefit to sorting, either. The nr_hw_queues == 1 case could also avoid sorting in that case. So maybe this whole patch could just be replaced with: diff --git a/block/blk-mq.c b/block/blk-mq.c index 7b7dff85cf6c..de93c14e4700 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1675,9 +1675,10 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule) unsigned int depth; list_splice_init(&plug->mq_list, &list); - plug->rq_count = 0; - list_sort(NULL, &list, plug_rq_cmp); + if (plug->rq_count > 2) + list_sort(NULL, &list, plug_rq_cmp); + plug->rq_count = 0; this_q = NULL; this_hctx = NULL; That will also handle the multi-queue case since we only plug one request per multi-queue device. Thoughts?
next prev parent reply other threads:[~2018-11-27 23:31 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-26 16:35 [PATCHSET 0/8] block plugging improvements Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-26 16:35 ` [PATCH 1/8] block: sum requests in the plug structure Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-26 17:02 ` Christoph Hellwig 2018-11-26 17:02 ` Christoph Hellwig 2018-11-26 16:35 ` [PATCH 2/8] block: improve logic around when to sort a plug list Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-27 23:31 ` Omar Sandoval [this message] 2018-11-27 23:31 ` Omar Sandoval 2018-11-27 23:49 ` Jens Axboe 2018-11-27 23:49 ` Jens Axboe 2018-11-27 23:55 ` Omar Sandoval 2018-11-27 23:55 ` Omar Sandoval 2018-11-27 23:59 ` Jens Axboe 2018-11-27 23:59 ` Jens Axboe 2018-11-28 0:05 ` Omar Sandoval 2018-11-28 0:05 ` Omar Sandoval 2018-11-28 0:16 ` Jens Axboe 2018-11-28 0:16 ` Jens Axboe 2018-11-26 16:35 ` [PATCH 3/8] blk-mq: add mq_ops->commit_rqs() Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-27 23:43 ` Omar Sandoval 2018-11-27 23:43 ` Omar Sandoval 2018-11-28 1:38 ` Ming Lei 2018-11-28 1:38 ` Ming Lei 2018-11-28 7:16 ` Christoph Hellwig 2018-11-28 7:16 ` Christoph Hellwig 2018-11-28 12:54 ` Jens Axboe 2018-11-28 12:54 ` Jens Axboe 2018-11-26 16:35 ` [PATCH 4/8] nvme: implement mq_ops->commit_rqs() hook Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-28 7:20 ` Christoph Hellwig 2018-11-28 7:20 ` Christoph Hellwig 2018-11-28 13:07 ` Jens Axboe 2018-11-28 13:07 ` Jens Axboe 2018-11-26 16:35 ` [PATCH 5/8] virtio_blk: " Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-27 23:45 ` Omar Sandoval 2018-11-27 23:45 ` Omar Sandoval 2018-11-28 3:05 ` Michael S. Tsirkin 2018-11-28 3:05 ` Michael S. Tsirkin 2018-11-28 2:10 ` Ming Lei 2018-11-28 2:10 ` Ming Lei 2018-11-28 2:34 ` Jens Axboe 2018-11-28 2:34 ` Jens Axboe 2018-11-29 1:23 ` Ming Lei 2018-11-29 1:23 ` Ming Lei 2018-11-29 2:19 ` Jens Axboe 2018-11-29 2:19 ` Jens Axboe 2018-11-29 2:51 ` Ming Lei 2018-11-29 2:51 ` Ming Lei 2018-11-29 3:13 ` Jens Axboe 2018-11-29 3:13 ` Jens Axboe 2018-11-29 3:27 ` Ming Lei 2018-11-29 3:27 ` Ming Lei 2018-11-29 3:53 ` Jens Axboe 2018-11-29 3:53 ` Jens Axboe 2018-11-28 7:21 ` Christoph Hellwig 2018-11-28 7:21 ` Christoph Hellwig 2018-11-26 16:35 ` [PATCH 6/8] ataflop: " Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-27 23:46 ` Omar Sandoval 2018-11-27 23:46 ` Omar Sandoval 2018-11-28 7:22 ` Christoph Hellwig 2018-11-28 7:22 ` Christoph Hellwig 2018-11-28 13:09 ` Jens Axboe 2018-11-28 13:09 ` Jens Axboe 2018-11-26 16:35 ` [PATCH 7/8] blk-mq: use bd->last == true for list inserts Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-27 23:49 ` Omar Sandoval 2018-11-27 23:49 ` Omar Sandoval 2018-11-27 23:51 ` Jens Axboe 2018-11-27 23:51 ` Jens Axboe 2018-11-28 1:49 ` Ming Lei 2018-11-28 1:49 ` Ming Lei 2018-11-28 2:37 ` Jens Axboe 2018-11-28 2:37 ` Jens Axboe 2018-11-26 16:35 ` [PATCH 8/8] blk-mq: add plug case for devices that implement ->commits_rqs() Jens Axboe 2018-11-26 16:35 ` Jens Axboe 2018-11-28 7:26 ` Christoph Hellwig 2018-11-28 7:26 ` Christoph Hellwig 2018-11-28 13:11 ` Jens Axboe 2018-11-28 13:11 ` Jens Axboe
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20181127233142.GB846@vader \ --to=osandov@osandov.com \ --cc=axboe@kernel.dk \ --cc=linux-block@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.