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

From: Vivek Goyal
Date: Mon Jan 17 2011 - 09:07:04 EST


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@xxxxxxxxx>
>
> ---
> 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);
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/