linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] use vmalloc for mem_cgroup allocation.
@ 2008-04-15  1:54 KAMEZAWA Hiroyuki
  2008-04-15  1:57 ` Li Zefan
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-04-15  1:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: balbir@linux.vnet.ibm.com, xemul@openvz.org, lizf, menage,
	linux-mm@kvack.org, LKML

On ia64, kmalloc() in mem_cgroup_create requires order-4 pages. But this is not
necessary to be phisically contiguous. And we'll see page allocation failure.
(Note: x86-32, which has small vmalloc area, has small mem_cgroup struct.)
For here, vmalloc is better.


Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Index: mm-2.6.25-rc8-mm2/mm/memcontrol.c
===================================================================
--- mm-2.6.25-rc8-mm2.orig/mm/memcontrol.c
+++ mm-2.6.25-rc8-mm2/mm/memcontrol.c
@@ -31,6 +31,7 @@
 #include <linux/spinlock.h>
 #include <linux/fs.h>
 #include <linux/seq_file.h>
+#include <linux/vmalloc.h>
 
 #include <asm/uaccess.h>
 
@@ -993,7 +994,7 @@ mem_cgroup_create(struct cgroup_subsys *
 		mem = &init_mem_cgroup;
 		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
 	} else
-		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
+		mem = vmalloc(sizeof(struct mem_cgroup));
 
 	if (mem == NULL)
 		return ERR_PTR(-ENOMEM);
@@ -1011,7 +1012,7 @@ free_out:
 	for_each_node_state(node, N_POSSIBLE)
 		free_mem_cgroup_per_zone_info(mem, node);
 	if (cont->parent != NULL)
-		kfree(mem);
+		vfree(mem);
 	return ERR_PTR(-ENOMEM);
 }
 
@@ -1031,7 +1032,7 @@ static void mem_cgroup_destroy(struct cg
 	for_each_node_state(node, N_POSSIBLE)
 		free_mem_cgroup_per_zone_info(mem, node);
 
-	kfree(mem_cgroup_from_cont(cont));
+	vfree(mem_cgroup_from_cont(cont));
 }
 
 static int mem_cgroup_populate(struct cgroup_subsys *ss,

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation.
  2008-04-15  1:54 [PATCH] use vmalloc for mem_cgroup allocation KAMEZAWA Hiroyuki
@ 2008-04-15  1:57 ` Li Zefan
  2008-04-15  2:07   ` KAMEZAWA Hiroyuki
  2008-04-15  2:10 ` [PATCH] use vmalloc for mem_cgroup allocation. v2 KAMEZAWA Hiroyuki
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Li Zefan @ 2008-04-15  1:57 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org,
	menage, linux-mm@kvack.org, LKML

KAMEZAWA Hiroyuki wrote:
> On ia64, kmalloc() in mem_cgroup_create requires order-4 pages. But this is not
> necessary to be phisically contiguous. And we'll see page allocation failure.
> (Note: x86-32, which has small vmalloc area, has small mem_cgroup struct.)
> For here, vmalloc is better.
> 
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> Index: mm-2.6.25-rc8-mm2/mm/memcontrol.c
> ===================================================================
> --- mm-2.6.25-rc8-mm2.orig/mm/memcontrol.c
> +++ mm-2.6.25-rc8-mm2/mm/memcontrol.c
> @@ -31,6 +31,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/fs.h>
>  #include <linux/seq_file.h>
> +#include <linux/vmalloc.h>
>  
>  #include <asm/uaccess.h>
>  
> @@ -993,7 +994,7 @@ mem_cgroup_create(struct cgroup_subsys *
>  		mem = &init_mem_cgroup;
>  		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
>  	} else
> -		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
> +		mem = vmalloc(sizeof(struct mem_cgroup));
>  

memset(mem, 0, sizeof(*mem));

should we initialize it with 0?

