[PATCH 3/6] CKRM: Add limit support for mem controller

[PATCH 3/6] CKRM: Add limit support for mem controller

[Chandra Seetharaman]

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]


-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------

[-- Attachment #2: 11-03-mem_core-limit --]
[-- Type: text/plain, Size: 7744 bytes --]

Patch 3 of 6 patches to support memory controller under CKRM framework.
This patch provides the limit support for teh controller.

 include/linux/ckrm_mem.h   |    2 
 kernel/ckrm/ckrm_memcore.c |  159 +++++++++++++++++++++++++++++++++++++++++++--
 kernel/ckrm/ckrm_memctlr.c |   16 ++++
 3 files changed, 172 insertions(+), 5 deletions(-)

Index: linux-2.6.12-rc1/include/linux/ckrm_mem.h
===================================================================
--- linux-2.6.12-rc1.orig/include/linux/ckrm_mem.h
+++ linux-2.6.12-rc1/include/linux/ckrm_mem.h
@@ -44,6 +44,8 @@ struct ckrm_mem_res {
 					 * parent if more than this is needed.
 					 */
 	int hier;			/* hiearchy level, root = 0 */
+	int impl_guar;			/* for classes with don't care guar */
+	int nr_dontcare;		/* # of dont care children */
 };
 
 extern atomic_t ckrm_mem_real_count;
Index: linux-2.6.12-rc1/kernel/ckrm/ckrm_memcore.c
===================================================================
--- linux-2.6.12-rc1.orig/kernel/ckrm/ckrm_memcore.c
+++ linux-2.6.12-rc1/kernel/ckrm/ckrm_memcore.c
@@ -95,9 +95,46 @@ mem_res_initcls_one(struct ckrm_mem_res 
 	INIT_LIST_HEAD(&res->mcls_list);
 
 	res->pg_unused = 0;
+	res->nr_dontcare = 1; /* for default class */
 	kref_init(&res->nr_users);
 }
 
+static void
+set_impl_guar_children(struct ckrm_mem_res *parres)
+{
+	struct ckrm_core_class *child = NULL;
+	struct ckrm_mem_res *cres;
+	int nr_dontcare = 1; /* for defaultclass */
+	int guar, impl_guar;
+	int resid = mem_rcbs.resid;
+
+	ckrm_lock_hier(parres->core);
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		/* treat NULL cres as don't care as that child is just being
+		 * created.
+		 * FIXME: need a better way to handle this case.
+		 */
+		if (!cres || cres->pg_guar == CKRM_SHARE_DONTCARE)
+			nr_dontcare++;
+	}
+
+	parres->nr_dontcare = nr_dontcare;
+	guar = (parres->pg_guar == CKRM_SHARE_DONTCARE) ?
+			parres->impl_guar : parres->pg_unused;
+	impl_guar = guar / parres->nr_dontcare;
+
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		if (cres && cres->pg_guar == CKRM_SHARE_DONTCARE) {
+			cres->impl_guar = impl_guar;
+			set_impl_guar_children(cres);
+		}
+	}
+	ckrm_unlock_hier(parres->core);
+
+}
+
 static void *
 mem_res_alloc(struct ckrm_core_class *core, struct ckrm_core_class *parent)
 {
@@ -139,14 +176,106 @@ mem_res_alloc(struct ckrm_core_class *co
 			res->pg_limit = ckrm_tot_lru_pages;
 			res->hier = 0;
 			ckrm_mem_root_class = res;
-		} else
+		} else {
+			int guar;
 			res->hier = pres->hier + 1;
+			set_impl_guar_children(pres);
+			guar = (pres->pg_guar == CKRM_SHARE_DONTCARE) ?
+				pres->impl_guar : pres->pg_unused;
+			res->impl_guar = guar / pres->nr_dontcare;
+		}
 		ckrm_nr_mem_classes++;
 	} else
 		printk(KERN_ERR "MEM_RC: alloc: GFP_ATOMIC failed\n");
 	return res;
 }
 
