Messages in this thread | | | Date | Thu, 12 Mar 2009 09:40:38 +0530 | From | Balbir Singh <> | Subject | Re: [RFC][PATCH 2/5] add softlimit to res_counter |
| |
* 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
| |