LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Justin TerAvest <teravest@google.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: jaxboe@fusionio.com, ctalbott@google.com, mrubin@google.com,
	jmoyer@redhat.com, guijianfeng@cn.fujitsu.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] Don't update group weights when on service tree.
Date: Fri, 11 Feb 2011 11:56:10 -0800	[thread overview]
Message-ID: <AANLkTi=+yE396LPP9rfA8gkzdffaBbGjLffpzNWxfodQ@mail.gmail.com> (raw)
In-Reply-To: <20110211185825.GJ8773@redhat.com>

On Fri, Feb 11, 2011 at 10:58 AM, Vivek Goyal <vgoyal@redhat.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
>

  reply	other threads:[~2011-02-11 19:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-10 21:08 [PATCH 0/1] " Justin TerAvest
2011-02-10 21:08 ` [PATCH 1/1] " Justin TerAvest
2011-02-11 18:58   ` Vivek Goyal
2011-02-11 19:56     ` Justin TerAvest [this message]
2011-02-11 18:54 ` [PATCH 0/1] " Vivek Goyal
2011-02-12  3:13 ` Gui Jianfeng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='AANLkTi=+yE396LPP9rfA8gkzdffaBbGjLffpzNWxfodQ@mail.gmail.com' \
    --to=teravest@google.com \
    --cc=ctalbott@google.com \
    --cc=guijianfeng@cn.fujitsu.com \
    --cc=jaxboe@fusionio.com \
    --cc=jmoyer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mrubin@google.com \
    --cc=vgoyal@redhat.com \
    --subject='Re: [PATCH 1/1] Don'\''t update group weights when on service tree.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).