[PATCH] CFQ: Don't store left slice when slice used up or for a idle workload

[PATCH] CFQ: Don't store left slice when slice used up or for a idle workload

[Gui Jianfeng]

It doesn't make sence to store left time slice for an idle workload
or for the cfqq that uses up its slice.

Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
 block/cfq-iosched.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index eb4086f..d985e38 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3418,7 +3418,7 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq)
 		 * - when there is a close cooperator
 		 */
 		if (cfq_slice_used(cfqq) || cfq_class_idle(cfqq))
-			cfq_slice_expired(cfqd, 1);
+			cfq_slice_expired(cfqd, 0);
 		else if (sync && cfqq_empty &&
 			 !cfq_close_cooperator(cfqd, cfqq)) {
 			cfqd->noidle_tree_requires_idle |=
-- 
1.5.4.rc3

Re: [PATCH] CFQ: Don't store left slice when slice used up or for a idle workload

[Jeff Moyer]

Gui Jianfeng <guijianfeng@cn.fujitsu.com> writes:

> It doesn't make sence to store left time slice for an idle workload
> or for the cfqq that uses up its slice.

Did you actually observe any problems?  As I understand it, if you
overrun your slice you get a negative offset, so I think we want to keep
that.

Cheers,
Jeff

Re: [PATCH] CFQ: Don't store left slice when slice used up or for a idle workload

[Gui Jianfeng]

Jeff Moyer wrote:
> Gui Jianfeng <guijianfeng@cn.fujitsu.com> writes:
> 
>> It doesn't make sence to store left time slice for an idle workload
>> or for the cfqq that uses up its slice.
> 
> Did you actually observe any problems?  As I understand it, if you
> overrun your slice you get a negative offset, so I think we want to keep
> that.

Hi Jeff

If that's the case, do we also need to store the negative offset when slice
used up in cfq_select_queue() and cfq_idle_slice_timer()?

Thanks
Gui

> 
> Cheers,
> Jeff
> 
> 

Re: [PATCH] CFQ: Don't store left slice when slice used up or for a idle workload

[Jeff Moyer]

Gui Jianfeng <guijianfeng@cn.fujitsu.com> writes:

> Jeff Moyer wrote:
>> Gui Jianfeng <guijianfeng@cn.fujitsu.com> writes:
>> 
>>> It doesn't make sence to store left time slice for an idle workload
>>> or for the cfqq that uses up its slice.
>> 
>> Did you actually observe any problems?  As I understand it, if you
>> overrun your slice you get a negative offset, so I think we want to keep
>> that.
>
> Hi Jeff
>
> If that's the case, do we also need to store the negative offset when slice
> used up in cfq_select_queue() and cfq_idle_slice_timer()?

Good question;  the code is inconsistent as it stands.

        /*
         * store what was left of this slice, if the queue idled/timed out
         */

If we are to believe that comment, then yes, we should also call
cfq_slice_expired with timed_out set to 1 in cfq_idle_slice_timer when
the slice is used, and for certain cases in select_queue.

I find this counter-intuitive, actually.  I would have stored residual
for quite the opposite situation: where you are preempted and so don't
get to run for your fair share.  But, there must be some
logic/experience behind the current mechanism....

Cheers,
Jeff