list_add corruption in slab.c

PATCH In review Daniel Yeisley 4 participants 8 messages Mar 25, 2008 – Mar 26, 2008 Memory management

Share & tools

Daniel Yeisley

author

2008-03-25 17:49 UTC #0 lore raw

PATCH 1/1 list_add corruption in slab.c

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 <[email protected]>

---

mm/slab.c +1 -1

diff -Naur linux-2.6.25-rc5/mm/slab.c linux-2.6.25-rc5-new/mm/slab.c
@@ -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

Pekka Enberg

2008-03-25 18:45 UTC #1 lore raw

Hi Daniel,

Quoted 12 lines · 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 <[email protected]>
---
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 @@

Quoted 5 lines

        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():

Quoted 6 lines

       /* 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

Daniel Yeisley

author

2008-03-25 21:03 UTC #2 lore raw

Quoted 28 lines · Pekka Enberg

Hi Daniel,
Quoted 12 lines · 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 <[email protected]>
---
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 @@
Quoted 5 lines
        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():
Quoted 4 lines
       /* 5) Replace the bootstrap kmem_list3's */
       {
               int nid;

Quoted 6 lines

Quoted 2 lines
               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 <[email protected]>

---

mm/slab.c +2 -2

diff -Nuar linux-2.6.25-rc6/mm/slab.c linux-2.6.25-rc6-new/mm/slab.c
@@ -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);

Pekka Enberg

Reviewed-by

2008-03-25 21:15 UTC #3 lore raw

Hi Daniel,

Quoted 3 lines · Daniel Yeisley

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.

Quoted 1 line

Signed-off-by: Dan Yeisley <[email protected]>

Reviewed-by Pekka Enberg <[email protected]>

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

Oliver Pinter

2008-03-25 21:27 UTC #4 lore raw

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 <[email protected]> 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 <[email protected]>>> Reviewed-by: Pekka Enberg <[email protected]>>> 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 [email protected]> More majordomo info at http://vger.kernel.org/majordomo-info.html> Please read the FAQ at http://www.tux.org/lkml/>

-- Thanks,Oliver��{.n�+��+%��ݶ��w��{.n�+��{��G��{ay�ʇڙ�,j��f��h��z_�(�階�ݢj"��m��G��?��&��~��iO��z��v�^�m��?�I�

Pekka Enberg

2008-03-25 21:41 UTC #5 lore raw

Hi Oliver,

Quoted 1 line · Oliver Pinter

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

Oliver Pinter

2008-03-25 21:46 UTC #6 lore raw

thanks for the fast answer

Quoted 12 lines · Pekka Enberg

Hi Oliver,
Quoted 1 line · Oliver Pinter
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

Signature · 2 lines

Thanks,
Oliver

Mel Gorman

2008-03-26 14:16 UTC #7 lore raw

In reply to Pekka Enberg

On (25/03/08 23:13), Pekka Enberg didst pronounce:

Quoted 10 lines

Hi Daniel,
Quoted 3 lines · Daniel Yeisley
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.

Quoted 8 lines

Quoted 1 line
???Signed-off-by: Dan Yeisley <[email protected]>
Reviewed-by Pekka Enberg <[email protected]>
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.

Signature · 3 lines

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