lkml.org 
[lkml]   [2008]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: performance "regression" in cfq compared to anticipatory, deadline and noop
On Fri, May 16 2008, Daniel J Blueman wrote:
> On Fri, May 16, 2008 at 8:57 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> > On Fri, May 16 2008, Jens Axboe wrote:
> >> On Fri, May 16 2008, Fabio Checconi wrote:
> >> > > From: Jens Axboe <jens.axboe@oracle.com>
> >> > > Date: Fri, May 16, 2008 08:40:03AM +0200
> >> > >
> >> > ...
> >> > > I think we can improve this further without getting too involved. If a
> >> > > 2nd request is seen in cfq_rq_enqueued(), then DO schedule a dispatch
> >> > > since this likely means that we wont be doing more merges on the first
> >> > > one.
> >> > >
> >> >
> >> > But isn't there the risk that even the second request would be
> >> > dispatched, while it still could have grown?
> >>
> >> Certainly, you'd only want to dispatch the first request. Ideally we'd
> >> just get rid of this logic of 'did empty dispatch round' and only
> >> dispatch requests once merging is done, it's basically the wrong thing
> >> to do to make it visible to the io scheduler so soon. Well of course
> >> even more ideally we'd always get big requests submitted, but
> >> unfortunately many producers aren't that nice.
> >>
> >> The per-process plugging actually solves this nicely, since we do the
> >> merging outside of the io scheduler. Perhaps just not dispatch on a
> >> plugged queue would help a bit. I'm somewhat against this principle of
> >> messing too much with dispatch logic in the schedulers, it'd be nicer to
> >> solve this higher up.
> >
> > Something like this...
> >
> > diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> > index 5dfb7b9..5ab1a17 100644
> > --- a/block/cfq-iosched.c
> > +++ b/block/cfq-iosched.c
> > @@ -1775,6 +1775,9 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq,
> >
> > cic->last_request_pos = rq->sector + rq->nr_sectors;
> >
> > + if (blk_queue_plugged(cfqd->queue))
> > + return;
> > +
> > if (cfqq == cfqd->active_queue) {
> > /*
> > * if we are waiting for a request for this queue, let it rip
> > @@ -1784,7 +1787,7 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq,
> > if (cfq_cfqq_wait_request(cfqq)) {
> > cfq_mark_cfqq_must_dispatch(cfqq);
> > del_timer(&cfqd->idle_slice_timer);
> > - blk_start_queueing(cfqd->queue);
> > + cfq_schedule_dispatch(cfqd);
> > }
> > } else if (cfq_should_preempt(cfqd, cfqq, rq)) {
> > /*
> > @@ -1794,7 +1797,7 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq,
> > */
> > cfq_preempt_queue(cfqd, cfqq);
> > cfq_mark_cfqq_must_dispatch(cfqq);
> > - blk_start_queueing(cfqd->queue);
> > + cfq_schedule_dispatch(cfqd);
> > }
> > }
> >
> > @@ -1997,11 +2000,10 @@ static void cfq_kick_queue(struct work_struct *work)
> > struct cfq_data *cfqd =
> > container_of(work, struct cfq_data, unplug_work);
> > struct request_queue *q = cfqd->queue;
> > - unsigned long flags;
> >
> > - spin_lock_irqsave(q->queue_lock, flags);
> > + spin_lock_irq(q->queue_lock);
> > blk_start_queueing(q);
> > - spin_unlock_irqrestore(q->queue_lock, flags);
> > + spin_unlock_irq(q->queue_lock);
> > }
> >
> > /*
>
> Platter speed at 64KB stride, but 16% (101MB/s) less performance at
> 4KB stride - perhaps merging isn't quite right?
>
> Both traces at http://quora.org/blktrace-profiles-3.tar.bz2 ; let me
> know if you'd like me to test Fabio's patch still.

If you have time, please do test that one as well, thanks :-)

--
Jens Axboe



\
 
 \ /
  Last update: 2008-05-16 11:01    [W:0.059 / U:1.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site