linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache().
@ 2010-01-06  7:25 Haicheng Li
  2010-01-06  8:20 ` Pekka Enberg
  2010-01-07 12:11 ` Pekka Enberg
  0 siblings, 2 replies; 8+ messages in thread
From: Haicheng Li @ 2010-01-06  7:25 UTC (permalink / raw)
  To: Christoph Lameter, linux-mm
  Cc: Matt Mackall, Pekka Enberg, Andi Kleen, Eric Dumazet,
	linux-kernel

Comparing with existing code, it's a simpler way to use kzalloc_node()
to ensure that each unused alien cache entry is NULL.

CC: Pekka Enberg <penberg@cs.helsinki.fi>
CC: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Christoph Lameter <cl@linux-foundation.org>
Reviewed-by: Matt Mackall <mpm@selenic.com>
Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>
---
  mm/slab.c |    6 ++----
  1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 7dfa481..5d1a782 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -971,13 +971,11 @@ static struct array_cache **alloc_alien_cache(int node, int limit, gfp_t gfp)

  	if (limit > 1)
  		limit = 12;
-	ac_ptr = kmalloc_node(memsize, gfp, node);
+	ac_ptr = kzalloc_node(memsize, gfp, node);
  	if (ac_ptr) {
  		for_each_node(i) {
-			if (i == node || !node_online(i)) {
-				ac_ptr[i] = NULL;
+			if (i == node || !node_online(i))
  				continue;
-			}
  			ac_ptr[i] = alloc_arraycache(node, limit, 0xbaadf00d, gfp);
  			if (!ac_ptr[i]) {
  				for (i--; i >= 0; i--)
-- 
1.5.3.8


--
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 related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache().
  2010-01-06  7:25 [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache() Haicheng Li
@ 2010-01-06  8:20 ` Pekka Enberg
  2010-01-06  8:39   ` Haicheng Li
  2010-01-07 12:11 ` Pekka Enberg
  1 sibling, 1 reply; 8+ messages in thread
From: Pekka Enberg @ 2010-01-06  8:20 UTC (permalink / raw)
  To: Haicheng Li
  Cc: Christoph Lameter, linux-mm, Matt Mackall, Andi Kleen,
	Eric Dumazet, linux-kernel

On Wed, Jan 6, 2010 at 9:25 AM, Haicheng Li <haicheng.li@linux.intel.com> wrote:
> Comparing with existing code, it's a simpler way to use kzalloc_node()
> to ensure that each unused alien cache entry is NULL.
>
> CC: Pekka Enberg <penberg@cs.helsinki.fi>
> CC: Eric Dumazet <eric.dumazet@gmail.com>
> Acked-by: Andi Kleen <ak@linux.intel.com>
> Acked-by: Christoph Lameter <cl@linux-foundation.org>
> Reviewed-by: Matt Mackall <mpm@selenic.com>
> Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com>

I can find a trace of Andi acking the previous version of this patch
but I don't see an ACK from Christoph nor a revieved-by from Matt. Was
I not CC'd on those emails or what's going on here?

                        Pekka

--
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] 8+ messages in thread

* Re: [PATCH v3] slab: initialize unused alien cache entry as NULL at  alloc_alien_cache().
  2010-01-06  8:20 ` Pekka Enberg
@ 2010-01-06  8:39   ` Haicheng Li
  2010-01-06  8:42     ` Pekka Enberg
  0 siblings, 1 reply; 8+ messages in thread
From: Haicheng Li @ 2010-01-06  8:39 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Christoph Lameter, linux-mm, Matt Mackall, Andi Kleen,
	Eric Dumazet, linux-kernel

Pekka Enberg wrote:
 > I can find a trace of Andi acking the previous version of this patch
 > but I don't see an ACK from Christoph nor a revieved-by from Matt. Was
 > I not CC'd on those emails or what's going on here?
 >

Pekka,

Christoph said he will ack this patch if remove the change of MAX_NUMNODES (see below),
so I add him directly as Acked-by in this revised patch. And also, I got review
comments from Matt for v1 and changed the patch accordingly.

Is it a violation of the rule? if so, I'm sorry, actually not quite clear with the rule.



Christoph Lameter wrote:
 > On Wed, 23 Dec 2009, Haicheng Li wrote:
 >
 >> @@ -966,18 +966,16 @@ static void *alternate_node_alloc(struct kmem_cache *,
 >> gfp_t);
 >>  static struct array_cache **alloc_alien_cache(int node, int limit, gfp_t gfp)
 >>  {
 >>  	struct array_cache **ac_ptr;
 >> -	int memsize = sizeof(void *) * nr_node_ids;
 >> +	int memsize = sizeof(void *) * MAX_NUMNODES;
 >>  	int i;
 >
 > Remove this change and I will ack the patch.
 >


--
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] 8+ messages in thread

* Re: [PATCH v3] slab: initialize unused alien cache entry as NULL at  alloc_alien_cache().
  2010-01-06  8:39   ` Haicheng Li
@ 2010-01-06  8:42     ` Pekka Enberg
  2010-01-06  8:46       ` Haicheng Li
  0 siblings, 1 reply; 8+ messages in thread
From: Pekka Enberg @ 2010-01-06  8:42 UTC (permalink / raw)
  To: Haicheng Li
  Cc: Christoph Lameter, linux-mm, Matt Mackall, Andi Kleen,
	Eric Dumazet, linux-kernel

Hi,

Haicheng Li wrote:
> Pekka Enberg wrote:
>  > I can find a trace of Andi acking the previous version of this patch
>  > but I don't see an ACK from Christoph nor a revieved-by from Matt. Was
>  > I not CC'd on those emails or what's going on here?
>  >
> 
> Christoph said he will ack this patch if remove the change of 
> MAX_NUMNODES (see below),
> so I add him directly as Acked-by in this revised patch. And also, I got 
> review
> comments from Matt for v1 and changed the patch accordingly.
> 
> Is it a violation of the rule? if so, I'm sorry, actually not quite 
> clear with the rule.

See Section 14 of Documentation/SubmittingPatches. You should never add 
tags unless they came from the said person. The ACKs from Andi is fine, 
the one from Christoph is borderline but OK and the one from Matt is 
_not_ OK.

I can fix those up but I'll wait from an explicit ACK from Christoph first.

                         Pekka

--
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] 8+ messages in thread

* Re: [PATCH v3] slab: initialize unused alien cache entry as NULL at  alloc_alien_cache().
  2010-01-06  8:42     ` Pekka Enberg
@ 2010-01-06  8:46       ` Haicheng Li
  2010-01-07 17:57         ` Christoph Lameter
  0 siblings, 1 reply; 8+ messages in thread
From: Haicheng Li @ 2010-01-06  8:46 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Christoph Lameter, linux-mm, Matt Mackall, Andi Kleen,
	Eric Dumazet, linux-kernel

understood. thanks!

Pekka Enberg wrote:
> Hi,
> 
> Haicheng Li wrote:
>> Pekka Enberg wrote:
>>  > I can find a trace of Andi acking the previous version of this patch
>>  > but I don't see an ACK from Christoph nor a revieved-by from Matt. Was
>>  > I not CC'd on those emails or what's going on here?
>>  >
>>
>> Christoph said he will ack this patch if remove the change of 
>> MAX_NUMNODES (see below),
>> so I add him directly as Acked-by in this revised patch. And also, I 
>> got review
>> comments from Matt for v1 and changed the patch accordingly.
>>
>> Is it a violation of the rule? if so, I'm sorry, actually not quite 
>> clear with the rule.
> 
> See Section 14 of Documentation/SubmittingPatches. You should never add 
> tags unless they came from the said person. The ACKs from Andi is fine, 
> the one from Christoph is borderline but OK and the one from Matt is 
> _not_ OK.
> 
> I can fix those up but I'll wait from an explicit ACK from Christoph first.
> 
>                         Pekka

--
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] 8+ messages in thread

* Re: [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache().
  2010-01-06  7:25 [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache() Haicheng Li
  2010-01-06  8:20 ` Pekka Enberg
@ 2010-01-07 12:11 ` Pekka Enberg
  2010-01-07 18:10   ` Matt Mackall
  1 sibling, 1 reply; 8+ messages in thread
From: Pekka Enberg @ 2010-01-07 12:11 UTC (permalink / raw)
  To: Haicheng Li
  Cc: Christoph Lameter, linux-mm, Matt Mackall, Andi Kleen,
	Eric Dumazet, linux-kernel

Haicheng Li kirjoitti:
> Comparing with existing code, it's a simpler way to use kzalloc_node()
> to ensure that each unused alien cache entry is NULL.
> 
> CC: Pekka Enberg <penberg@cs.helsinki.fi>
> CC: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  mm/slab.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index 7dfa481..5d1a782 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -971,13 +971,11 @@ static struct array_cache **alloc_alien_cache(int 
> node, int limit, gfp_t gfp)
> 
>      if (limit > 1)
>          limit = 12;
> -    ac_ptr = kmalloc_node(memsize, gfp, node);
> +    ac_ptr = kzalloc_node(memsize, gfp, node);
>      if (ac_ptr) {
>          for_each_node(i) {
> -            if (i == node || !node_online(i)) {
> -                ac_ptr[i] = NULL;
> +            if (i == node || !node_online(i))
>                  continue;
> -            }
>              ac_ptr[i] = alloc_arraycache(node, limit, 0xbaadf00d, gfp);
>              if (!ac_ptr[i]) {
>                  for (i--; i >= 0; i--)

Christoph? Matt?

--
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] 8+ messages in thread

* Re: [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache().
  2010-01-06  8:46       ` Haicheng Li
@ 2010-01-07 17:57         ` Christoph Lameter
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Lameter @ 2010-01-07 17:57 UTC (permalink / raw)
  To: Haicheng Li
  Cc: Pekka Enberg, linux-mm, Matt Mackall, Andi Kleen, Eric Dumazet,
	linux-kernel

Explicit Ack..

Acked-by: Christoph Lameter <cl@linux-foundation.org>

--
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] 8+ messages in thread

* Re: [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache().
  2010-01-07 12:11 ` Pekka Enberg
@ 2010-01-07 18:10   ` Matt Mackall
  0 siblings, 0 replies; 8+ messages in thread
From: Matt Mackall @ 2010-01-07 18:10 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Haicheng Li, Christoph Lameter, linux-mm, Andi Kleen,
	Eric Dumazet, linux-kernel

On Thu, 2010-01-07 at 14:11 +0200, Pekka Enberg wrote:
> Haicheng Li kirjoitti:
> > Comparing with existing code, it's a simpler way to use kzalloc_node()
> > to ensure that each unused alien cache entry is NULL.
> > 
> > CC: Pekka Enberg <penberg@cs.helsinki.fi>
> > CC: Eric Dumazet <eric.dumazet@gmail.com>
> > ---
> >  mm/slab.c |    6 ++----
> >  1 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/slab.c b/mm/slab.c
> > index 7dfa481..5d1a782 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -971,13 +971,11 @@ static struct array_cache **alloc_alien_cache(int 
> > node, int limit, gfp_t gfp)
> > 
> >      if (limit > 1)
> >          limit = 12;
> > -    ac_ptr = kmalloc_node(memsize, gfp, node);
> > +    ac_ptr = kzalloc_node(memsize, gfp, node);
> >      if (ac_ptr) {
> >          for_each_node(i) {
> > -            if (i == node || !node_online(i)) {
> > -                ac_ptr[i] = NULL;
> > +            if (i == node || !node_online(i))
> >                  continue;
> > -            }
> >              ac_ptr[i] = alloc_arraycache(node, limit, 0xbaadf00d, gfp);
> >              if (!ac_ptr[i]) {
> >                  for (i--; i >= 0; i--)
> 
> Christoph? Matt?

Looks like a fine cleanup.

Acked-by: Matt Mackall <mpm@selenic.com>

-- 
http://selenic.com : development and support for Mercurial and Linux


--
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] 8+ messages in thread

end of thread, other threads:[~2010-01-07 18:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-06  7:25 [PATCH v3] slab: initialize unused alien cache entry as NULL at alloc_alien_cache() Haicheng Li
2010-01-06  8:20 ` Pekka Enberg
2010-01-06  8:39   ` Haicheng Li
2010-01-06  8:42     ` Pekka Enberg
2010-01-06  8:46       ` Haicheng Li
2010-01-07 17:57         ` Christoph Lameter
2010-01-07 12:11 ` Pekka Enberg
2010-01-07 18:10   ` Matt Mackall

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