From: Christoph Hellwig <hch@infradead.org>
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@fb.com>,
linux-block@vger.kernel.org,
Christoph Hellwig <hch@infradead.org>,
Mike Snitzer <snitzer@redhat.com>,
dm-devel@redhat.com, Bart Van Assche <bart.vanassche@sandisk.com>,
Laurence Oberman <loberman@redhat.com>,
Paolo Valente <paolo.valente@linaro.org>,
Oleksandr Natalenko <oleksandr@natalenko.name>,
Tom Nguyen <tom81094@gmail.com>,
linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
Omar Sandoval <osandov@fb.com>
Subject: Re: [PATCH V5 7/7] blk-mq-sched: don't dequeue request until all in ->dispatch are flushed
Date: Tue, 3 Oct 2017 02:11:28 -0700 [thread overview]
Message-ID: <20171003091128.GD21184@infradead.org> (raw)
In-Reply-To: <20170930102720.30219-8-ming.lei@redhat.com>
This looks good in general:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Minor nitpicks below:
> const bool has_sched_dispatch = e && e->type->ops.mq.dispatch_request;
This is now only tested once, so you can remove the local variable
for it.
> + /*
> + * We may clear DISPATCH_BUSY just after it
> + * is set from another context, the only cost
> + * is that one request is dequeued a bit early,
> + * we can survive that. Given the window is
> + * small enough, no need to worry about performance
> + * effect.
> + */
Use your 80 line real estate for comments please.
> if (!has_sched_dispatch)
> + if (!q->queue_depth) {
> + blk_mq_flush_busy_ctxs(hctx, &rq_list);
> + blk_mq_dispatch_rq_list(q, &rq_list);
> + } else {
> + blk_mq_do_dispatch_ctx(q, hctx);
> + }
> + } else {
> blk_mq_do_dispatch_sched(q, e, hctx);
> + }
Maybe flatten this out to:
if (e && e->type->ops.mq.dispatch_request) {
blk_mq_do_dispatch_sched(q, e, hctx);
} else if (q->queue_depth) {
blk_mq_do_dispatch_ctx(q, hctx);
} else {
blk_mq_flush_busy_ctxs(hctx, &rq_list);
blk_mq_dispatch_rq_list(q, &rq_list);
}
next prev parent reply other threads:[~2017-10-03 9:11 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-30 10:27 [PATCH V5 00/14] blk-mq-sched: improve sequential I/O performance(part 1) Ming Lei
2017-09-30 10:27 ` [PATCH V5 1/7] blk-mq: issue rq directly in blk_mq_request_bypass_insert() Ming Lei
2017-10-03 8:58 ` Christoph Hellwig
2017-10-03 13:39 ` Ming Lei
2017-09-30 10:27 ` [PATCH V5 2/7] blk-mq-sched: fix scheduler bad performance Ming Lei
2017-10-02 14:19 ` Christoph Hellwig
2017-09-30 10:27 ` [PATCH V5 3/7] sbitmap: introduce __sbitmap_for_each_set() Ming Lei
2017-09-30 10:27 ` [PATCH V5 4/7] blk-mq: introduce blk_mq_dequeue_from_ctx() Ming Lei
2017-10-03 9:01 ` Christoph Hellwig
2017-10-09 4:36 ` Ming Lei
2017-09-30 10:27 ` [PATCH V5 5/7] blk-mq-sched: move actual dispatching into one helper Ming Lei
2017-10-02 14:19 ` Christoph Hellwig
2017-10-09 9:07 ` Ming Lei
2017-09-30 10:27 ` [PATCH V5 6/7] blk-mq-sched: improve dispatching from sw queue Ming Lei
2017-10-03 9:05 ` Christoph Hellwig
2017-10-09 10:15 ` Ming Lei
2017-09-30 10:27 ` [PATCH V5 7/7] blk-mq-sched: don't dequeue request until all in ->dispatch are flushed Ming Lei
2017-10-03 9:11 ` Christoph Hellwig [this message]
2017-10-09 10:40 ` Ming Lei
2017-09-30 10:32 ` [PATCH V5 00/14] blk-mq-sched: improve sequential I/O performance(part 1) Ming Lei
2017-10-09 12:09 ` John Garry
2017-10-09 12:09 ` John Garry
2017-10-09 15:04 ` Ming Lei
2017-10-10 1:46 ` Ming Lei
2017-10-10 12:24 ` John Garry
2017-10-10 12:24 ` John Garry
2017-10-10 12:34 ` Johannes Thumshirn
2017-10-10 12:34 ` Johannes Thumshirn
2017-10-10 12:37 ` Paolo Valente
2017-10-10 12:37 ` Paolo Valente
2017-10-10 13:45 ` Ming Lei
2017-10-10 15:10 ` John Garry
2017-10-10 15:10 ` John Garry
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=20171003091128.GD21184@infradead.org \
--to=hch@infradead.org \
--cc=axboe@fb.com \
--cc=bart.vanassche@sandisk.com \
--cc=dm-devel@redhat.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=loberman@redhat.com \
--cc=ming.lei@redhat.com \
--cc=oleksandr@natalenko.name \
--cc=osandov@fb.com \
--cc=paolo.valente@linaro.org \
--cc=snitzer@redhat.com \
--cc=tom81094@gmail.com \
/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: link
Be 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.