lkml.org 
[lkml]   [2017]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V5 7/7] blk-mq-sched: don't dequeue request until all in ->dispatch are flushed
On Tue, Oct 03, 2017 at 02:11:28AM -0700, Christoph Hellwig wrote:
> 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.

There are still two users of the local variable, so I suggest to
keep 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.

OK.

>
> > 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);
> }
>

OK.


--
Ming

\
 
 \ /
  Last update: 2017-10-09 12:40    [W:0.055 / U:2.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site