From: Balbir Singh <balbir@linux.vnet.ibm.com> To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: "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>, "kosaki.motohiro@jp.fujitsu.com" <kosaki.motohiro@jp.fujitsu.com> Subject: Re: [RFC][PATCH 2/5] add softlimit to res_counter Date: Thu, 12 Mar 2009 09:40:38 +0530 [thread overview] Message-ID: <20090312041038.GF23583@balbir.in.ibm.com> (raw) In-Reply-To: <20090312125839.3b01e20c.kamezawa.hiroyu@jp.fujitsu.com> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-03-12 12:58:39]: > On Thu, 12 Mar 2009 09:24:44 +0530 > Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > > > > +int res_counter_set_softlimit(struct res_counter *cnt, unsigned long long val) > > > +{ > > > + unsigned long flags; > > > + > > > + spin_lock_irqsave(&cnt->lock, flags); > > > + cnt->softlimit = val; > > > + spin_unlock_irqrestore(&cnt->lock, flags); > > > + return 0; > > > +} > > > + > > > +bool res_counter_check_under_softlimit(struct res_counter *cnt) > > > +{ > > > + struct res_counter *c; > > > + unsigned long flags; > > > + bool ret = true; > > > + > > > + local_irq_save(flags); > > > + for (c = cnt; ret && c != NULL; c = c->parent) { > > > + spin_lock(&c->lock); > > > + if (c->softlimit < c->usage) > > > + ret = false; > > > > So if a child was under the soft limit and the parent is *not*, we > > _override_ ret and return false? > > > yes. If you don't want this behavior I'll rename this to > res_counter_check_under_softlimit_hierarchical(). > That is a nicer name. > > > > + spin_unlock(&c->lock); > > > + } > > > + local_irq_restore(flags); > > > + return ret; > > > +} > > > > Why is the check_under_softlimit hierarchical? > > At checking whether a mem_cgroup is a candidate for softlimit-reclaim, > we need to check all parents. > > > BTW, this patch is buggy. See above. > > > > Not buggy. Just meets my requiremnt. Correct me if I am wrong, but this boils down to checking if the top root is above it's soft limit? Instead of checking all the way up in the hierarchy, can't we do a conditional check for c->parent == NULL && (c->softlimit < c->usage) BTW, I would prefer to split the word softlimit to soft_limit, it is more readable that way. -- Balbir
WARNING: multiple messages have this Message-ID (diff)
From: Balbir Singh <balbir@linux.vnet.ibm.com> To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: "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>, "kosaki.motohiro@jp.fujitsu.com" <kosaki.motohiro@jp.fujitsu.com> Subject: Re: [RFC][PATCH 2/5] add softlimit to res_counter Date: Thu, 12 Mar 2009 09:40:38 +0530 [thread overview] Message-ID: <20090312041038.GF23583@balbir.in.ibm.com> (raw) In-Reply-To: <20090312125839.3b01e20c.kamezawa.hiroyu@jp.fujitsu.com> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2009-03-12 12:58:39]: > On Thu, 12 Mar 2009 09:24:44 +0530 > Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > > > > +int res_counter_set_softlimit(struct res_counter *cnt, unsigned long long val) > > > +{ > > > + unsigned long flags; > > > + > > > + spin_lock_irqsave(&cnt->lock, flags); > > > + cnt->softlimit = val; > > > + spin_unlock_irqrestore(&cnt->lock, flags); > > > + return 0; > > > +} > > > + > > > +bool res_counter_check_under_softlimit(struct res_counter *cnt) > > > +{ > > > + struct res_counter *c; > > > + unsigned long flags; > > > + bool ret = true; > > > + > > > + local_irq_save(flags); > > > + for (c = cnt; ret && c != NULL; c = c->parent) { > > > + spin_lock(&c->lock); > > > + if (c->softlimit < c->usage) > > > + ret = false; > > > > So if a child was under the soft limit and the parent is *not*, we > > _override_ ret and return false? > > > yes. If you don't want this behavior I'll rename this to > res_counter_check_under_softlimit_hierarchical(). > That is a nicer name. > > > > + spin_unlock(&c->lock); > > > + } > > > + local_irq_restore(flags); > > > + return ret; > > > +} > > > > Why is the check_under_softlimit hierarchical? > > At checking whether a mem_cgroup is a candidate for softlimit-reclaim, > we need to check all parents. > > > BTW, this patch is buggy. See above. > > > > Not buggy. Just meets my requiremnt. Correct me if I am wrong, but this boils down to checking if the top root is above it's soft limit? Instead of checking all the way up in the hierarchy, can't we do a conditional check for c->parent == NULL && (c->softlimit < c->usage) BTW, I would prefer to split the word softlimit to soft_limit, it is more readable that way. -- Balbir -- 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-03-12 4:11 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2009-03-12 0:52 [RFC][PATCH 0/5] memcg softlimit (Another one) v4 KAMEZAWA Hiroyuki 2009-03-12 0:52 ` KAMEZAWA Hiroyuki 2009-03-12 0:55 ` [BUGFIX][PATCH 1/5] memcg use correct scan number at reclaim KAMEZAWA Hiroyuki 2009-03-12 0:55 ` KAMEZAWA Hiroyuki 2009-03-12 3:49 ` Balbir Singh 2009-03-12 3:49 ` Balbir Singh 2009-03-12 3:51 ` KAMEZAWA Hiroyuki 2009-03-12 3:51 ` KAMEZAWA Hiroyuki 2009-03-12 4:00 ` Balbir Singh 2009-03-12 4:00 ` Balbir Singh 2009-03-12 4:05 ` KAMEZAWA Hiroyuki 2009-03-12 4:05 ` KAMEZAWA Hiroyuki 2009-03-12 4:14 ` Balbir Singh 2009-03-12 4:14 ` Balbir Singh 2009-03-12 4:17 ` KAMEZAWA Hiroyuki 2009-03-12 4:17 ` KAMEZAWA Hiroyuki 2009-03-12 7:45 ` KOSAKI Motohiro 2009-03-12 7:45 ` KOSAKI Motohiro 2009-03-12 9:45 ` Balbir Singh 2009-03-12 9:45 ` Balbir Singh 2009-03-12 11:23 ` KOSAKI Motohiro 2009-03-12 11:23 ` KOSAKI Motohiro 2009-03-12 0:56 ` [RFC][PATCH 2/5] add softlimit to res_counter KAMEZAWA Hiroyuki 2009-03-12 0:56 ` KAMEZAWA Hiroyuki 2009-03-12 3:54 ` Balbir Singh 2009-03-12 3:54 ` Balbir Singh 2009-03-12 3:58 ` KAMEZAWA Hiroyuki 2009-03-12 3:58 ` KAMEZAWA Hiroyuki 2009-03-12 4:10 ` Balbir Singh [this message] 2009-03-12 4:10 ` Balbir Singh 2009-03-12 4:14 ` KAMEZAWA Hiroyuki 2009-03-12 4:14 ` KAMEZAWA Hiroyuki 2009-03-12 0:57 ` [RFC][PATCH 3/5] memcg per zone softlimit scheduler core KAMEZAWA Hiroyuki 2009-03-12 0:57 ` KAMEZAWA Hiroyuki 2009-03-12 0:58 ` [RFC][PATCH 4/5] memcg softlimit_priority KAMEZAWA Hiroyuki 2009-03-12 0:58 ` KAMEZAWA Hiroyuki 2009-03-12 1:00 ` [RFC][PATCH 5/5] memcg softlimit hooks to kswapd KAMEZAWA Hiroyuki 2009-03-12 1:00 ` KAMEZAWA Hiroyuki 2009-03-12 3:58 ` Balbir Singh 2009-03-12 3:58 ` Balbir Singh 2009-03-12 4:02 ` KAMEZAWA Hiroyuki 2009-03-12 4:02 ` KAMEZAWA Hiroyuki 2009-03-12 4:59 ` KAMEZAWA Hiroyuki 2009-03-12 4:59 ` KAMEZAWA Hiroyuki 2009-03-12 1:01 ` [RFC][PATCH 6/5] softlimit document KAMEZAWA Hiroyuki 2009-03-12 1:01 ` KAMEZAWA Hiroyuki 2009-03-12 1:54 ` Li Zefan 2009-03-12 1:54 ` Li Zefan 2009-03-12 2:01 ` KAMEZAWA Hiroyuki 2009-03-12 2:01 ` KAMEZAWA Hiroyuki 2009-03-12 3:46 ` [RFC][PATCH 0/5] memcg softlimit (Another one) v4 Balbir Singh 2009-03-12 3:46 ` Balbir Singh 2009-03-12 4:39 ` KAMEZAWA Hiroyuki 2009-03-12 4:39 ` KAMEZAWA Hiroyuki 2009-03-12 5:04 ` Balbir Singh 2009-03-12 5:04 ` Balbir Singh 2009-03-12 5:32 ` KAMEZAWA Hiroyuki 2009-03-12 5:32 ` KAMEZAWA Hiroyuki 2009-03-12 8:26 ` Balbir Singh 2009-03-12 8:26 ` Balbir Singh 2009-03-12 8:45 ` KAMEZAWA Hiroyuki 2009-03-12 8:45 ` KAMEZAWA Hiroyuki 2009-03-12 9:53 ` Balbir Singh 2009-03-12 9:53 ` Balbir Singh 2009-03-14 18:52 ` Balbir Singh 2009-03-14 18:52 ` Balbir Singh 2009-03-16 0:10 ` KAMEZAWA Hiroyuki 2009-03-16 0:10 ` 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=20090312041038.GF23583@balbir.in.ibm.com \ --to=balbir@linux.vnet.ibm.com \ --cc=kamezawa.hiroyu@jp.fujitsu.com \ --cc=kosaki.motohiro@jp.fujitsu.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.