Messages in this thread | | | Date | Tue, 22 Mar 2011 18:06:51 -0400 | From | Vivek Goyal <> | Subject | Re: [PATCH 1/2] cfq-iosched: Don't set active queue in preempt |
| |
On Tue, Mar 22, 2011 at 02:58:48PM -0700, Justin TerAvest wrote: [..] > >> cfqd->active_queue = cfqq; > >> @@ -3332,7 +3338,8 @@ static void cfq_preempt_queue(struct cfq_data *cfqd, struct cfq_queue *cfqq) > >> BUG_ON(!cfq_cfqq_on_rr(cfqq)); > >> > >> cfq_service_tree_add(cfqd, cfqq, 1); > >> - __cfq_set_active_queue(cfqd, cfqq); > >> + > >> + cfq_clear_queue_stats(cfqd, cfqq); > > > > Hi Justin, > > > > Why do we have to clear queue stats here for the preempting queue? > > > > Especially look at cfqq->dispatch_start = jiffies. We have not started > > the dispatch yet. When this queue is selected next, then we will start > > the dispatch. > > > > So this patch has introduced another bug now. Now after preemption if > > we don't select this group, then we have a queue with wrong dispatch > > start and that will result in huge slice_used for the queue and > > it will not get its fair share. > > Hi Vivek, > > Ugh, you're right. Sorry, I had some bad ideas in my head for how > preemption worked that clearly aren't true. > I think that if the stats aren't cleared here, everything should then > be fine because jiffies will then be picked up when the active queue > is set. Does that sound sane to you? >
Yes, that makes sense. Primarily you are looking for unaccounted seek time (slice_start - dispatch_start) and dispatch_start will be set when the preempting queue is selected for dispatch. So I don't think you have to clear up anything in cfq_preempt_queue().
Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |