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