On Wed, Sep 20, 2017 at 08:20:58PM +0800, Ming Lei wrote:
> On Tue, Sep 19, 2017 at 02:37:50PM -0600, Jens Axboe wrote:
> > On 09/02/2017 09:17 AM, Ming Lei wrote:
> > > @@ -142,18 +178,31 @@ void blk_mq_sched_dispatch_requests(struct
> > > blk_mq_hw_ctx *hctx)
> > > if (!list_empty(&rq_list)) {
> > > blk_mq_sched_mark_restart_hctx(hctx);
> > > do_sched_dispatch = blk_mq_dispatch_rq_list(q, &rq_list);
> > > - } else if (!has_sched_dispatch) {
> > > + } else if (!has_sched_dispatch && !q->queue_depth) {
> > > + /*
> > > + * If there is no per-request_queue depth, we
> > > + * flush all requests in this hw queue, otherwise
> > > + * pick up request one by one from sw queue for
> > > + * avoiding to mess up I/O merge when dispatch
> > > + * is busy, which can be triggered easily by
> > > + * per-request_queue queue depth
> > > + */
> > > blk_mq_flush_busy_ctxs(hctx, &rq_list);
> > > blk_mq_dispatch_rq_list(q, &rq_list);
> > > }
> >
> > I don't like this part at all. It's a heuristic, and on top of that,
> > it's a heuristic based on a weak assumption that if q->queue_depth == 0,
> > then we never run into BUSY constraints. I think that would be stronger
> > if it was based on "is this using shared tags", but even that is not
> > great at all. It's perfectly possible to have a shared tags setup and
> > never run into resource constraints. The opposite condition is also true
> > - you can run without shared tags, and yet hit resource constraints
> > constantly. Hence this is NOT a good test for deciding whether to flush
> > everything at once or not. In short, I think a much better test case
> > would be "has this device ever returned BLK_STS_RESOURCE. As it so
> > happens, that's easy enough to keep track of and base this test on.
>
> Hi Jens,
>
> The attached patch follows your suggestion, and uses EWMA to
> compute the average length of hctx->dispatch, then only flush
> all requests from ctxs if the average length is zero, what do
> you think about this approach? Or other suggestions?
Hi Jens,
I am going to prepare for V5, as suggested by Omar.
Could you suggest which way you prefer to? Keeping to check
q->queue_depth, or the approach of using average length of
hctx->dispath, or others?
--
Ming
