[PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Jesper Juhl]

Hi (please CC me on replies),


Apologies to those who receive this multiple times. I screwed up the To: 
field in my original mail :-(


In mem_cgroup_alloc() we currently do either kmalloc() or vmalloc() then 
followed by memset() to zero the memory. This can be more efficiently 
achieved by using kzalloc() and vzalloc().


Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 memcontrol.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 9a99cfa..90da698 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4199,14 +4199,13 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
 
 	/* Can be very big if MAX_NUMNODES is very big */
 	if (size < PAGE_SIZE)
-		mem = kmalloc(size, GFP_KERNEL);
+		mem = kzalloc(size, GFP_KERNEL);
 	else
-		mem = vmalloc(size);
+		mem = vzalloc(size);
 
 	if (!mem)
 		return NULL;
 
-	memset(mem, 0, size);
 	mem->stat = alloc_percpu(struct mem_cgroup_stat_cpu);
 	if (!mem->stat) {
 		if (size < PAGE_SIZE)


-- 
Jesper Juhl <jj@chaosbits.net>             http://www.chaosbits.net/
Plain text mails only, please      http://www.expita.com/nomime.html
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html

Re: [PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Hiroyuki Kamezawa]

2010/11/2 Jesper Juhl <jj@chaosbits.net>:
> Hi (please CC me on replies),
>
>
> Apologies to those who receive this multiple times. I screwed up the To:
> field in my original mail :-(
>
>
> In mem_cgroup_alloc() we currently do either kmalloc() or vmalloc() then
> followed by memset() to zero the memory. This can be more efficiently
> achieved by using kzalloc() and vzalloc().
>
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Thanks,
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Re: [PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Johannes Weiner]

On Mon, Nov 01, 2010 at 08:40:56PM +0100, Jesper Juhl wrote:
> Hi (please CC me on replies),
> 
> 
> Apologies to those who receive this multiple times. I screwed up the To: 
> field in my original mail :-(
> 
> 
> In mem_cgroup_alloc() we currently do either kmalloc() or vmalloc() then 
> followed by memset() to zero the memory. This can be more efficiently 
> achieved by using kzalloc() and vzalloc().
> 
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Looks good to me, but there is also the memset after kmalloc in
alloc_mem_cgroup_per_zone_info().  Can you switch that over as well in
this patch?  You can pass __GFP_ZERO to kmalloc_node() for zeroing.

Thanks!

Re: [PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Jesper Juhl]

On Mon, 1 Nov 2010, Johannes Weiner wrote:

> On Mon, Nov 01, 2010 at 08:40:56PM +0100, Jesper Juhl wrote:
> > Hi (please CC me on replies),
> > 
> > 
> > Apologies to those who receive this multiple times. I screwed up the To: 
> > field in my original mail :-(
> > 
> > 
> > In mem_cgroup_alloc() we currently do either kmalloc() or vmalloc() then 
> > followed by memset() to zero the memory. This can be more efficiently 
> > achieved by using kzalloc() and vzalloc().
> > 
> > 
> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> 
> Looks good to me, but there is also the memset after kmalloc in
> alloc_mem_cgroup_per_zone_info(). 

Dang, I missed that one. Thanks for pointing it out.

Hmm, I'm wondering if we should perhaps add kzalloc_node()/vzalloc_node() 
just like kzalloc() and vzalloc()..


> Can you switch that over as well in
> this patch?  You can pass __GFP_ZERO to kmalloc_node() for zeroing.
> 

Sure thing.


Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 memcontrol.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 9a99cfa..bc32ffe 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4169,13 +4169,11 @@ static int alloc_mem_cgroup_per_zone_info(struct mem_cgroup *mem, int node)
 	 */
 	if (!node_state(node, N_NORMAL_MEMORY))
 		tmp = -1;
-	pn = kmalloc_node(sizeof(*pn), GFP_KERNEL, tmp);
+	pn = kmalloc_node(sizeof(*pn), GFP_KERNEL|__GFP_ZERO, tmp);
 	if (!pn)
 		return 1;
 
 	mem->info.nodeinfo[node] = pn;
-	memset(pn, 0, sizeof(*pn));
-
 	for (zone = 0; zone < MAX_NR_ZONES; zone++) {
 		mz = &pn->zoneinfo[zone];
 		for_each_lru(l)
@@ -4199,14 +4197,13 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
 
 	/* Can be very big if MAX_NUMNODES is very big */
 	if (size < PAGE_SIZE)
-		mem = kmalloc(size, GFP_KERNEL);
+		mem = kzalloc(size, GFP_KERNEL);
 	else
-		mem = vmalloc(size);
+		mem = vzalloc(size);
 
 	if (!mem)
 		return NULL;
 
-	memset(mem, 0, size);
 	mem->stat = alloc_percpu(struct mem_cgroup_stat_cpu);
 	if (!mem->stat) {
 		if (size < PAGE_SIZE)


-- 
Jesper Juhl <jj@chaosbits.net>             http://www.chaosbits.net/
Plain text mails only, please      http://www.expita.com/nomime.html
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html

Re: [PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Minchan Kim]

On Tue, Nov 2, 2010 at 4:59 AM, Jesper Juhl <jj@chaosbits.net> wrote:
> On Mon, 1 Nov 2010, Johannes Weiner wrote:
>
>> On Mon, Nov 01, 2010 at 08:40:56PM +0100, Jesper Juhl wrote:
>> > Hi (please CC me on replies),
>> >
>> >
>> > Apologies to those who receive this multiple times. I screwed up the To:
>> > field in my original mail :-(
>> >
>> >
>> > In mem_cgroup_alloc() we currently do either kmalloc() or vmalloc() then
>> > followed by memset() to zero the memory. This can be more efficiently
>> > achieved by using kzalloc() and vzalloc().
>> >
>> >
>> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
>>
>> Looks good to me, but there is also the memset after kmalloc in
>> alloc_mem_cgroup_per_zone_info().
>
> Dang, I missed that one. Thanks for pointing it out.
>
> Hmm, I'm wondering if we should perhaps add kzalloc_node()/vzalloc_node()
> just like kzalloc() and vzalloc()..

I am not against that.
As looking code, there are many places to use kzalloc_node but few vzalloc_node.
Although it is, Only adding kzalloc_node would make code rather ugly like this.

if (some size > limit)
  ptr =  kzalloc_node(...);
else  {
  prt = vmalloc_node(...)
  vmalloced = 1;
}

if (ptr && vmalloced)
   memset(prt, xxxx);

So if we will add kzalloc_node, we have to add vzalloc_node, too.

>
>> Can you switch that over as well in
>> this patch?  You can pass __GFP_ZERO to kmalloc_node() for zeroing.
>>
>
> Sure thing.
>
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

-- 
Kind regards,
Minchan Kim

Re: [PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Johannes Weiner]

On Mon, Nov 01, 2010 at 08:59:13PM +0100, Jesper Juhl wrote:
> On Mon, 1 Nov 2010, Johannes Weiner wrote:
> 
> > On Mon, Nov 01, 2010 at 08:40:56PM +0100, Jesper Juhl wrote:
> > > In mem_cgroup_alloc() we currently do either kmalloc() or vmalloc() then 
> > > followed by memset() to zero the memory. This can be more efficiently 
> > > achieved by using kzalloc() and vzalloc().
> > > 
> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > 
> > Looks good to me, but there is also the memset after kmalloc in
> > alloc_mem_cgroup_per_zone_info().  Can you switch that over as well
> > in this patch?  You can pass __GFP_ZERO to kmalloc_node() for zeroing.
>
> Sure thing.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Thanks.

Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

Re: [PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Wu Fengguang]

On Mon, Nov 01, 2010 at 08:59:13PM +0100, Jesper Juhl wrote:

> @@ -4169,13 +4169,11 @@ static int alloc_mem_cgroup_per_zone_info(struct mem_cgroup *mem, int node)
>  	 */
>  	if (!node_state(node, N_NORMAL_MEMORY))
>  		tmp = -1;
> -	pn = kmalloc_node(sizeof(*pn), GFP_KERNEL, tmp);
> +	pn = kmalloc_node(sizeof(*pn), GFP_KERNEL|__GFP_ZERO, tmp);

Use the simpler kzalloc_node()? It's introduced here:

        commit 979b0fea2d9ae5d57237a368d571cbc84655fba6
        Author: Jeff Layton <jlayton@redhat.com>
        Date:   Thu Jun 5 22:47:00 2008 -0700

            vm: add kzalloc_node() inline


Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>

Thanks,
Fengguang

Re: [PATCH] cgroup: prefer [kv]zalloc over [kv]malloc+memset in memory controller code.

[Balbir Singh]

* Jesper Juhl <jj@chaosbits.net> [2010-11-01 20:40:56]:

> Hi (please CC me on replies),
> 
> 
> Apologies to those who receive this multiple times. I screwed up the To: 
> field in my original mail :-(
> 
> 
> In mem_cgroup_alloc() we currently do either kmalloc() or vmalloc() then 
> followed by memset() to zero the memory. This can be more efficiently 
> achieved by using kzalloc() and vzalloc().
> 
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  memcontrol.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 9a99cfa..90da698 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4199,14 +4199,13 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
> 
>  	/* Can be very big if MAX_NUMNODES is very big */
>  	if (size < PAGE_SIZE)
> -		mem = kmalloc(size, GFP_KERNEL);
> +		mem = kzalloc(size, GFP_KERNEL);
>  	else
> -		mem = vmalloc(size);
> +		mem = vzalloc(size);
> 
>  	if (!mem)
>  		return NULL;
> 
> -	memset(mem, 0, size);
>  	mem->stat = alloc_percpu(struct mem_cgroup_stat_cpu);
>  	if (!mem->stat) {
>  		if (size < PAGE_SIZE)
>

 
Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
 

-- 
	Three Cheers,
	Balbir