[PATCH] cfq-iosched: avoid slice overrun when idling

[PATCH] cfq-iosched: avoid slice overrun when idling

[Corrado Zoccolo]

Idle window for a queue is reduced when the queue is about to finish
its slice.

Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
---
 block/cfq-iosched.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 4ab33d8..55bb8ca 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1105,8 +1105,10 @@ static void cfq_arm_slice_timer(struct cfq_data *cfqd)
 	 * we don't want to idle for seeks, but we do want to allow
 	 * fair distribution of slice time for a process doing back-to-back
 	 * seeks. so allow a little bit of time for him to submit a new rq
+	 * but avoid overrunning its timeslice
 	 */
-	sl = cfqd->cfq_slice_idle;
+	sl = min_t(unsigned long, cfqd->cfq_slice_idle,
+		cfqq->slice_end - jiffies);
 	if (sample_valid(cic->seek_samples) && CIC_SEEKY(cic))
 		sl = min(sl, msecs_to_jiffies(CFQ_MIN_TT));
 
-- 
1.6.2.5

Re: [PATCH] cfq-iosched: avoid slice overrun when idling

[Jens Axboe]

On Wed, Oct 07 2009, Corrado Zoccolo wrote:
> Idle window for a queue is reduced when the queue is about to finish
> its slice.
> 
> Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
> ---
>  block/cfq-iosched.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 4ab33d8..55bb8ca 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -1105,8 +1105,10 @@ static void cfq_arm_slice_timer(struct cfq_data *cfqd)
>  	 * we don't want to idle for seeks, but we do want to allow
>  	 * fair distribution of slice time for a process doing back-to-back
>  	 * seeks. so allow a little bit of time for him to submit a new rq
> +	 * but avoid overrunning its timeslice
>  	 */
> -	sl = cfqd->cfq_slice_idle;
> +	sl = min_t(unsigned long, cfqd->cfq_slice_idle,
> +		cfqq->slice_end - jiffies);
>  	if (sample_valid(cic->seek_samples) && CIC_SEEKY(cic))
>  		sl = min(sl, msecs_to_jiffies(CFQ_MIN_TT));

This was actually done this way on purpose, since shorter idling more
often don't suceed. So the logic was rather overrun the slice slightly
than wait shortly and just miss the incoming IO.

Of course this will overrun the slice even more, which the above will
also do since it wants to do IO within that time frame too.

So I think we should either leave it as-is, OR simply not arm the idle
timer when it has less than slice_idle time left and immediately select
a new queue.

-- 
Jens Axboe

Re: [PATCH] cfq-iosched: avoid slice overrun when idling

[Corrado Zoccolo]

On Wed, Oct 7, 2009 at 7:51 PM, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Wed, Oct 07 2009, Corrado Zoccolo wrote:
>> Idle window for a queue is reduced when the queue is about to finish
>> its slice.
>>
>> Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
>> ---
>>  block/cfq-iosched.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
>> index 4ab33d8..55bb8ca 100644
>> --- a/block/cfq-iosched.c
>> +++ b/block/cfq-iosched.c
>> @@ -1105,8 +1105,10 @@ static void cfq_arm_slice_timer(struct cfq_data *cfqd)
>>        * we don't want to idle for seeks, but we do want to allow
>>        * fair distribution of slice time for a process doing back-to-back
>>        * seeks. so allow a little bit of time for him to submit a new rq
>> +      * but avoid overrunning its timeslice
>>        */
>> -     sl = cfqd->cfq_slice_idle;
>> +     sl = min_t(unsigned long, cfqd->cfq_slice_idle,
>> +             cfqq->slice_end - jiffies);
>>       if (sample_valid(cic->seek_samples) && CIC_SEEKY(cic))
>>               sl = min(sl, msecs_to_jiffies(CFQ_MIN_TT));
>
> This was actually done this way on purpose, since shorter idling more
> often don't suceed. So the logic was rather overrun the slice slightly
> than wait shortly and just miss the incoming IO.
>
> Of course this will overrun the slice even more, which the above will
> also do since it wants to do IO within that time frame too.
>
> So I think we should either leave it as-is, OR simply not arm the idle
> timer when it has less than slice_idle time left and immediately select
> a new queue.

Maybe we can arm the timer only if the think time is less than the
remaining slice, otherwise skip it.

Corrado
>
> --
> Jens Axboe
>
>



-- 
__________________________________________________________________________

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

[PATCH v2] cfq-iosched: avoid probable slice overrun when idling

[Corrado Zoccolo]

Idle timer is not armed if remaining slice is less than average think
time.

Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
---
 block/cfq-iosched.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index b35cc56..af1d621 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1099,6 +1099,14 @@ static void cfq_arm_slice_timer(struct cfq_data *cfqd)
 	if (!cic || !atomic_read(&cic->ioc->nr_tasks))
 		return;
 
+	/*
+	 * think time is greater than remaining time slice
+	 * don't idle, to avoid overrunnig the time slice
+	 */
+	if (sample_valid(cic->ttime_samples) &&
+		(cfqq->slice_end - jiffies < cic->ttime_mean))
+		return;
+
 	cfq_mark_cfqq_wait_request(cfqq);
 
 	/*
-- 
1.6.2.5

Re: [PATCH v2] cfq-iosched: avoid probable slice overrun when idling

[Jens Axboe]

On Wed, Oct 07 2009, Corrado Zoccolo wrote:
> Idle timer is not armed if remaining slice is less than average think
> time.

Thanks, this looks a lot more appropriate. I will commit it, but expand
your changelog somewhat. The changelog needs to always explain the
rationale behind the change, this one is a bit on the terse side.

-- 
Jens Axboe