+/*
+ * It is the caller's responsibility to make sure that the parent only
+ * has chilren that are to be accounted. i.e if a new child is added
+ * this function should be called after it has been added, and if a
+ * child is deleted this should be called after the child is removed.
+ */
+static void
+child_maxlimit_changed_local(struct ckrm_mem_res *parres)
+{
+	int maxlimit = 0;
+	struct ckrm_mem_res *childres;
+	struct ckrm_core_class *child = NULL;
+
+	/* run thru parent's children and get new max_limit of parent */
+	ckrm_lock_hier(parres->core);
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		childres = ckrm_get_res_class(child, mem_rcbs.resid,
+				struct ckrm_mem_res);
+		if (maxlimit < childres->shares.my_limit)
+			maxlimit = childres->shares.my_limit;
+	}
+	ckrm_unlock_hier(parres->core);
+	parres->shares.cur_max_limit = maxlimit;
+}
+
+/*
+ * Recalculate the guarantee and limit in # of pages... and propagate the
+ * same to children.
+ * Caller is responsible for protecting res and for the integrity of parres
+ */
+static void
+recalc_and_propagate(struct ckrm_mem_res * res, struct ckrm_mem_res * parres)
+{
+	struct ckrm_core_class *child = NULL;
+	struct ckrm_mem_res *cres;
+	int resid = mem_rcbs.resid;
+	struct ckrm_shares *self = &res->shares;
+
+	if (parres) {
+		struct ckrm_shares *par = &parres->shares;
+
+		/* calculate pg_guar and pg_limit */
+		if (parres->pg_guar == CKRM_SHARE_DONTCARE ||
+				self->my_guarantee == CKRM_SHARE_DONTCARE) {
+			res->pg_guar = CKRM_SHARE_DONTCARE;
+		} else if (par->total_guarantee) {
+			u64 temp = (u64) self->my_guarantee * parres->pg_guar;
+			do_div(temp, par->total_guarantee);
+			res->pg_guar = (int) temp;
+			res->impl_guar = CKRM_SHARE_DONTCARE;
+		} else {
+			res->pg_guar = 0;
+			res->impl_guar = CKRM_SHARE_DONTCARE;
+		}
+
+		if (parres->pg_limit == CKRM_SHARE_DONTCARE ||
+				self->my_limit == CKRM_SHARE_DONTCARE) {
+			res->pg_limit = CKRM_SHARE_DONTCARE;
+		} else if (par->max_limit) {
+			u64 temp = (u64) self->my_limit * parres->pg_limit;
+			do_div(temp, par->max_limit);
+			res->pg_limit = (int) temp;
+		} else
+			res->pg_limit = 0;
+	}
+
+	/* Calculate unused units */
+	if (res->pg_guar == CKRM_SHARE_DONTCARE)
+		res->pg_unused = CKRM_SHARE_DONTCARE;
+	else if (self->total_guarantee) {
+		u64 temp = (u64) self->unused_guarantee * res->pg_guar;
+		do_div(temp, self->total_guarantee);
+		res->pg_unused = (int) temp;
+	} else
+		res->pg_unused = 0;
+
+	/* propagate to children */
+	ckrm_lock_hier(res->core);
+	while ((child = ckrm_get_next_child(res->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		recalc_and_propagate(cres, res);
+	}
+	ckrm_unlock_hier(res->core);
+	return;
+}
+
 static void
 mem_res_free(void *my_res)
 {
@@ -161,6 +290,14 @@ mem_res_free(void *my_res)
 	pres = ckrm_get_res_class(res->parent, mem_rcbs.resid,
 			struct ckrm_mem_res);
 
+	if (pres) {
+		child_guarantee_changed(&pres->shares,
+				res->shares.my_guarantee, 0);
+		child_maxlimit_changed_local(pres);
+		recalc_and_propagate(pres, NULL);
+		set_impl_guar_children(pres);
+	}
+
 	/*
 	 * Making it all zero as freeing of data structure could 
 	 * happen later.
@@ -186,13 +323,24 @@ static int
 mem_set_share_values(void *my_res, struct ckrm_shares *shares)
 {
 	struct ckrm_mem_res *res = my_res;
+	struct ckrm_mem_res *parres;
+	int rc;
 
 	if (!res)
 		return -EINVAL;
 
-	printk(KERN_INFO "set_share called for %s resource of class %s\n",
-			MEM_RES_NAME, res->core->name);
-	return 0;
+	parres = ckrm_get_res_class(res->parent, mem_rcbs.resid,
+		struct ckrm_mem_res);
+
+	rc = set_shares(shares, &res->shares, parres ? &parres->shares : NULL);
+
+	if ((rc == 0) && (parres != NULL)) {
+		child_maxlimit_changed_local(parres);
+		recalc_and_propagate(parres, NULL);
+		set_impl_guar_children(parres);
+	}
+
+	return rc;
 }
 
 static int
Index: linux-2.6.12-rc1/kernel/ckrm/ckrm_memctlr.c
===================================================================
--- linux-2.6.12-rc1.orig/kernel/ckrm/ckrm_memctlr.c
+++ linux-2.6.12-rc1/kernel/ckrm/ckrm_memctlr.c
@@ -66,7 +66,20 @@ decr_use_count(struct ckrm_mem_res *cls,
 int
 ckrm_class_limit_ok(struct ckrm_mem_res *cls)
 {
-	return 1; /* stub for now */
+	int ret, i, pg_total = 0;
+
+	if ((mem_rcbs.resid == -1) || !cls)
+		return 1;
+	for (i = 0; i < MAX_NR_ZONES; i++)
+		pg_total += cls->pg_total[i];
+	if (cls->pg_limit == CKRM_SHARE_DONTCARE) {
+		struct ckrm_mem_res *parcls = ckrm_get_res_class(cls->parent,
+					mem_rcbs.resid, struct ckrm_mem_res);
+		ret = (parcls ? ckrm_class_limit_ok(parcls) : 0);
+	} else
+		ret = (pg_total <= cls->pg_limit);
+
+	return ret;
 }
 
 static void migrate_list(struct list_head *list,

Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Dave Hansen]

On Fri, 2005-04-01 at 19:13 -0800, Chandra Seetharaman wrote:
> +static void
> +set_impl_guar_children(struct ckrm_mem_res *parres)
> +{
> +       struct ckrm_core_class *child = NULL;
> +       struct ckrm_mem_res *cres;
> +       int nr_dontcare = 1; /* for defaultclass */
> +       int guar, impl_guar;
> +       int resid = mem_rcbs.resid;

This sets off one of my internal "this is just wrong" checks: too many
variables for a function that short.  Can that function be broken up a
bit?

Also, I get a little nervous when I see variable names like "parres" and
"mem_rcbs".  Can they get some real names?  If they're going to be
nonsense, at least make it understandable, like 

>+               cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);

I think there are probably enough calls to ckrm_get_res_class() with
just the memory controller struct to justify wrapping it in its own
macro.  sysfs does lots of tricks like this, and that's the approach
that it takes to hide some of the ugliness.

> +       while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {

You might even want a for_each_child() macro or something.  That looks a
little hairy.

> 
> +               /* treat NULL cres as don't care as that child is just
> being
> +                * created.
> +                * FIXME: need a better way to handle this case.
> +                */
> +               if (!cres || cres->pg_guar == CKRM_SHARE_DONTCARE)
> +                       nr_dontcare++;

If it needs to be fixed, why did you post it?

> +       parres->nr_dontcare = nr_dontcare;
> +       guar = (parres->pg_guar == CKRM_SHARE_DONTCARE) ?
> +                       parres->impl_guar : parres->pg_unused;
> +       impl_guar = guar / parres->nr_dontcare;

Please don't tell me this is too messy:
        
        parres->nr_dontcare = nr_dontcare;
        if (parres->pg_guar == CKRM_SHARE_DONTCARE)
        	guar = parres->impl_guar;
        else
        	guar = parres->pg_unused;
        impl_guar = guar / parres->nr_dontcare;

All of this 'impl' stufff just looks weird.  I might even wrap that
CKRM_SHARE_DONTCARE logic in a little function.  get_child_guarantee()?

All of the logic surrounding that pg_{limit,guar} being set to DONTCARE
seems like it was special-cased in after it was originally written.
Like someone went back over it, adding conditionals everywhere.  It
greatly adds to the clutter.

"DONTCARE" is also multiplexed.  It means "no guarantee" or "no limit"
depending on context.  I don't think it would hurt to have one variable
for each of these cases.

What does "impl" stand for, anyway?  implied?  implicit? implemented?

-- Dave

--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Chandra Seetharaman]

On Mon, Apr 04, 2005 at 07:10:50AM -0700, Dave Hansen wrote:
> On Fri, 2005-04-01 at 19:13 -0800, Chandra Seetharaman wrote:
> > +static void
> > +set_impl_guar_children(struct ckrm_mem_res *parres)
> > +{
> > +       struct ckrm_core_class *child = NULL;
> > +       struct ckrm_mem_res *cres;
> > +       int nr_dontcare = 1; /* for defaultclass */
> > +       int guar, impl_guar;
> > +       int resid = mem_rcbs.resid;
> 
> This sets off one of my internal "this is just wrong" checks: too many
> variables for a function that short.  Can that function be broken up a
> bit?

It doesn't make sense to break the function to a smaller one, and is an
atomic one. nevertheless, it won't do any good to the number of variables :)
I 'll look at what can be done for teh number of variables.
> 
> Also, I get a little nervous when I see variable names like "parres" and
> "mem_rcbs".  Can they get some real names?  If they're going to be
parres - parent resource data structure(do differentiate from parent core
class data structure)
mem_rcbs - memory resource's callback structure
Will think about good names.
> nonsense, at least make it understandable, like 

Just wondering... how can a thing be both understandable and nonsense :)
> 
> >+               cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
> 
> I think there are probably enough calls to ckrm_get_res_class() with
> just the memory controller struct to justify wrapping it in its own
> macro.  sysfs does lots of tricks like this, and that's the approach
> that it takes to hide some of the ugliness.

Good idea... 
> 
> > +       while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
> 
> You might even want a for_each_child() macro or something.  That looks a
> little hairy.

Good idea....
> 
> > 
> > +               /* treat NULL cres as don't care as that child is just
> > being
> > +                * created.
> > +                * FIXME: need a better way to handle this case.
> > +                */
> > +               if (!cres || cres->pg_guar == CKRM_SHARE_DONTCARE)
> > +                       nr_dontcare++;
> 
> If it needs to be fixed, why did you post it?

Because, I didn't want to wait (to post) until I fixed everything.

> 
> > +       parres->nr_dontcare = nr_dontcare;
> > +       guar = (parres->pg_guar == CKRM_SHARE_DONTCARE) ?
> > +                       parres->impl_guar : parres->pg_unused;
> > +       impl_guar = guar / parres->nr_dontcare;
> 
> Please don't tell me this is too messy:
>         
>         parres->nr_dontcare = nr_dontcare;
>         if (parres->pg_guar == CKRM_SHARE_DONTCARE)
>         	guar = parres->impl_guar;
>         else
>         	guar = parres->pg_unused;
>         impl_guar = guar / parres->nr_dontcare;
> 
> All of this 'impl' stufff just looks weird.  I might even wrap that
> CKRM_SHARE_DONTCARE logic in a little function.  get_child_guarantee()?
> 
> All of the logic surrounding that pg_{limit,guar} being set to DONTCARE
> seems like it was special-cased in after it was originally written.
> Like someone went back over it, adding conditionals everywhere.  It
> greatly adds to the clutter.

hmm... will look into it.
> 
> "DONTCARE" is also multiplexed.  It means "no guarantee" or "no limit"
> depending on context.  I don't think it would hurt to have one variable
> for each of these cases.

It is agnostic... and the name doesn't suggest one way or other... so, I
don't see a problem in multiplexing it.

> 
> What does "impl" stand for, anyway?  implied?  implicit? implemented?

I meant implicit... you can also say implied.... will add in comments to
the dats structure definition.

> 
> -- Dave
> 

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Dave Hansen]

On Tue, 2005-04-05 at 10:42 -0700, Chandra Seetharaman wrote:
> On Mon, Apr 04, 2005 at 07:10:50AM -0700, Dave Hansen wrote:
> > "DONTCARE" is also multiplexed.  It means "no guarantee" or "no limit"
> > depending on context.  I don't think it would hurt to have one variable
> > for each of these cases.
> 
> It is agnostic... and the name doesn't suggest one way or other... so, I
> don't see a problem in multiplexing it.

I think that variable names should be as suggestive as possible.  *So*
suggestive that I know what they actually do. :)

