Re: [PATCH] cfq-iosched: reduce write depth only if sync was delayed

[Corrado Zoccolo <czoccolo@gmail.com>]

Hi Jeff,
I remember you saw large performance drop on your SAN for sequential
writes with low_latency=1.
Can you test if Shaohua's and this patch fix allow to recover some bandwidth?
I think that enabling the queue depth ramp up only if a sync request
was delayed should disable it
for fast hardware like yours, so you should not be seeing the slowdown any more.

Thanks,
Corrado

On Sat, Dec 5, 2009 at 12:13 PM, Corrado Zoccolo <czoccolo@gmail.com> wrote:
> On Fri, Dec 4, 2009 at 1:35 PM, Corrado Zoccolo <czoccolo@gmail.com> wrote:
>> The introduction of ramp-up formula for async queue depths has
>> slowed down dirty page reclaim, by reducing async write performance.
>> This patch makes sure the formula kicks in only when sync request
>> was recently delayed.
>>
>> Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
>> ---
>>  block/cfq-iosched.c |    9 +++++----
>>  1 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
>> index b00ca4c..a594388 100644
>> --- a/block/cfq-iosched.c
>> +++ b/block/cfq-iosched.c
>> @@ -284,7 +284,7 @@ struct cfq_data {
>>         */
>>        struct cfq_queue oom_cfqq;
>>
>> -       unsigned long last_end_sync_rq;
>> +       unsigned long last_delayed_sync;
>>
>>        /* List of cfq groups being managed on this device*/
>>        struct hlist_head cfqg_list;
>> @@ -2264,7 +2264,7 @@ static bool cfq_may_dispatch(struct cfq_data *cfqd,
>> struct cfq_queue *cfqq)
>>         * based on the last sync IO we serviced
>>         */
>>        if (!cfq_cfqq_sync(cfqq) && cfqd->cfq_latency) {
>> -               unsigned long last_sync = jiffies - cfqd->last_end_sync_rq;
>> +               unsigned long last_sync = jiffies - cfqd->last_delayed_sync;
>>                unsigned int depth;
>>
>>                depth = last_sync / cfqd->cfq_slice[1];
>> @@ -3272,7 +3272,8 @@ static void cfq_completed_request(struct request_queue
>> *q, struct request *rq)
>>
>>        if (sync) {
>>                RQ_CIC(rq)->last_end_request = now;
>> -               cfqd->last_end_sync_rq = now;
>> +               if (time_after(rq->start_time + cfqd->cfq_fifo_expire[1], now))
>> +                       cfqd->last_delayed_sync = now;
>
> Sorry, the check be reversed (!time_after).
>
>>        }
>>
>>        /*
>> @@ -3706,7 +3707,7 @@ static void *cfq_init_queue(struct request_queue *q)
>>        cfqd->cfq_latency = 1;
>>        cfqd->cfq_group_isolation = 0;
>>        cfqd->hw_tag = -1;
>> -       cfqd->last_end_sync_rq = jiffies;
>> +       cfqd->last_delayed_sync = jiffies - HZ;
>>        return cfqd;
>>  }
>>
>> --
>> 1.6.2.5
>>
>>
>>
>>
>



-- 
__________________________________________________________________________

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