>  	if (mem == NULL)
>  		return ERR_PTR(-ENOMEM);
> @@ -1011,7 +1012,7 @@ free_out:
>  	for_each_node_state(node, N_POSSIBLE)
>  		free_mem_cgroup_per_zone_info(mem, node);
>  	if (cont->parent != NULL)
> -		kfree(mem);
> +		vfree(mem);
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> @@ -1031,7 +1032,7 @@ static void mem_cgroup_destroy(struct cg
>  	for_each_node_state(node, N_POSSIBLE)
>  		free_mem_cgroup_per_zone_info(mem, node);
>  
> -	kfree(mem_cgroup_from_cont(cont));
> +	vfree(mem_cgroup_from_cont(cont));
>  }
>  
>  static int mem_cgroup_populate(struct cgroup_subsys *ss,
> 
>

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation.
  2008-04-15  1:57 ` Li Zefan
@ 2008-04-15  2:07   ` KAMEZAWA Hiroyuki
  0 siblings, 0 replies; 14+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-04-15  2:07 UTC (permalink / raw)
  To: Li Zefan
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org,
	menage, linux-mm@kvack.org, LKML

On Tue, 15 Apr 2008 09:57:17 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:

> >  #include <asm/uaccess.h>
> >  
> > @@ -993,7 +994,7 @@ mem_cgroup_create(struct cgroup_subsys *
> >  		mem = &init_mem_cgroup;
> >  		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> >  	} else
> > -		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
> > +		mem = vmalloc(sizeof(struct mem_cgroup));
> >  
> 
> memset(mem, 0, sizeof(*mem));
> 
ok, will rewrite.

> should we initialize it with 0?
> 
yes. (at least, per-cpu stat. mem->css, per-node pointer, ... should be zero.)

Thanks,
-Kame


> >  	if (mem == NULL)
> >  		return ERR_PTR(-ENOMEM);
> > @@ -1011,7 +1012,7 @@ free_out:
> >  	for_each_node_state(node, N_POSSIBLE)
> >  		free_mem_cgroup_per_zone_info(mem, node);
> >  	if (cont->parent != NULL)
> > -		kfree(mem);
> > +		vfree(mem);
> >  	return ERR_PTR(-ENOMEM);
> >  }
> >  
> > @@ -1031,7 +1032,7 @@ static void mem_cgroup_destroy(struct cg
> >  	for_each_node_state(node, N_POSSIBLE)
> >  		free_mem_cgroup_per_zone_info(mem, node);
> >  
> > -	kfree(mem_cgroup_from_cont(cont));
> > +	vfree(mem_cgroup_from_cont(cont));
> >  }
> >  
> >  static int mem_cgroup_populate(struct cgroup_subsys *ss,
> > 
> >
> 

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  2:10 ` [PATCH] use vmalloc for mem_cgroup allocation. v2 KAMEZAWA Hiroyuki
@ 2008-04-15  2:08   ` Li Zefan
  2008-04-15  2:20     ` Andrew Morton
  2008-04-15  2:17   ` Andrew Morton
  1 sibling, 1 reply; 14+ messages in thread
From: Li Zefan @ 2008-04-15  2:08 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org,
	menage, linux-mm@kvack.org, LKML

KAMEZAWA Hiroyuki wrote:
> On ia64, this kmalloc() requires order-4 pages. But this is not
> necessary to be phisically contiguous. (and x86-32, which has
> small vmalloc area, has small mem_cgroup struct.)
> 
> For here, vmalloc is better.
> 
> Changelog: v1->v2
>  - added memset().
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> Index: mm-2.6.25-rc8-mm2/mm/memcontrol.c
> ===================================================================
> --- mm-2.6.25-rc8-mm2.orig/mm/memcontrol.c
> +++ mm-2.6.25-rc8-mm2/mm/memcontrol.c
> @@ -31,6 +31,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/fs.h>
>  #include <linux/seq_file.h>
> +#include <linux/vmalloc.h>
>  
>  #include <asm/uaccess.h>
>  
> @@ -992,8 +993,10 @@ mem_cgroup_create(struct cgroup_subsys *
>  	if (unlikely((cont->parent) == NULL)) {
>  		mem = &init_mem_cgroup;
>  		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> -	} else
> -		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
> +	} else {
> +		mem = vmalloc(sizeof(struct mem_cgroup));
> +		memset(mem, 0, sizeof(*mem));

what if mem == NULL. ;)

> +	}
>  
>  	if (mem == NULL)
>  		return ERR_PTR(-ENOMEM);

So we can move this NULL check to the above else branch, in the if brach,
mem won't be NULL.