> > What does "impl" stand for, anyway?  implied?  implicit? implemented?
> 
> I meant implicit... you can also say implied.... will add in comments to
> the dats structure definition.

How about changing the name of the structure member?  Comments suck.

-- Dave

--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Chandra Seetharaman]

On Tue, Apr 05, 2005 at 10:59:02AM -0700, Dave Hansen wrote:
> On Tue, 2005-04-05 at 10:42 -0700, Chandra Seetharaman wrote:
> > On Mon, Apr 04, 2005 at 07:10:50AM -0700, Dave Hansen wrote:
> > > "DONTCARE" is also multiplexed.  It means "no guarantee" or "no limit"
> > > depending on context.  I don't think it would hurt to have one variable
> > > for each of these cases.
> > 
> > It is agnostic... and the name doesn't suggest one way or other... so, I
> > don't see a problem in multiplexing it.
> 
> I think that variable names should be as suggestive as possible.  *So*
> suggestive that I know what they actually do. :)

I think you mean the macro... It does mean it.... it is a DONT CARE :) be
it limit or guarantee...

> 
> > > What does "impl" stand for, anyway?  implied?  implicit? implemented?
> > 
> > I meant implicit... you can also say implied.... will add in comments to
> > the dats structure definition.
> 
> How about changing the name of the structure member?  Comments suck.

