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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTi=+yE396LPP9rfA8gkzdffaBbGjLffpzNWxfodQ@mail.gmail.com>
Date:	Fri, 11 Feb 2011 11:56:10 -0800
From:	Justin TerAvest <teravest@...gle.com>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	jaxboe@...ionio.com, ctalbott@...gle.com, mrubin@...gle.com,
	jmoyer@...hat.com, guijianfeng@...fujitsu.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] Don't update group weights when on service tree.

On Fri, Feb 11, 2011 at 10:58 AM, Vivek Goyal <vgoyal@...hat.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/