> @@ -1011,7 +1014,7 @@ free_out:
>  	for_each_node_state(node, N_POSSIBLE)
>  		free_mem_cgroup_per_zone_info(mem, node);
>  	if (cont->parent != NULL)
> -		kfree(mem);
> +		vfree(mem);
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> @@ -1031,7 +1034,7 @@ static void mem_cgroup_destroy(struct cg
>  	for_each_node_state(node, N_POSSIBLE)
>  		free_mem_cgroup_per_zone_info(mem, node);
>  
> -	kfree(mem_cgroup_from_cont(cont));
> +	vfree(mem_cgroup_from_cont(cont));
>  }
>  
>  static int mem_cgroup_populate(struct cgroup_subsys *ss,
> 
> 
> 

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  1:54 [PATCH] use vmalloc for mem_cgroup allocation KAMEZAWA Hiroyuki
  2008-04-15  1:57 ` Li Zefan
@ 2008-04-15  2:10 ` KAMEZAWA Hiroyuki
  2008-04-15  2:08   ` Li Zefan
  2008-04-15  2:17   ` Andrew Morton
  2008-04-15  5:12 ` [PATCH] use vmalloc for mem_cgroup allocation. v3 KAMEZAWA Hiroyuki
  2008-04-16 19:19 ` [PATCH] use vmalloc for mem_cgroup allocation Christoph Lameter
  3 siblings, 2 replies; 14+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-04-15  2:10 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org, lizf,
	menage, linux-mm@kvack.org, LKML

On ia64, this kmalloc() requires order-4 pages. But this is not
necessary to be phisically contiguous. (and x86-32, which has
small vmalloc area, has small mem_cgroup struct.)

For here, vmalloc is better.

Changelog: v1->v2
 - added memset().

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Index: mm-2.6.25-rc8-mm2/mm/memcontrol.c
===================================================================
--- mm-2.6.25-rc8-mm2.orig/mm/memcontrol.c
+++ mm-2.6.25-rc8-mm2/mm/memcontrol.c
@@ -31,6 +31,7 @@
 #include <linux/spinlock.h>
 #include <linux/fs.h>
 #include <linux/seq_file.h>
+#include <linux/vmalloc.h>
 
 #include <asm/uaccess.h>
 
@@ -992,8 +993,10 @@ mem_cgroup_create(struct cgroup_subsys *
 	if (unlikely((cont->parent) == NULL)) {
 		mem = &init_mem_cgroup;
 		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
-	} else
-		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
+	} else {
+		mem = vmalloc(sizeof(struct mem_cgroup));
+		memset(mem, 0, sizeof(*mem));
+	}
 
 	if (mem == NULL)
 		return ERR_PTR(-ENOMEM);
@@ -1011,7 +1014,7 @@ free_out:
 	for_each_node_state(node, N_POSSIBLE)
 		free_mem_cgroup_per_zone_info(mem, node);
 	if (cont->parent != NULL)
-		kfree(mem);
+		vfree(mem);
 	return ERR_PTR(-ENOMEM);
 }
 