you mean explicit name like implicit_guarantee ? if comments suck, IMHO,
impl_guar is good enough an option for a field that holds implicit
guarantee.
> 
> -- Dave
> 

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [ckrm-tech] Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Dave Hansen]

On Tue, 2005-04-05 at 11:26 -0700, Chandra Seetharaman wrote:
> On Tue, Apr 05, 2005 at 10:59:02AM -0700, Dave Hansen wrote:
> > On Tue, 2005-04-05 at 10:42 -0700, Chandra Seetharaman wrote:
> > > On Mon, Apr 04, 2005 at 07:10:50AM -0700, Dave Hansen wrote: 
> > > > What does "impl" stand for, anyway?  implied?  implicit? implemented?
> > > 
> > > I meant implicit... you can also say implied.... will add in comments to
> > > the dats structure definition.
> > 
> > How about changing the name of the structure member?  Comments suck.
> 
> you mean explicit name like implicit_guarantee ? if comments suck, IMHO,
> impl_guar is good enough an option for a field that holds implicit
> guarantee.

I think you possibly suffer from an a case of ibmersdontlikevowelsitis
which seems to be endemic to our company.  In case you're wondering,
Linus already found our missing vowels:

	http://www.ussg.iu.edu/hypermail/linux/kernel/0110.1/1294.html

-- Dave

--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [ckrm-tech] Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Chandra Seetharaman]

On Tue, Apr 05, 2005 at 12:01:45PM -0700, Dave Hansen wrote:
> On Tue, 2005-04-05 at 11:26 -0700, Chandra Seetharaman wrote:
> > On Tue, Apr 05, 2005 at 10:59:02AM -0700, Dave Hansen wrote:
> > > On Tue, 2005-04-05 at 10:42 -0700, Chandra Seetharaman wrote:
> > > > On Mon, Apr 04, 2005 at 07:10:50AM -0700, Dave Hansen wrote: 
> > > > > What does "impl" stand for, anyway?  implied?  implicit? implemented?
> > > > 
> > > > I meant implicit... you can also say implied.... will add in comments to
> > > > the dats structure definition.
> > > 
> > > How about changing the name of the structure member?  Comments suck.
> > 
> > you mean explicit name like implicit_guarantee ? if comments suck, IMHO,
> > impl_guar is good enough an option for a field that holds implicit
> > guarantee.
> 
> I think you possibly suffer from an a case of ibmersdontlikevowelsitis
> which seems to be endemic to our company.  In case you're wondering,
> Linus already found our missing vowels:
> 
> 	http://www.ussg.iu.edu/hypermail/linux/kernel/0110.1/1294.html

To my knowledge, 'i', 'u' and 'a' are vowels(impl_guar)....  May be I
am wrong.. I think I 've relearn my alphabets.

BTW, Can you point me to the latest version ?

Seriously, I don't get your argument that 'impl_guar' doesn't imply
implicit guarantee....(especially even after I agreed that I would add some
comments)

> 
> -- Dave
> 

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [ckrm-tech] Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Chandra Seetharaman]

On Tue, Apr 05, 2005 at 01:29:45PM -0700, Matthew Helsley wrote:
> On Tue, 2005-04-05 at 12:48, Chandra Seetharaman wrote:
> > On Tue, Apr 05, 2005 at 12:01:45PM -0700, Dave Hansen wrote:
> > > On Tue, 2005-04-05 at 11:26 -0700, Chandra Seetharaman wrote:
> > > > On Tue, Apr 05, 2005 at 10:59:02AM -0700, Dave Hansen wrote:
> > > > > On Tue, 2005-04-05 at 10:42 -0700, Chandra Seetharaman wrote:
> > > > > > On Mon, Apr 04, 2005 at 07:10:50AM -0700, Dave Hansen wrote: 
> > > > > > > What does "impl" stand for, anyway?  implied?  implicit? implemented?
> > > > > > 
> > > > > > I meant implicit... you can also say implied.... will add in comments to
> > > > > > the dats structure definition.
> > > > > 
> > > > > How about changing the name of the structure member?  Comments suck.
> > > > 
> > > > you mean explicit name like implicit_guarantee ? if comments suck, IMHO,
> > > > impl_guar is good enough an option for a field that holds implicit
> > > > guarantee.
> > > 
> > > I think you possibly suffer from an a case of ibmersdontlikevowelsitis
> > > which seems to be endemic to our company.  In case you're wondering,
> > > Linus already found our missing vowels:
> > > 
> > > 	http://www.ussg.iu.edu/hypermail/linux/kernel/0110.1/1294.html
> > 
> > To my knowledge, 'i', 'u' and 'a' are vowels(impl_guar)....  May be I
> > am wrong.. I think I 've relearn my alphabets.
> 
> 	While I agree with Dave's point about 'impl' being vague, I think he's
> just being snarky about the vowels. :)
> 
> > BTW, Can you point me to the latest version ?
> > 
> > Seriously, I don't get your argument that 'impl_guar' doesn't imply
> > implicit guarantee....(especially even after I agreed that I would add some
> > comments)
> 
> 	I think Dave's point is "impl" is an ambiguous prefix. If the intent is
> "implied" or "implicit" then the extra letters are well worth the
> clarification they offer -- more so than any amount of comments since
> the field name will get used in numerous places where the comment is not
> likely to appear.

Adding 3 characters doesn't cost much... but... in this context, what
else could one think "impl" mean ?

One will get the idea if you assume one of implicit implied, or implemented...


