Re: [PATCH 1/1] Don't update group weights when on service tree.

[Justin TerAvest]

On Fri, Feb 11, 2011 at 10:58 AM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Thu, Feb 10, 2011 at 01:08:06PM -0800, Justin TerAvest wrote:
>
> [..]
>> +static void
>> +cfq_update_group_weight(struct cfq_group *cfqg)
>> +{
>> +     BUG_ON(!RB_EMPTY_NODE(&cfqg->rb_node));
>> +     if (cfqg->needs_update) {
>> +             cfqg->weight = cfqg->new_weight;
>> +             cfqg->needs_update = false;
>> +     }
>> +}
>> +
>>  static inline unsigned int cfq_cfqq_slice_usage(struct cfq_queue *cfqq)
>>  {
>>       unsigned int slice_used;
>> @@ -952,9 +977,11 @@ static void cfq_group_served(struct cfq_data *cfqd, struct cfq_group *cfqg,
>>               charge = cfqq->allocated_slice;
>>
>>       /* Can't update vdisktime while group is on service tree */
>> -     cfq_rb_erase(&cfqg->rb_node, st);
>> +     cfq_group_service_tree_del(st, cfqg);
>>       cfqg->vdisktime += cfq_scale_slice(charge, cfqg);
>> -     __cfq_group_service_tree_add(st, cfqg);
>> +     /* If a new weight was requested, update now, off tree */
>> +     cfq_update_group_weight(cfqg);
>> +     cfq_group_service_tree_add(st, cfqg);
>
> Justin,
>
> Can we move cfq_update_group_weight() inside cfq_group_service_tree_add()?
> That way any time a group is added fresh to service tree, we check for
> weight updates.

Vivek,

That sounds reasonable-- I'll send a v2 patch shortly.

>
> Currently a weight update will not take effect until and unless a group
> has gone through addition/deletion cycle.
>
> If a group remains backlogged for long time, then also weight update will
> not take effect. Sadly we can't take queue lock in weight update path
> so we need to do this with some kind of worker thread. I guess we will
> deal with that later.

I agree. The weight update being delayed is still better than what
happens today.

Thanks,
Justin


> For the time being cheking for group updates more
> frequently will make sure weight update will be processed even when
> a new group gets backlogged for the first time.

>
> Thanks
> Vivek
>
--
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/