Messages in this thread | | | Date | Wed, 2 Dec 2009 15:14:22 +0100 | Subject | Re: [PATCH 4/4] cfq-iosched: fix corner cases in idling logic | From | Corrado Zoccolo <> |
| |
Hi Jeff, On Wed, Dec 2, 2009 at 2:42 PM, Jeff Moyer <jmoyer@redhat.com> wrote: > Corrado Zoccolo <czoccolo@gmail.com> writes: > >> Idling logic was disabled in some corner cases, leading to unfair share >> for noidle queues. >> * the idle timer was not armed if there were other requests in the >> driver. unfortunately, those requests could come from other workloads, >> or queues for which we don't enable idling. So we will check only >> pending requests from the active queue >> * rq_noidle check on no-idle queue could disable the end of tree idle if >> the last completed request was rq_noidle. Now, we will disable that >> idle only if all the queues served in the no-idle tree had rq_noidle >> requests. >> >> Reported-by: Vivek Goyal <vgoyal@redhat.com> >> Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com> > >> @@ -2606,17 +2608,27 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq) >> cfq_clear_cfqq_slice_new(cfqq); >> } >> /* >> - * If there are no requests waiting in this queue, and >> - * there are other queues ready to issue requests, AND >> - * those other queues are issuing requests within our >> - * mean seek distance, give them a chance to run instead >> - * of idling. >> + * Idling is not enabled on: >> + * - expired queues >> + * - idle-priority queues >> + * - async queues >> + * - queues with still some requests queued >> + * - when there is a close cooperator >> */ > > I'm not sure this logic is correct. Is this for the 2.6.33 branch? Yes. > If so, the coop flag now means that multiple processes share the same > cfqq. Are you sure this is the right thing to do for close cooperators? I'm not sure. I didn't change the logic for close cooperators: - else if (cfqq_empty && !cfq_close_cooperator(cfqd, cfqq) && - sync && !rq_noidle(rq)) - cfq_arm_slice_timer(cfqd); + else if (sync && cfqq_empty && + !cfq_close_cooperator(cfqd, cfqq)) { + cfqd->noidle_tree_requires_idle |= !rq_noidle(rq);
I changed the rq_noidle part, and rewrote the comment to be aligned with the code. So I don't mind if you improve (or just remove) the close cooperator part. Probably, you should do a test where close cooperating processes are competing with a sequential reader, to see the effect of idling or not on them.
Thanks Corrado
> > Cheers, > Jeff
-- __________________________________________________________________________
dott. Corrado Zoccolo mailto:czoccolo@gmail.com PhD - Department of Computer Science - University of Pisa, Italy -------------------------------------------------------------------------- The self-confidence of a warrior is not the self-confidence of the average man. The average man seeks certainty in the eyes of the onlooker and calls that self-confidence. The warrior seeks impeccability in his own eyes and calls that humbleness. Tales of Power - C. Castaneda -- 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/
| |