> Cheers,
> 	-Matt
> 

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [ckrm-tech] Re: [PATCH 3/6] CKRM: Add limit support for mem controller

[Gerrit Huizenga]

On Wed, 06 Apr 2005 09:48:56 PDT, Chandra Seetharaman wrote:
> On Tue, Apr 05, 2005 at 01:29:45PM -0700, Matthew Helsley wrote:
> > On Tue, 2005-04-05 at 12:48, Chandra Seetharaman wrote:
> > > On Tue, Apr 05, 2005 at 12:01:45PM -0700, Dave Hansen wrote:
> > > > On Tue, 2005-04-05 at 11:26 -0700, Chandra Seetharaman wrote:
> > > > > On Tue, Apr 05, 2005 at 10:59:02AM -0700, Dave Hansen wrote:
> > > > > > On Tue, 2005-04-05 at 10:42 -0700, Chandra Seetharaman wrote:
> > > > > > > On Mon, Apr 04, 2005 at 07:10:50AM -0700, Dave Hansen wrote: 
> > > > > > > > What does "impl" stand for, anyway?  implied?  implicit? implemented?
> > > > > > > 
> > > > > > > I meant implicit... you can also say implied.... will add in comments to
> > > > > > > the dats structure definition.
> > > > > > 
> > > > > > How about changing the name of the structure member?  Comments suck.
> > > > > 
> > > > > you mean explicit name like implicit_guarantee ? if comments suck, IMHO,
> > > > > impl_guar is good enough an option for a field that holds implicit
> > > > > guarantee.
> > > > 
> > > > I think you possibly suffer from an a case of ibmersdontlikevowelsitis
> > > > which seems to be endemic to our company.  In case you're wondering,
> > > > Linus already found our missing vowels:
> > > > 
> > > > 	http://www.ussg.iu.edu/hypermail/linux/kernel/0110.1/1294.html
> > > 
> > > To my knowledge, 'i', 'u' and 'a' are vowels(impl_guar)....  May be I
> > > am wrong.. I think I 've relearn my alphabets.
> > 
> > 	While I agree with Dave's point about 'impl' being vague, I think he's
> > just being snarky about the vowels. :)
> > 
> > > BTW, Can you point me to the latest version ?
> > > 
> > > Seriously, I don't get your argument that 'impl_guar' doesn't imply
> > > implicit guarantee....(especially even after I agreed that I would add some
> > > comments)
> > 
> > 	I think Dave's point is "impl" is an ambiguous prefix. If the intent is
> > "implied" or "implicit" then the extra letters are well worth the
> > clarification they offer -- more so than any amount of comments since
> > the field name will get used in numerous places where the comment is not
> > likely to appear.
> 
> Adding 3 characters doesn't cost much... but... in this context, what
> else could one think "impl" mean ?

Okay - remember that people with no context will occasionally be reading
this code.  So, better names are, well, better.  Being ambiguous with
multiple meaning obviously leads to questions like these.  And for
every question asked, there are 9 more that weren't asked.  So, my
strong recommendation is make the names clear, consistent, and use
them as your first line of documentation defense.

Keep in mind that comments get out of date, where code tends not
to if it is actually used.  ;)  So, fixing the variable names now is
cheap.  This much debate about something indicates a problem that
should be fixed.

gerrit
--
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:"aart@kvack.org"> aart@kvack.org </a>

[PATCH 3/6] CKRM: Add limit support for mem controller

[Chandra Seetharaman]

Patch 3 of 6 patches to support memory controller under CKRM framework.
This patch provides the limit support for the controller.
----------------------------------------


 include/linux/ckrm_mem.h   |    2 
 kernel/ckrm/ckrm_memcore.c |  156 +++++++++++++++++++++++++++++++++++++
++++++--
 kernel/ckrm/ckrm_memctlr.c |   15 ++++
 3 files changed, 168 insertions(+), 5 deletions(-)

Content-Disposition: inline; filename=11-03-mem_core-limit
Index: linux-2.6.12/include/linux/ckrm_mem.h
===================================================================
--- linux-2.6.12.orig/include/linux/ckrm_mem.h
+++ linux-2.6.12/include/linux/ckrm_mem.h
@@ -44,6 +44,8 @@ struct ckrm_mem_res {
 					 * parent if more than this is needed.
 					 */
 	int hier;			/* hiearchy level, root = 0 */
+	int implicit_guar;		/* for classes with don't care guar */
+	int nr_dontcare;		/* # of dont care children */
 };
 
 extern atomic_t ckrm_mem_real_count;
Index: linux-2.6.12/kernel/ckrm/ckrm_memcore.c
===================================================================
--- linux-2.6.12.orig/kernel/ckrm/ckrm_memcore.c
+++ linux-2.6.12/kernel/ckrm/ckrm_memcore.c
@@ -95,9 +95,46 @@ mem_res_initcls_one(struct ckrm_mem_res 
 	INIT_LIST_HEAD(&res->mcls_list);
 
 	res->pg_unused = 0;
+	res->nr_dontcare = 1; /* for default class */
 	kref_init(&res->nr_users);
 }
 
