Messages in this thread Patch in this message | | | Date | Fri, 16 May 2008 09:57:15 +0200 | From | Jens Axboe <> | Subject | Re: performance "regression" in cfq compared to anticipatory, deadline and noop |
| |
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); } /* -- Jens Axboe
| |