@@ -1031,7 +1034,7 @@ static void mem_cgroup_destroy(struct cg
 	for_each_node_state(node, N_POSSIBLE)
 		free_mem_cgroup_per_zone_info(mem, node);
 
-	kfree(mem_cgroup_from_cont(cont));
+	vfree(mem_cgroup_from_cont(cont));
 }
 
 static int mem_cgroup_populate(struct cgroup_subsys *ss,

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  2:10 ` [PATCH] use vmalloc for mem_cgroup allocation. v2 KAMEZAWA Hiroyuki
  2008-04-15  2:08   ` Li Zefan
@ 2008-04-15  2:17   ` Andrew Morton
  2008-04-15  3:16     ` KAMEZAWA Hiroyuki
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2008-04-15  2:17 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: balbir@linux.vnet.ibm.com, xemul@openvz.org, lizf, menage,
	linux-mm@kvack.org, LKML

On Tue, 15 Apr 2008 11:10:38 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On ia64, this kmalloc() requires order-4 pages. But this is not
> necessary to be phisically contiguous. (and x86-32, which has
> small vmalloc area, has small mem_cgroup struct.)
> 
> For here, vmalloc is better.
> 
> Changelog: v1->v2
>  - added memset().
> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> Index: mm-2.6.25-rc8-mm2/mm/memcontrol.c
> ===================================================================
> --- mm-2.6.25-rc8-mm2.orig/mm/memcontrol.c
> +++ mm-2.6.25-rc8-mm2/mm/memcontrol.c
> @@ -31,6 +31,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/fs.h>
>  #include <linux/seq_file.h>
> +#include <linux/vmalloc.h>
>  
>  #include <asm/uaccess.h>
>  
> @@ -992,8 +993,10 @@ mem_cgroup_create(struct cgroup_subsys *
>  	if (unlikely((cont->parent) == NULL)) {
>  		mem = &init_mem_cgroup;
>  		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> -	} else
> -		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
> +	} else {
> +		mem = vmalloc(sizeof(struct mem_cgroup));
> +		memset(mem, 0, sizeof(*mem));
> +	}
>  
>  	if (mem == NULL)
>  		return ERR_PTR(-ENOMEM);
> @@ -1011,7 +1014,7 @@ free_out:
>  	for_each_node_state(node, N_POSSIBLE)
>  		free_mem_cgroup_per_zone_info(mem, node);
>  	if (cont->parent != NULL)
> -		kfree(mem);
> +		vfree(mem);
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> @@ -1031,7 +1034,7 @@ static void mem_cgroup_destroy(struct cg
>  	for_each_node_state(node, N_POSSIBLE)
>  		free_mem_cgroup_per_zone_info(mem, node);
>  
> -	kfree(mem_cgroup_from_cont(cont));
> +	vfree(mem_cgroup_from_cont(cont));
>  }
>  
>  static int mem_cgroup_populate(struct cgroup_subsys *ss,

Well...  vmalloced memory is of course a little slower to use - additional
TLB pressure.

Do you think the memcgroup is accessed frequently enough to use vmalloc()
only on those architectures which actually need it?

Because it'd be pretty simple to implement:

	if (sizeof(struct mem_group) > PAGE_SIZE)
		vmalloc()
	else
		kmalloc()

	...

	if (sizeof(struct mem_group) > PAGE_SIZE)
		vfree()
	else
		kfree()

the compiler will optimise away the `if'.

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  2:08   ` Li Zefan
@ 2008-04-15  2:20     ` Andrew Morton
  2008-04-15  2:28       ` Harvey Harrison
  2008-04-15  3:12       ` KAMEZAWA Hiroyuki
  0 siblings, 2 replies; 14+ messages in thread
From: Andrew Morton @ 2008-04-15  2:20 UTC (permalink / raw)
  To: Li Zefan
  Cc: KAMEZAWA Hiroyuki, balbir@linux.vnet.ibm.com, xemul@openvz.org,
	menage, linux-mm@kvack.org, LKML

On Tue, 15 Apr 2008 10:08:25 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:

> > @@ -992,8 +993,10 @@ mem_cgroup_create(struct cgroup_subsys *
> >  	if (unlikely((cont->parent) == NULL)) {
> >  		mem = &init_mem_cgroup;
> >  		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> > -	} else
> > -		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
> > +	} else {
> > +		mem = vmalloc(sizeof(struct mem_cgroup));
> > +		memset(mem, 0, sizeof(*mem));
> 
> what if mem == NULL. ;)
> 
> > +	}
> >  
> >  	if (mem == NULL)
> >  		return ERR_PTR(-ENOMEM);
> 
> So we can move this NULL check to the above else branch, in the if brach,
> mem won't be NULL.

err, yes.

So I have:

	if (unlikely((cont->parent) == NULL)) {
		mem = &init_mem_cgroup;
		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
	} else {
		mem = vmalloc(sizeof(struct mem_cgroup));
		if (mem == NULL)
			return ERR_PTR(-ENOMEM);
		memset(mem, 0, sizeof(*mem));
	}


--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  2:20     ` Andrew Morton
@ 2008-04-15  2:28       ` Harvey Harrison
  2008-04-15  3:12       ` KAMEZAWA Hiroyuki
  1 sibling, 0 replies; 14+ messages in thread
From: Harvey Harrison @ 2008-04-15  2:28 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Li Zefan, KAMEZAWA Hiroyuki, balbir@linux.vnet.ibm.com,
	xemul@openvz.org, menage, linux-mm@kvack.org, LKML

On Mon, 2008-04-14 at 19:20 -0700, Andrew Morton wrote:
> On Tue, 15 Apr 2008 10:08:25 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:
> 
> > > @@ -992,8 +993,10 @@ mem_cgroup_create(struct cgroup_subsys *
> > >  	if (unlikely((cont->parent) == NULL)) {
> > >  		mem = &init_mem_cgroup;
> > >  		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> > > -	} else
> > > -		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
> > > +	} else {
> > > +		mem = vmalloc(sizeof(struct mem_cgroup));
> > > +		memset(mem, 0, sizeof(*mem));
> > 
> > what if mem == NULL. ;)
> > 
> > > +	}
> > >  
> > >  	if (mem == NULL)
> > >  		return ERR_PTR(-ENOMEM);
> > 
> > So we can move this NULL check to the above else branch, in the if brach,
> > mem won't be NULL.
> 
> err, yes.
> 
> So I have:
> 
> 	if (unlikely((cont->parent) == NULL)) {

	if (unlikely(cont->parent == NULL)) {

> 		mem = &init_mem_cgroup;
> 		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> 	} else {
> 		mem = vmalloc(sizeof(struct mem_cgroup));
> 		if (mem == NULL)

		if (!mem)

> 			return ERR_PTR(-ENOMEM);
> 		memset(mem, 0, sizeof(*mem));
> 	}
> 

Cheers,

Harvey


--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  2:20     ` Andrew Morton
  2008-04-15  2:28       ` Harvey Harrison
@ 2008-04-15  3:12       ` KAMEZAWA Hiroyuki
  1 sibling, 0 replies; 14+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-04-15  3:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Li Zefan, balbir@linux.vnet.ibm.com, xemul@openvz.org, menage,
	linux-mm@kvack.org, LKML

On Mon, 14 Apr 2008 19:20:42 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 15 Apr 2008 10:08:25 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:
> 
> > > @@ -992,8 +993,10 @@ mem_cgroup_create(struct cgroup_subsys *
> > >  	if (unlikely((cont->parent) == NULL)) {
> > >  		mem = &init_mem_cgroup;
> > >  		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> > > -	} else
> > > -		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
> > > +	} else {
> > > +		mem = vmalloc(sizeof(struct mem_cgroup));
> > > +		memset(mem, 0, sizeof(*mem));
> > 
> > what if mem == NULL. ;)
> > 
> > > +	}
> > >  
> > >  	if (mem == NULL)
> > >  		return ERR_PTR(-ENOMEM);
> > 
> > So we can move this NULL check to the above else branch, in the if brach,
> > mem won't be NULL.
> 
> err, yes.
> 
> So I have:
> 
> 	if (unlikely((cont->parent) == NULL)) {
> 		mem = &init_mem_cgroup;
> 		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
> 	} else {
> 		mem = vmalloc(sizeof(struct mem_cgroup));
> 		if (mem == NULL)
> 			return ERR_PTR(-ENOMEM);
> 		memset(mem, 0, sizeof(*mem));
> 	}
> 
Sorry for poor patch....
And thank you for catching.

Thanks,
-Kame


--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  3:16     ` KAMEZAWA Hiroyuki
@ 2008-04-15  3:14       ` Li Zefan
  2008-04-15  3:22         ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 14+ messages in thread
From: Li Zefan @ 2008-04-15  3:14 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org,
	menage, linux-mm@kvack.org, LKML

KAMEZAWA Hiroyuki wrote:
> On Mon, 14 Apr 2008 19:17:30 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
>> Well...  vmalloced memory is of course a little slower to use - additional
>> TLB pressure.
>>
>> Do you think the memcgroup is accessed frequently enough to use vmalloc()
>> only on those architectures which actually need it?
>>
>> Because it'd be pretty simple to implement:
>>
>> 	if (sizeof(struct mem_group) > PAGE_SIZE)
>> 		vmalloc()
>> 	else
>> 		kmalloc()
>>
>> 	...
>>
>> 	if (sizeof(struct mem_group) > PAGE_SIZE)
>> 		vfree()
>> 	else
>> 		kfree()
>>
>> the compiler will optimise away the `if'.
>>
> 
> Hmm, ok. I'll rewrite one to do that.
> 

It will be better to use wrappers for these: mem_cgroup_alloc() and mem_cgroup_free()

> Thanks,
> -Kame
> 
> 
> 

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  2:17   ` Andrew Morton
@ 2008-04-15  3:16     ` KAMEZAWA Hiroyuki
  2008-04-15  3:14       ` Li Zefan
  0 siblings, 1 reply; 14+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-04-15  3:16 UTC (permalink / raw)
  To: Andrew Morton
  Cc: balbir@linux.vnet.ibm.com, xemul@openvz.org, lizf, menage,
	linux-mm@kvack.org, LKML

On Mon, 14 Apr 2008 19:17:30 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> Well...  vmalloced memory is of course a little slower to use - additional
> TLB pressure.
> 
> Do you think the memcgroup is accessed frequently enough to use vmalloc()
> only on those architectures which actually need it?
> 
> Because it'd be pretty simple to implement:
> 
> 	if (sizeof(struct mem_group) > PAGE_SIZE)
> 		vmalloc()
> 	else
> 		kmalloc()
> 
> 	...
> 
> 	if (sizeof(struct mem_group) > PAGE_SIZE)
> 		vfree()
> 	else
> 		kfree()
> 
> the compiler will optimise away the `if'.
> 

Hmm, ok. I'll rewrite one to do that.

Thanks,
-Kame

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation. v2
  2008-04-15  3:14       ` Li Zefan
@ 2008-04-15  3:22         ` KAMEZAWA Hiroyuki
  0 siblings, 0 replies; 14+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-04-15  3:22 UTC (permalink / raw)
  To: Li Zefan
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org,
	menage, linux-mm@kvack.org, LKML

On Tue, 15 Apr 2008 11:14:24 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:

> KAMEZAWA Hiroyuki wrote:
> > On Mon, 14 Apr 2008 19:17:30 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> >> Well...  vmalloced memory is of course a little slower to use - additional
> >> TLB pressure.
> >>
> >> Do you think the memcgroup is accessed frequently enough to use vmalloc()
> >> only on those architectures which actually need it?
> >>
> >> Because it'd be pretty simple to implement:
> >>
> >> 	if (sizeof(struct mem_group) > PAGE_SIZE)
> >> 		vmalloc()
> >> 	else
> >> 		kmalloc()
> >>
> >> 	...
> >>
> >> 	if (sizeof(struct mem_group) > PAGE_SIZE)
> >> 		vfree()
> >> 	else
> >> 		kfree()
> >>
> >> the compiler will optimise away the `if'.
> >>
> > 
> > Hmm, ok. I'll rewrite one to do that.
> > 
> 
> It will be better to use wrappers for these: mem_cgroup_alloc() and mem_cgroup_free()
> 
yes. will do

-Kame
> > Thanks,
> > -Kame
> > 
> > 
> > 
> 

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] use vmalloc for mem_cgroup allocation. v3
  2008-04-15  1:54 [PATCH] use vmalloc for mem_cgroup allocation KAMEZAWA Hiroyuki
  2008-04-15  1:57 ` Li Zefan
  2008-04-15  2:10 ` [PATCH] use vmalloc for mem_cgroup allocation. v2 KAMEZAWA Hiroyuki
@ 2008-04-15  5:12 ` KAMEZAWA Hiroyuki
  2008-04-16 19:19 ` [PATCH] use vmalloc for mem_cgroup allocation Christoph Lameter
  3 siblings, 0 replies; 14+ messages in thread
From: KAMEZAWA Hiroyuki @ 2008-04-15  5:12 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org, lizf,
	menage, linux-mm@kvack.org, LKML

Tested on ia64/NUMA and x86/smp.
==
On ia64, this kmalloc() requires order-4 pages. But this is not
necessary to be phisically contiguous. 
For big mem_cgroup, vmalloc is better. For small ones, kmalloc is used.


Changelog: v2->v3
 - fixed the place of memset.
 - added mem_cgroup_alloc()/free()
 - use kmalloc if mem_cgroup is enough small.
Changelog: v1->v2
 - added memset().

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Index: mm-2.6.25-rc8-mm2/mm/memcontrol.c
===================================================================
--- mm-2.6.25-rc8-mm2.orig/mm/memcontrol.c
+++ mm-2.6.25-rc8-mm2/mm/memcontrol.c
@@ -31,6 +31,7 @@
 #include <linux/spinlock.h>
 #include <linux/fs.h>
 #include <linux/seq_file.h>
+#include <linux/vmalloc.h>
 
 #include <asm/uaccess.h>
 
@@ -983,6 +984,31 @@ static void free_mem_cgroup_per_zone_inf
 	kfree(mem->info.nodeinfo[node]);
 }
 
+static struct mem_cgroup *mem_cgroup_alloc(void)
+{
+	struct mem_cgroup *mem;
+
+	if (sizeof(*mem) < PAGE_SIZE)
+		mem = kmalloc(sizeof(*mem), GFP_KERNEL);
+	else
+		mem = vmalloc(sizeof(*mem));
+
+	if (!mem)
+		return NULL;
+
+	memset(mem, 0, sizeof(*mem));
+	return mem;
+}
+
+static void mem_cgroup_free(struct mem_cgroup *mem)
+{
+	if (sizeof(*mem) < PAGE_SIZE)
+		kfree(mem);
+	else
+		vfree(mem);
+}
+
+
 static struct cgroup_subsys_state *
 mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 {
@@ -992,11 +1018,11 @@ mem_cgroup_create(struct cgroup_subsys *
 	if (unlikely((cont->parent) == NULL)) {
 		mem = &init_mem_cgroup;
 		page_cgroup_cache = KMEM_CACHE(page_cgroup, SLAB_PANIC);
-	} else
-		mem = kzalloc(sizeof(struct mem_cgroup), GFP_KERNEL);
-
-	if (mem == NULL)
-		return ERR_PTR(-ENOMEM);
+	} else {
+		mem = mem_cgroup_alloc();
+		if (!mem)
+			return ERR_PTR(-ENOMEM);
+	}
 
 	res_counter_init(&mem->res);
 
@@ -1011,7 +1037,7 @@ free_out:
 	for_each_node_state(node, N_POSSIBLE)
 		free_mem_cgroup_per_zone_info(mem, node);
 	if (cont->parent != NULL)
-		kfree(mem);
+		mem_cgroup_free(mem);
 	return ERR_PTR(-ENOMEM);
 }
 
@@ -1031,7 +1057,7 @@ static void mem_cgroup_destroy(struct cg
 	for_each_node_state(node, N_POSSIBLE)
 		free_mem_cgroup_per_zone_info(mem, node);
 
-	kfree(mem_cgroup_from_cont(cont));
+	mem_cgroup_free(mem_cgroup_from_cont(cont));
 }
 
 static int mem_cgroup_populate(struct cgroup_subsys *ss,

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] use vmalloc for mem_cgroup allocation.
  2008-04-15  1:54 [PATCH] use vmalloc for mem_cgroup allocation KAMEZAWA Hiroyuki
                   ` (2 preceding siblings ...)
  2008-04-15  5:12 ` [PATCH] use vmalloc for mem_cgroup allocation. v3 KAMEZAWA Hiroyuki
@ 2008-04-16 19:19 ` Christoph Lameter
  3 siblings, 0 replies; 14+ messages in thread
From: Christoph Lameter @ 2008-04-16 19:19 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, balbir@linux.vnet.ibm.com, xemul@openvz.org, lizf,
	menage, linux-mm@kvack.org, LKML

On Tue, 15 Apr 2008, KAMEZAWA Hiroyuki wrote:

> On ia64, kmalloc() in mem_cgroup_create requires order-4 pages. But this is not
> necessary to be phisically contiguous. And we'll see page allocation failure.
> (Note: x86-32, which has small vmalloc area, has small mem_cgroup struct.)
> For here, vmalloc is better.

I need to get my virtualizable compound stuff in order. That would address 
these issues.

--
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>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2008-04-16 19:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-15  1:54 [PATCH] use vmalloc for mem_cgroup allocation KAMEZAWA Hiroyuki
2008-04-15  1:57 ` Li Zefan
2008-04-15  2:07   ` KAMEZAWA Hiroyuki
2008-04-15  2:10 ` [PATCH] use vmalloc for mem_cgroup allocation. v2 KAMEZAWA Hiroyuki
2008-04-15  2:08   ` Li Zefan
2008-04-15  2:20     ` Andrew Morton
2008-04-15  2:28       ` Harvey Harrison
2008-04-15  3:12       ` KAMEZAWA Hiroyuki
2008-04-15  2:17   ` Andrew Morton
2008-04-15  3:16     ` KAMEZAWA Hiroyuki
2008-04-15  3:14       ` Li Zefan
2008-04-15  3:22         ` KAMEZAWA Hiroyuki
2008-04-15  5:12 ` [PATCH] use vmalloc for mem_cgroup allocation. v3 KAMEZAWA Hiroyuki
2008-04-16 19:19 ` [PATCH] use vmalloc for mem_cgroup allocation Christoph Lameter

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).