+static void
+set_impl_guar_children(struct ckrm_mem_res *parres)
+{
+	struct ckrm_core_class *child = NULL;
+	struct ckrm_mem_res *cres;
+	int nr_dontcare = 1; /* for defaultclass */
+	int guar, impl_guar;
+	int resid = mem_rcbs.resid;
+
+	ckrm_lock_hier(parres->core);
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		/* treat NULL cres as don't care as that child is just being
+		 * created.
+		 * FIXME: need a better way to handle this case.
+		 */
+		if (!cres || cres->pg_guar == CKRM_SHARE_DONTCARE)
+			nr_dontcare++;
+	}
+
+	parres->nr_dontcare = nr_dontcare;
+	guar = (parres->pg_guar == CKRM_SHARE_DONTCARE) ?
+			parres->implicit_guar : parres->pg_unused;
+	impl_guar = guar / parres->nr_dontcare;
+
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		if (cres && cres->pg_guar == CKRM_SHARE_DONTCARE) {
+			cres->implicit_guar = impl_guar;
+			set_impl_guar_children(cres);
+		}
+	}
+	ckrm_unlock_hier(parres->core);
+
+}
+
 static void *
 mem_res_alloc(struct ckrm_core_class *core, struct ckrm_core_class
*parent)
 {
@@ -139,14 +176,106 @@ mem_res_alloc(struct ckrm_core_class *co
 			res->pg_limit = ckrm_tot_lru_pages;
 			res->hier = 0;
 			ckrm_mem_root_class = res;
-		} else
+		} else {
+			int guar;
 			res->hier = pres->hier + 1;
+			set_impl_guar_children(pres);
+			guar = (pres->pg_guar == CKRM_SHARE_DONTCARE) ?
+				pres->implicit_guar : pres->pg_unused;
+			res->implicit_guar = guar / pres->nr_dontcare;
+		}
 		ckrm_nr_mem_classes++;
 	} else
 		printk(KERN_ERR "MEM_RC: alloc: GFP_ATOMIC failed\n");
 	return res;
 }
 
