Re: [PATCH 2/2]block cfq: compensate preempted queue even if it has no slice assigned

[Vivek Goyal <vgoyal@redhat.com>]

On Tue, Jan 11, 2011 at 04:51:57PM +0800, Shaohua Li wrote:
> If a queue is preempted before it gets slice assigned, the queue doesn't get
> compensation, which looks unfair. For such queue, we compensate it for a whole
> slice.
> 
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> 
> ---
>  block/cfq-iosched.c |   19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> Index: linux/block/cfq-iosched.c
> ===================================================================
> --- linux.orig/block/cfq-iosched.c	2011-01-10 15:37:33.000000000 +0800
> +++ linux/block/cfq-iosched.c	2011-01-10 15:54:28.000000000 +0800
> @@ -605,8 +605,8 @@ cfq_group_slice(struct cfq_data *cfqd, s
>  	return cfq_target_latency * cfqg->weight / st->total_weight;
>  }
>  
> -static inline void
> -cfq_set_prio_slice(struct cfq_data *cfqd, struct cfq_queue *cfqq)
> +static inline unsigned
> +cfq_scaled_group_slice(struct cfq_data *cfqd, struct cfq_queue *cfqq)
>  {

Shaohua,

Above name "cfq_scaled_group_slice()" does not seem appropriate. It sounds
as if we are calculating scaled group slice length but the fact is we
are trying to come up with slice length of cfqq. So a better name might
be cfq_scaled_slice_cfqq() or cfqq_scaled_slice() something like that. 

Thanks
Vivek

>  	unsigned slice = cfq_prio_to_slice(cfqd, cfqq);
>  	if (cfqd->cfq_latency) {
> @@ -632,6 +632,14 @@ cfq_set_prio_slice(struct cfq_data *cfqd
>  				    low_slice);
>  		}
>  	}
> +	return slice;
> +}
> +
> +static inline void
> +cfq_set_prio_slice(struct cfq_data *cfqd, struct cfq_queue *cfqq)
> +{
> +	unsigned slice = cfq_scaled_group_slice(cfqd, cfqq);
> +
>  	cfqq->slice_start = jiffies;
>  	cfqq->slice_end = jiffies + slice;
>  	cfqq->allocated_slice = slice;
> @@ -1672,8 +1680,11 @@ __cfq_slice_expired(struct cfq_data *cfq
>  	/*
>  	 * store what was left of this slice, if the queue idled/timed out
>  	 */
> -	if (timed_out && !cfq_cfqq_slice_new(cfqq)) {
> -		cfqq->slice_resid = cfqq->slice_end - jiffies;
> +	if (timed_out) {
> +		if (cfq_cfqq_slice_new(cfqq))
> +			cfqq->slice_resid = cfq_scaled_group_slice(cfqd, cfqq);
> +		else
> +			cfqq->slice_resid = cfqq->slice_end - jiffies;
>  		cfq_log_cfqq(cfqd, cfqq, "resid=%ld", cfqq->slice_resid);
>  	}
>  
>