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 <[email protected]>
CC: Eric Dumazet <[email protected]>
Acked-by: Andi Kleen <[email protected]>
Acked-by: Christoph Lameter <[email protected]>
Reviewed-by: Matt Mackall <[email protected]>
Signed-off-by: Haicheng Li <[email protected]>
---
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
On Wed, Jan 6, 2010 at 9:25 AM, Haicheng Li <[email protected]> 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 <[email protected]>
> CC: Eric Dumazet <[email protected]>
> Acked-by: Andi Kleen <[email protected]>
> Acked-by: Christoph Lameter <[email protected]>
> Reviewed-by: Matt Mackall <[email protected]>
> Signed-off-by: Haicheng Li <[email protected]>
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
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.
>
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
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
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 <[email protected]>
> CC: Eric Dumazet <[email protected]>
> ---
> 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?
Explicit Ack..
Acked-by: Christoph Lameter <[email protected]>
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 <[email protected]>
> > CC: Eric Dumazet <[email protected]>
> > ---
> > 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 <[email protected]>
--
http://selenic.com : development and support for Mercurial and Linux