+/*
+ * It is the caller's responsibility to make sure that the parent only
+ * has chilren that are to be accounted. i.e if a new child is added
+ * this function should be called after it has been added, and if a
+ * child is deleted this should be called after the child is removed.
+ */
+static void
+child_maxlimit_changed_local(struct ckrm_mem_res *parres)
+{
+	int maxlimit = 0;
+	struct ckrm_mem_res *childres;
+	struct ckrm_core_class *child = NULL;
+
+	/* run thru parent's children and get new max_limit of parent */
+	ckrm_lock_hier(parres->core);
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		childres = ckrm_get_res_class(child, mem_rcbs.resid,
+				struct ckrm_mem_res);
+		if (maxlimit < childres->shares.my_limit)
+			maxlimit = childres->shares.my_limit;
+	}
+	ckrm_unlock_hier(parres->core);
+	parres->shares.cur_max_limit = maxlimit;
+}
+
+/*
+ * Recalculate the guarantee and limit in # of pages... and propagate
the
+ * same to children.
+ * Caller is responsible for protecting res and for the integrity of
parres
+ */
+static void
+recalc_and_propagate(struct ckrm_mem_res * res, struct ckrm_mem_res *
parres)
+{
+	struct ckrm_core_class *child = NULL;
+	struct ckrm_mem_res *cres;
+	int resid = mem_rcbs.resid;
+	struct ckrm_shares *self = &res->shares;
+
+	if (parres) {
+		struct ckrm_shares *par = &parres->shares;
+
+		/* calculate pg_guar and pg_limit */
+		if (parres->pg_guar == CKRM_SHARE_DONTCARE ||
+				self->my_guarantee == CKRM_SHARE_DONTCARE) {
+			res->pg_guar = CKRM_SHARE_DONTCARE;
+		} else if (par->total_guarantee) {
+			u64 temp = (u64) self->my_guarantee * parres->pg_guar;
+			do_div(temp, par->total_guarantee);
+			res->pg_guar = (int) temp;
+			res->implicit_guar = CKRM_SHARE_DONTCARE;
+		} else {
+			res->pg_guar = 0;
+			res->implicit_guar = CKRM_SHARE_DONTCARE;
+		}
+
+		if (parres->pg_limit == CKRM_SHARE_DONTCARE ||
+				self->my_limit == CKRM_SHARE_DONTCARE) {
+			res->pg_limit = CKRM_SHARE_DONTCARE;
+		} else if (par->max_limit) {
+			u64 temp = (u64) self->my_limit * parres->pg_limit;
+			do_div(temp, par->max_limit);
+			res->pg_limit = (int) temp;
+		} else
+			res->pg_limit = 0;
+	}
+
+	/* Calculate unused units */
+	if (res->pg_guar == CKRM_SHARE_DONTCARE)
+		res->pg_unused = CKRM_SHARE_DONTCARE;
+	else if (self->total_guarantee) {
+		u64 temp = (u64) self->unused_guarantee * res->pg_guar;
+		do_div(temp, self->total_guarantee);
+		res->pg_unused = (int) temp;
+	} else
+		res->pg_unused = 0;
+
+	/* propagate to children */
+	ckrm_lock_hier(res->core);
+	while ((child = ckrm_get_next_child(res->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		recalc_and_propagate(cres, res);
+	}
+	ckrm_unlock_hier(res->core);
+	return;
+}
+
 static void
 mem_res_free(void *my_res)
 {
@@ -161,6 +290,14 @@ mem_res_free(void *my_res)
 	pres = ckrm_get_res_class(res->parent, mem_rcbs.resid,
 			struct ckrm_mem_res);
 
+	if (pres) {
+		child_guarantee_changed(&pres->shares,
+				res->shares.my_guarantee, 0);
+		child_maxlimit_changed_local(pres);
+		recalc_and_propagate(pres, NULL);
+		set_impl_guar_children(pres);
+	}
+
 	/*
 	 * Making it all zero as freeing of data structure could
 	 * happen later.
@@ -186,13 +323,24 @@ static int
 mem_set_share_values(void *my_res, struct ckrm_shares *shares)
 {
 	struct ckrm_mem_res *res = my_res;
+	struct ckrm_mem_res *parres;
+	int rc;
 
 	if (!res)
 		return -EINVAL;
 
-	printk(KERN_INFO "set_share called for %s resource of class %s\n",
-			MEM_RES_NAME, res->core->name);
-	return 0;
+	parres = ckrm_get_res_class(res->parent, mem_rcbs.resid,
+		struct ckrm_mem_res);
+
+	rc = set_shares(shares, &res->shares, parres ? &parres->shares :
NULL);
+
+	if ((rc == 0) && (parres != NULL)) {
+		child_maxlimit_changed_local(parres);
+		recalc_and_propagate(parres, NULL);
+		set_impl_guar_children(parres);
+	}
+
+	return rc;
 }
 
 static int
Index: linux-2.6.12/kernel/ckrm/ckrm_memctlr.c
===================================================================
--- linux-2.6.12.orig/kernel/ckrm/ckrm_memctlr.c
+++ linux-2.6.12/kernel/ckrm/ckrm_memctlr.c
@@ -66,7 +66,20 @@ decr_use_count(struct ckrm_mem_res *cls,
 int
 ckrm_class_limit_ok(struct ckrm_mem_res *cls)
 {
-	return 1; /* stub for now */
+	int ret, i, pg_total = 0;
+
+	if ((mem_rcbs.resid == -1) || !cls)
+		return 1;
+	for (i = 0; i < MAX_NR_ZONES; i++)
+		pg_total += cls->pg_total[i];
+	if (cls->pg_limit == CKRM_SHARE_DONTCARE) {
+		struct ckrm_mem_res *parcls = ckrm_get_res_class(cls->parent,
+					mem_rcbs.resid, struct ckrm_mem_res);
+		ret = (parcls ? ckrm_class_limit_ok(parcls) : 0);
+	} else
+		ret = (pg_total <= cls->pg_limit);
+
+	return ret;
 }
 
 static void migrate_list(struct list_head *list,

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------


--
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:"aart@kvack.org"> aart@kvack.org </a>

[PATCH 3/6] CKRM: Add limit support for mem controller

[Chandra Seetharaman]

Patch 3 of 6 patches to support memory controller under CKRM framework.
This patch provides the limit support for the controller.
----------------------------------------
Following changes have been made since the last release:
 - change impl_guar to implicit_guar
----------------------------------------

 include/linux/ckrm_mem.h   |    2 
 kernel/ckrm/ckrm_memcore.c |  156 +++++++++++++++++++++++++++++++++++++++++++--
 kernel/ckrm/ckrm_memctlr.c |   15 ++++
 3 files changed, 168 insertions(+), 5 deletions(-)

Content-Disposition: inline; filename=11-03-mem_core-limit
Index: linux-2612-rc3/include/linux/ckrm_mem.h
===================================================================
--- linux-2612-rc3.orig/include/linux/ckrm_mem.h
+++ linux-2612-rc3/include/linux/ckrm_mem.h
@@ -44,6 +44,8 @@ struct ckrm_mem_res {
 					 * parent if more than this is needed.
 					 */
 	int hier;			/* hiearchy level, root = 0 */
+	int implicit_guar;		/* for classes with don't care guar */
+	int nr_dontcare;		/* # of dont care children */
 };
 
 extern atomic_t ckrm_mem_real_count;
Index: linux-2612-rc3/kernel/ckrm/ckrm_memcore.c
===================================================================
--- linux-2612-rc3.orig/kernel/ckrm/ckrm_memcore.c
+++ linux-2612-rc3/kernel/ckrm/ckrm_memcore.c
@@ -95,9 +95,46 @@ mem_res_initcls_one(struct ckrm_mem_res 
 	INIT_LIST_HEAD(&res->mcls_list);
 
 	res->pg_unused = 0;
+	res->nr_dontcare = 1; /* for default class */
 	kref_init(&res->nr_users);
 }
 
+static void
+set_impl_guar_children(struct ckrm_mem_res *parres)
+{
+	struct ckrm_core_class *child = NULL;
+	struct ckrm_mem_res *cres;
+	int nr_dontcare = 1; /* for defaultclass */
+	int guar, impl_guar;
+	int resid = mem_rcbs.resid;
+
+	ckrm_lock_hier(parres->core);
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		/* treat NULL cres as don't care as that child is just being
+		 * created.
+		 * FIXME: need a better way to handle this case.
+		 */
+		if (!cres || cres->pg_guar == CKRM_SHARE_DONTCARE)
+			nr_dontcare++;
+	}
+
+	parres->nr_dontcare = nr_dontcare;
+	guar = (parres->pg_guar == CKRM_SHARE_DONTCARE) ?
+			parres->implicit_guar : parres->pg_unused;
+	impl_guar = guar / parres->nr_dontcare;
+
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		if (cres && cres->pg_guar == CKRM_SHARE_DONTCARE) {
+			cres->implicit_guar = impl_guar;
+			set_impl_guar_children(cres);
+		}
+	}
+	ckrm_unlock_hier(parres->core);
+
+}
+
 static void *
 mem_res_alloc(struct ckrm_core_class *core, struct ckrm_core_class *parent)
 {
@@ -139,14 +176,106 @@ mem_res_alloc(struct ckrm_core_class *co
 			res->pg_limit = ckrm_tot_lru_pages;
 			res->hier = 0;
 			ckrm_mem_root_class = res;
-		} else
+		} else {
+			int guar;
 			res->hier = pres->hier + 1;
+			set_impl_guar_children(pres);
+			guar = (pres->pg_guar == CKRM_SHARE_DONTCARE) ?
+				pres->implicit_guar : pres->pg_unused;
+			res->implicit_guar = guar / pres->nr_dontcare;
+		}
 		ckrm_nr_mem_classes++;
 	} else
 		printk(KERN_ERR "MEM_RC: alloc: GFP_ATOMIC failed\n");
 	return res;
 }
 
+/*
+ * It is the caller's responsibility to make sure that the parent only
+ * has chilren that are to be accounted. i.e if a new child is added
+ * this function should be called after it has been added, and if a
+ * child is deleted this should be called after the child is removed.
+ */
+static void
+child_maxlimit_changed_local(struct ckrm_mem_res *parres)
+{
+	int maxlimit = 0;
+	struct ckrm_mem_res *childres;
+	struct ckrm_core_class *child = NULL;
+
+	/* run thru parent's children and get new max_limit of parent */
+	ckrm_lock_hier(parres->core);
+	while ((child = ckrm_get_next_child(parres->core, child)) != NULL) {
+		childres = ckrm_get_res_class(child, mem_rcbs.resid,
+				struct ckrm_mem_res);
+		if (maxlimit < childres->shares.my_limit)
+			maxlimit = childres->shares.my_limit;
+	}
+	ckrm_unlock_hier(parres->core);
+	parres->shares.cur_max_limit = maxlimit;
+}
+
+/*
+ * Recalculate the guarantee and limit in # of pages... and propagate the
+ * same to children.
+ * Caller is responsible for protecting res and for the integrity of parres
+ */
+static void
+recalc_and_propagate(struct ckrm_mem_res * res, struct ckrm_mem_res * parres)
+{
+	struct ckrm_core_class *child = NULL;
+	struct ckrm_mem_res *cres;
+	int resid = mem_rcbs.resid;
+	struct ckrm_shares *self = &res->shares;
+
+	if (parres) {
+		struct ckrm_shares *par = &parres->shares;
+
+		/* calculate pg_guar and pg_limit */
+		if (parres->pg_guar == CKRM_SHARE_DONTCARE ||
+				self->my_guarantee == CKRM_SHARE_DONTCARE) {
+			res->pg_guar = CKRM_SHARE_DONTCARE;
+		} else if (par->total_guarantee) {
+			u64 temp = (u64) self->my_guarantee * parres->pg_guar;
+			do_div(temp, par->total_guarantee);
+			res->pg_guar = (int) temp;
+			res->implicit_guar = CKRM_SHARE_DONTCARE;
+		} else {
+			res->pg_guar = 0;
+			res->implicit_guar = CKRM_SHARE_DONTCARE;
+		}
+
+		if (parres->pg_limit == CKRM_SHARE_DONTCARE ||
+				self->my_limit == CKRM_SHARE_DONTCARE) {
+			res->pg_limit = CKRM_SHARE_DONTCARE;
+		} else if (par->max_limit) {
+			u64 temp = (u64) self->my_limit * parres->pg_limit;
+			do_div(temp, par->max_limit);
+			res->pg_limit = (int) temp;
+		} else
+			res->pg_limit = 0;
+	}
+
+	/* Calculate unused units */
+	if (res->pg_guar == CKRM_SHARE_DONTCARE)
+		res->pg_unused = CKRM_SHARE_DONTCARE;
+	else if (self->total_guarantee) {
+		u64 temp = (u64) self->unused_guarantee * res->pg_guar;
+		do_div(temp, self->total_guarantee);
+		res->pg_unused = (int) temp;
+	} else
+		res->pg_unused = 0;
+
+	/* propagate to children */
+	ckrm_lock_hier(res->core);
+	while ((child = ckrm_get_next_child(res->core, child)) != NULL) {
+		cres = ckrm_get_res_class(child, resid, struct ckrm_mem_res);
+		recalc_and_propagate(cres, res);
+	}
+	ckrm_unlock_hier(res->core);
+	return;
+}
+
 static void
 mem_res_free(void *my_res)
 {
@@ -161,6 +290,14 @@ mem_res_free(void *my_res)
 	pres = ckrm_get_res_class(res->parent, mem_rcbs.resid,
 			struct ckrm_mem_res);
 
+	if (pres) {
+		child_guarantee_changed(&pres->shares,
+				res->shares.my_guarantee, 0);
+		child_maxlimit_changed_local(pres);
+		recalc_and_propagate(pres, NULL);
+		set_impl_guar_children(pres);
+	}
+
 	/*
 	 * Making it all zero as freeing of data structure could 
 	 * happen later.
@@ -186,13 +323,24 @@ static int
 mem_set_share_values(void *my_res, struct ckrm_shares *shares)
 {
 	struct ckrm_mem_res *res = my_res;
+	struct ckrm_mem_res *parres;
+	int rc;
 
 	if (!res)
 		return -EINVAL;
 
-	printk(KERN_INFO "set_share called for %s resource of class %s\n",
-			MEM_RES_NAME, res->core->name);
-	return 0;
+	parres = ckrm_get_res_class(res->parent, mem_rcbs.resid,
+		struct ckrm_mem_res);
+
+	rc = set_shares(shares, &res->shares, parres ? &parres->shares : NULL);
+
+	if ((rc == 0) && (parres != NULL)) {
+		child_maxlimit_changed_local(parres);
+		recalc_and_propagate(parres, NULL);
+		set_impl_guar_children(parres);
+	}
+
+	return rc;
 }
 
 static int
Index: linux-2612-rc3/kernel/ckrm/ckrm_memctlr.c
===================================================================
--- linux-2612-rc3.orig/kernel/ckrm/ckrm_memctlr.c
+++ linux-2612-rc3/kernel/ckrm/ckrm_memctlr.c
@@ -66,7 +66,20 @@ decr_use_count(struct ckrm_mem_res *cls,
 int
 ckrm_class_limit_ok(struct ckrm_mem_res *cls)
 {
-	return 1; /* stub for now */
+	int ret, i, pg_total = 0;
+
+	if ((mem_rcbs.resid == -1) || !cls)
+		return 1;
+	for (i = 0; i < MAX_NR_ZONES; i++)
+		pg_total += cls->pg_total[i];
+	if (cls->pg_limit == CKRM_SHARE_DONTCARE) {
+		struct ckrm_mem_res *parcls = ckrm_get_res_class(cls->parent,
+					mem_rcbs.resid, struct ckrm_mem_res);
+		ret = (parcls ? ckrm_class_limit_ok(parcls) : 0);
+	} else
+		ret = (pg_total <= cls->pg_limit);
+
+	return ret;
 }
 
 static void migrate_list(struct list_head *list,
--
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:"aart@kvack.org"> aart@kvack.org </a>