lkml.org 
[lkml]   [2009]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 4/4] cfq-iosched: fix corner cases in idling logic
From
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/

\
 
 \ /
  Last update: 2009-12-02 15:17    [W:0.149 / U:0.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site