[PATCH] list_add corruption in slab.c

[PATCH] list_add corruption in slab.c

[Daniel Yeisley]

I've been seeing list_add corruption in slab.c on the ES7000 since the
2.6.24.1 kernel.  There are several places where the initkmem_list3
array is access by [somevalue + node].  This also needs to be done in
kmem_cache_init().  

Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>

---
diff -Naur linux-2.6.25-rc5/mm/slab.c linux-2.6.25-rc5-new/mm/slab.c
--- linux-2.6.25-rc5/mm/slab.c	2008-03-10 01:22:27.000000000 -0400
+++ linux-2.6.25-rc5-new/mm/slab.c	2008-03-20 13:59:24.000000000 -0400
@@ -1481,7 +1481,7 @@
 	list_add(&cache_cache.next, &cache_chain);
 	cache_cache.colour_off = cache_line_size();
 	cache_cache.array[smp_processor_id()] = &initarray_cache.cache;
-	cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE];
+	cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE + node];
 
 	/*
 	 * struct kmem_cache size depends on nr_node_ids, which

Re: [PATCH] list_add corruption in slab.c

[Pekka Enberg]

Hi Daniel,

On Tue, Mar 25, 2008 at 6:57 PM, Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> I've been seeing list_add corruption in slab.c on the ES7000 since the
>  2.6.24.1 kernel.  There are several places where the initkmem_list3
>  array is access by [somevalue + node].  This also needs to be done in
>  kmem_cache_init().
>
>  Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>
>
>  ---
>  diff -Naur linux-2.6.25-rc5/mm/slab.c linux-2.6.25-rc5-new/mm/slab.c
>  --- linux-2.6.25-rc5/mm/slab.c  2008-03-10 01:22:27.000000000 -0400
>  +++ linux-2.6.25-rc5-new/mm/slab.c      2008-03-20 13:59:24.000000000 -0400
>  @@ -1481,7 +1481,7 @@
>         list_add(&cache_cache.next, &cache_chain);
>         cache_cache.colour_off = cache_line_size();
>         cache_cache.array[smp_processor_id()] = &initarray_cache.cache;
>  -       cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE];
>  +       cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE + node];

Good catch! You'd need to fix up the use of initkmem_list3 farther
down in kmem_init_cache():

>        /* 5) Replace the bootstrap kmem_list3's */
>        {
>                int nid;
>
>                for_each_online_node(nid) {
>                        init_list(&cache_cache, &initkmem_list3[CACHE_CACHE], nid);

Care to send a tested patch that fixes that as well?

                          Pekka

Re: [PATCH] list_add corruption in slab.c

[Daniel Yeisley]

On Tue, 2008-03-25 at 20:45 +0200, Pekka Enberg wrote:
> Hi Daniel,
> 
> On Tue, Mar 25, 2008 at 6:57 PM, Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> > I've been seeing list_add corruption in slab.c on the ES7000 since the
> >  2.6.24.1 kernel.  There are several places where the initkmem_list3
> >  array is access by [somevalue + node].  This also needs to be done in
> >  kmem_cache_init().
> >
> >  Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>
> >
> >  ---
> >  diff -Naur linux-2.6.25-rc5/mm/slab.c linux-2.6.25-rc5-new/mm/slab.c
> >  --- linux-2.6.25-rc5/mm/slab.c  2008-03-10 01:22:27.000000000 -0400
> >  +++ linux-2.6.25-rc5-new/mm/slab.c      2008-03-20 13:59:24.000000000 -0400
> >  @@ -1481,7 +1481,7 @@
> >         list_add(&cache_cache.next, &cache_chain);
> >         cache_cache.colour_off = cache_line_size();
> >         cache_cache.array[smp_processor_id()] = &initarray_cache.cache;
> >  -       cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE];
> >  +       cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE + node];
> 
> Good catch! You'd need to fix up the use of initkmem_list3 farther
> down in kmem_init_cache():
> 
> >        /* 5) Replace the bootstrap kmem_list3's */
> >        {
> >                int nid;
> >
> >                for_each_online_node(nid) {
> >                        init_list(&cache_cache, &initkmem_list3[CACHE_CACHE], nid);
> 
> Care to send a tested patch that fixes that as well?
> 
>                           Pekka

I actually saw that initkmem_list reference, but didn't change it since
my original patch fixed my list corruption.  Anyway, I made the changed
and tested it.  The system booted fine.

Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>

---
diff -Nuar linux-2.6.25-rc6/mm/slab.c linux-2.6.25-rc6-new/mm/slab.c
--- linux-2.6.25-rc6/mm/slab.c	2008-03-25 15:39:07.000000000 -0400
+++ linux-2.6.25-rc6-new/mm/slab.c	2008-03-25 15:13:01.000000000 -0400
@@ -1481,7 +1481,7 @@
 	list_add(&cache_cache.next, &cache_chain);
 	cache_cache.colour_off = cache_line_size();
 	cache_cache.array[smp_processor_id()] = &initarray_cache.cache;
-	cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE];
+	cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE + node];
 
 	/*
 	 * struct kmem_cache size depends on nr_node_ids, which
@@ -1602,7 +1602,7 @@
 		int nid;
 
 		for_each_online_node(nid) {
-			init_list(&cache_cache, &initkmem_list3[CACHE_CACHE], nid);
+			init_list(&cache_cache, &initkmem_list3[CACHE_CACHE + nid], nid);
 
 			init_list(malloc_sizes[INDEX_AC].cs_cachep,
 				  &initkmem_list3[SIZE_AC + nid], nid);

Re: [PATCH] list_add corruption in slab.c

[Pekka Enberg]

Hi Daniel,

Daniel Yeisley wrote:
> I actually saw that initkmem_list reference, but didn't change it since
> my original patch fixed my list corruption.  Anyway, I made the changed
> and tested it.  The system booted fine.

Yeah, but the second change is needed; otherwise we forget to fix up 
some of the bootstrap caches.

 > Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>

Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>

Mel, as this change is related to the memoryless node fix that went in 
2.6.24, any chance you'd give this patch a spin on your machines so we 
can get the fix in 2.6.25 and 2.6.24-stable?

			Pekka

Re: [PATCH] list_add corruption in slab.c

[Oliver Pinter]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 1554 bytes --]

Hi Pekka
this patch for 2.6.22?(http://repo.or.cz/w/linux-2.6.22.y-op.git)
--8<--        /* 1) create the cache_cache */        INIT_LIST_HEAD(&cache_chain);        list_add(&cache_cache.next, &cache_chain);        cache_cache.colour_off = cache_line_size();        cache_cache.array[smp_processor_id()] = &initarray_cache.cache;        cache_cache.nodelists[node] = &initkmem_list3[CACHE_CACHE];-->8--from 2.6.22's slab.c
On 3/25/08, Pekka Enberg <penberg@cs.helsinki.fi> wrote:> Hi Daniel,>> Daniel Yeisley wrote:> > I actually saw that initkmem_list reference, but didn't change it since> > my original patch fixed my list corruption.  Anyway, I made the changed> > and tested it.  The system booted fine.>> Yeah, but the second change is needed; otherwise we forget to fix up> some of the bootstrap caches.>>  > Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>>> Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>>> Mel, as this change is related to the memoryless node fix that went in> 2.6.24, any chance you'd give this patch a spin on your machines so we> can get the fix in 2.6.25 and 2.6.24-stable?>> 			Pekka> --> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in> the body of a message to majordomo@vger.kernel.org> More majordomo info at  http://vger.kernel.org/majordomo-info.html> Please read the FAQ at  http://www.tux.org/lkml/>

-- Thanks,Oliverÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] list_add corruption in slab.c

[Pekka Enberg]

Hi Oliver,

On Tue, Mar 25, 2008 at 11:27 PM, Oliver Pinter <oliver.pntr@gmail.com> wrote:
>  this patch for 2.6.22?

No. It fixes the following commit which is not in nor is it required for 2.6.22:

  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=556a169dab38b5100df6f4a45b655dddd3db94c1

Re: [PATCH] list_add corruption in slab.c

[Oliver Pinter]

thanks for the fast answer

On 3/25/08, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> Hi Oliver,
>
> On Tue, Mar 25, 2008 at 11:27 PM, Oliver Pinter <oliver.pntr@gmail.com>
> wrote:
> >  this patch for 2.6.22?
>
> No. It fixes the following commit which is not in nor is it required for
> 2.6.22:
>
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=556a169dab38b5100df6f4a45b655dddd3db94c1
>


-- 
Thanks,
Oliver

Re: [PATCH] list_add corruption in slab.c

[Mel Gorman]

On (25/03/08 23:13), Pekka Enberg didst pronounce:
> Hi Daniel,
> 
> Daniel Yeisley wrote:
> >I actually saw that initkmem_list reference, but didn't change it since
> >my original patch fixed my list corruption.  Anyway, I made the changed
> >and tested it.  The system booted fine.
> 
> Yeah, but the second change is needed; otherwise we forget to fix up 
> some of the bootstrap caches.
> 
> > ???Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>
> 
> Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> Mel, as this change is related to the memoryless node fix that went in 
> 2.6.24, any chance you'd give this patch a spin on your machines so we 
> can get the fix in 2.6.25 and 2.6.24-stable?
> 

Of course. Tested against 2.6.25-rc5 on normal and memoryless machines
running just kernbench. So far, I have seen no problems.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab