From: "KAMEZAWA Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com>
To: Balbir Singh <balbir@linux.vnet.ibm.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"nishimura@mxp.nes.nec.co.jp" <nishimura@mxp.nes.nec.co.jp>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Subject: Re: [mmotm][PATCH 2/2 v2] memcg: reduce calls for soft limit excess
Date: Thu, 3 Sep 2009 09:34:23 +0900 (JST) [thread overview]
Message-ID: <ff8069241ae388c64cbb2d7d8d51fe4e.squirrel@webmail-b.css.fujitsu.com> (raw)
In-Reply-To: <661de9470909021315m3af0de32h29f1ac8fd574249d@mail.gmail.com>
Balbir Singh さんは書きました：
> On Wed, Sep 2, 2009 at 11:26 AM, KAMEZAWA
> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> In charge/uncharge/reclaim path, usage_in_excess is calculated
>> repeatedly and
>> it takes res_counter's spin_lock every time.
>>
>
> I think the changelog needs to mention some refactoring you've done
> below as well, like change new_charge_in_excess to excess.
>
will do when I sent out v3. (and I'll have to do, anyway.)
Bye,
-Kame
>
>
>> This patch removes unnecessary calls for res_count_soft_limit_excess.
>>
>> Changelog:
>>  - fixed description.
>>  - fixed unsigned long to be unsigned long long (Thanks, Nishimura)
>>
>> Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> ---
>>  mm/memcontrol.c |   31 +++++++++++++++----------------
>>  1 file changed, 15 insertions(+), 16 deletions(-)
>>
>> Index: mmotm-2.6.31-Aug27/mm/memcontrol.c
>> ===================================================================
>> --- mmotm-2.6.31-Aug27.orig/mm/memcontrol.c
>> +++ mmotm-2.6.31-Aug27/mm/memcontrol.c
>> @@ -313,7 +313,8 @@ soft_limit_tree_from_page(struct page *p
>>  static void
>>  __mem_cgroup_insert_exceeded(struct mem_cgroup *mem,
>>                    
           struct mem_cgroup_per_zone
*mz,
>> -                    
          struct mem_cgroup_tree_per_zone
*mctz)
>> +                    
          struct mem_cgroup_tree_per_zone
*mctz,
>> +                    
          unsigned long long
new_usage_in_excess)
>>  {
>>        struct rb_node **p = &mctz->rb_root.rb_node;
>>        struct rb_node *parent = NULL;
>> @@ -322,7 +323,9 @@ __mem_cgroup_insert_exceeded(struct mem_
>>        if (mz->on_tree)
>>                return;
>>
>> -       mz->usage_in_excess =
res_counter_soft_limit_excess(&mem->res);
>> +       mz->usage_in_excess = new_usage_in_excess;
>> +       if (!mz->usage_in_excess)
>> +               return;
>>        while (*p) {
>>                parent = *p;
>>                mz_node =
rb_entry(parent, struct mem_cgroup_per_zone,
>> @@ -382,7 +385,7 @@ static bool mem_cgroup_soft_limit_check(
>>
>>  static void mem_cgroup_update_tree(struct mem_cgroup *mem, struct
page *page)
>>  {
>> -       unsigned long long new_usage_in_excess;
>> +       unsigned long long excess;
>>        struct mem_cgroup_per_zone *mz;
>>        struct mem_cgroup_tree_per_zone *mctz;
>>        int nid = page_to_nid(page);
>> @@ -395,25 +398,21 @@ static void mem_cgroup_update_tree(struc
>>         */
>>        for (; mem; mem = parent_mem_cgroup(mem)) {
>>                mz =
mem_cgroup_zoneinfo(mem, nid, zid);
>> -               new_usage_in_excess =
>> -                    
  res_counter_soft_limit_excess(&mem->res);
>> +               excess =
res_counter_soft_limit_excess(&mem->res);
>>                /*
>>                 * We have to
update the tree if mz is on RB-tree or
>>                 * mem is over
its softlimit.
>>                 */
>> -               if
(new_usage_in_excess || mz->on_tree) {
>> +               if (excess ||
mz->on_tree) {
>>                    
   spin_lock(&mctz->lock);
>>                    
   /* if on-tree, remove it */
>>                    
   if (mz->on_tree)
>>                    
         
 __mem_cgroup_remove_exceeded(mem, mz, mctz);
>>                    
   /*
>> -                    
   * if over soft limit, insert again. mz->usage_in_excess
>> -                    
   * will be updated properly.
>> +                    
   * Insert again. mz->usage_in_excess will be updated.
>> +                    
   * If excess is 0, no tree ops.
>>                    
    */
>> -                    
  if (new_usage_in_excess)
>> -                    
          __mem_cgroup_insert_exceeded(mem,
mz, mctz);
>> -                    
  else
>> -                    
          mz->usage_in_excess = 0;
>> +                    
  __mem_cgroup_insert_exceeded(mem, mz, mctz, excess);
>>                    
   spin_unlock(&mctz->lock);
>>                }
>>        }
>> @@ -2216,6 +2215,7 @@ unsigned long mem_cgroup_soft_limit_recl
>>        unsigned long reclaimed;
>>        int loop = 0;
>>        struct mem_cgroup_tree_per_zone *mctz;
>> +       unsigned long long excess;
>>
>>        if (order > 0)
>>                return 0;
>> @@ -2260,9 +2260,8 @@ unsigned long mem_cgroup_soft_limit_recl
>>                    
         
 __mem_cgroup_largest_soft_limit_node(mctz);
>>                    
   } while (next_mz == mz);
>>                }
>> -               mz->usage_in_excess =
>> -                    
  res_counter_soft_limit_excess(&mz->mem->res);
>>              
 __mem_cgroup_remove_exceeded(mz->mem, mz, mctz);
>> +               excess =
res_counter_soft_limit_excess(&mz->mem->res);
>>                /*
>>                 * One school of
thought says that we should not add
>>                 * back the node
to the tree if reclaim returns 0.
>> @@ -2271,8 +2270,8 @@ unsigned long mem_cgroup_soft_limit_recl
>>                 * memory to
reclaim from. Consider this as a longer
>>                 * term TODO.
>>                 */
>> -               if
(mz->usage_in_excess)
>> -                    
  __mem_cgroup_insert_exceeded(mz->mem, mz, mctz);
>> +               /* If excess == 0,
no tree ops */
>> +              
__mem_cgroup_insert_exceeded(mz->mem, mz, mctz, excess);
>>              
 spin_unlock(&mctz->lock);
>>              
 css_put(&mz->mem->css);
>>                loop++;
>
> OK.. so everytime we call __mem_cgroup_insert_exceeded we save one
> res_counter operation.
>
> Looks good
>
> Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
>
> Balbir Singh
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2009-09-03 0:34 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-02 0:34 [mmotm][PATCH 1/2] memcg: softlimit clean up KAMEZAWA Hiroyuki
2009-09-02 0:35 ` [mmotm][PATCH 2/2] memcg: reduce calls for soft limit excess KAMEZAWA Hiroyuki
2009-09-02 5:16 ` Daisuke Nishimura
2009-09-02 5:20 ` KAMEZAWA Hiroyuki
2009-09-02 5:56 ` [mmotm][PATCH 2/2 v2] " KAMEZAWA Hiroyuki
2009-09-02 20:15 ` Balbir Singh
2009-09-03 0:34 ` KAMEZAWA Hiroyuki [this message]
2009-09-02 3:56 ` [mmotm][PATCH 1/2] memcg: softlimit clean up Daisuke Nishimura
2009-09-02 4:41 ` [mmotm][PATCH] coalescing uncharge KAMEZAWA Hiroyuki
2009-09-02 6:15 ` KAMEZAWA Hiroyuki
2009-09-02 9:29 ` [mmotm][experimental][PATCH] coalescing charge KAMEZAWA Hiroyuki
2009-09-02 19:58 ` Balbir Singh
2009-09-03 0:24 ` KAMEZAWA Hiroyuki
2009-09-03 5:17 ` Daisuke Nishimura
2009-09-04 4:18 ` KAMEZAWA Hiroyuki
2009-09-04 5:11 ` Daisuke Nishimura
2009-09-04 5:21 ` KAMEZAWA Hiroyuki
2009-09-04 5:26 ` KAMEZAWA Hiroyuki
2009-09-04 6:40 ` Daisuke Nishimura
2009-09-04 6:50 ` KAMEZAWA Hiroyuki
2009-09-04 7:37 ` [mmotm][BUGFIX][PATCH] memcg: fix softlimit css refcnt handling KAMEZAWA Hiroyuki
2009-09-04 7:45 ` KAMEZAWA Hiroyuki
2009-09-04 10:07 ` Daisuke Nishimura
2009-09-06 23:04 ` [mmotm][BUGFIX][PATCH] memcg: fix softlimit css refcnt handling(yet another one) Daisuke Nishimura
2009-09-07 0:49 ` KAMEZAWA Hiroyuki
2009-09-07 4:51 ` Balbir Singh
2009-09-04 5:47 ` [mmotm][experimental][PATCH] coalescing charge KAMEZAWA Hiroyuki
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=ff8069241ae388c64cbb2d7d8d51fe4e.squirrel@webmail-b.css.fujitsu.com \
--to=kamezawa.hiroyu@jp.fujitsu.com \
--cc=akpm@linux-foundation.org \
--cc=balbir@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nishimura@mxp.nes.nec.co.jp \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox