kmalloc() fix^2

kmalloc() fix^2

Post by David Mosberge » Sat, 12 Apr 2003 00:00:21



It's all very embarassing, but my previous patch was horribly broken:
it added the NULL terminator to the wrong array...  Of course, adding
it to the correct array uncovered another bug... ;-(

Patch below fixes both problems.  Reall, I mean it.

Andrew, you may want to double-check---I didn't look through all of
slab.c whether there might be problems (there was no other mention of
ARRAY_SIZE(malloc_sizes) though.

Thanks,

        --david

===== mm/slab.c 1.74 vs edited =====
--- 1.74/mm/slab.c      Wed Apr  9 13:28:18 2003

 } malloc_sizes[] = {
 #define CACHE(x) { .cs_size = (x) },
 #include <linux/kmalloc_sizes.h>
+       {0, }
 #undef CACHE
 };

 } cache_names[] = {
 #define CACHE(x) { .name = "size-" #x, .name_dma = "size-" #x "(DMA)" },
 #include <linux/kmalloc_sizes.h>
-       { 0, }
 #undef CACHE
 };

        if (num_physpages > (32 << 20) >> PAGE_SHIFT)
                slab_break_gfp_order = BREAK_GFP_ORDER_HI;

-       for (i = 0; i < ARRAY_SIZE(malloc_sizes); i++) {
+       for (i = 0; i < ARRAY_SIZE(malloc_sizes) - 1; i++) {
                struct cache_sizes *sizes = malloc_sizes + i;
                /* For performance, all the general caches are L1 aligned.
                 * This should be particularly beneficial on SMP boxes, as it
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

kmalloc() fix^2

Post by Brian Gers » Sat, 12 Apr 2003 02:10:08



> It's all very embarassing, but my previous patch was horribly broken:
> it added the NULL terminator to the wrong array...  Of course, adding
> it to the correct array uncovered another bug... ;-(

> Patch below fixes both problems.  Reall, I mean it.

> Andrew, you may want to double-check---I didn't look through all of
> slab.c whether there might be problems (there was no other mention of
> ARRAY_SIZE(malloc_sizes) though.

> Thanks,

>    --david

My fault, I got a bit overzealous while cleaning that code up.  Here's a
better patch that gets rid of ARRAY_SIZE since the null is readded.

--
                                Brian Gerst

[ kmalloc_sizes-3 1K ]
--- linux-2.5.67-bk/mm/slab.c   2003-04-10 08:36:50.000000000 -0400

 } malloc_sizes[] = {
 #define CACHE(x) { .cs_size = (x) },
 #include <linux/kmalloc_sizes.h>
+       { 0, }
 #undef CACHE
 };

 /* Must match cache_sizes above. Out of line to keep cache footprint low. */
-static struct {
+static struct cache_names {
        char *name;
        char *name_dma;

  */
 void __init kmem_cache_sizes_init(void)
 {
-       int i;
+       struct cache_sizes *sizes = malloc_sizes;
+       struct cache_names *names = cache_names;
+
        /*
         * Fragmentation resistance on low memory - only use bigger

        if (num_physpages > (32 << 20) >> PAGE_SHIFT)
                slab_break_gfp_order = BREAK_GFP_ORDER_HI;

-       for (i = 0; i < ARRAY_SIZE(malloc_sizes); i++) {
-               struct cache_sizes *sizes = malloc_sizes + i;
+       while (sizes->cs_size) {
                /* For performance, all the general caches are L1 aligned.
                 * This should be particularly beneficial on SMP boxes, as it
                 * eliminates "false sharing".
                 * Note for systems short on memory removing the alignment will
                 * allow tighter packing of the smaller caches. */
                sizes->cs_cachep = kmem_cache_create(
-                       cache_names[i].name, sizes->cs_size,
+                       names->name, sizes->cs_size,
                        0, SLAB_HWCACHE_ALIGN, NULL, NULL);
                if (!sizes->cs_cachep)

                }

                sizes->cs_dmacachep = kmem_cache_create(
-                       cache_names[i].name_dma, sizes->cs_size,
+                       names->name_dma, sizes->cs_size,
                        0, SLAB_CACHE_DMA|SLAB_HWCACHE_ALIGN, NULL, NULL);
                if (!sizes->cs_dmacachep)
                        BUG();
+
+               sizes++;
+               names++;
        }
        /*
         * The generic caches are running - time to kick out the

 
 
 

1. fix weird kmalloc bug

Last night, Manfred and I found an interesting bug with kmalloc on
ppc32, where the kmalloc in alloc_super() (fs/super.c) was requesting
432 bytes but only getting 256 bytes.  The reason was that PAGE_SIZE
wasn't defined at the point where the kmalloc() inline function
occurs.  Thus the CACHE(32) entry got omitted from the list in
kmalloc_sizes.h, and kmalloc therefore used the entry in
malloc_sizes[] before the correct entry.

This patch fixes it by including asm/page.h and asm/cache.h in
linux/slab.h.  The list in kmalloc_sizes.h depends on L1_CACHE_BYTES
as well as PAGE_SIZE, which is why I added asm/cache.h.

Paul.

diff -urN linux-2.5/include/linux/slab.h pmac-2.5-smp/include/linux/slab.h
--- linux-2.5/include/linux/slab.h      2003-06-12 10:43:55.000000000 +1000

 #include       <linux/gfp.h>
 #include       <linux/types.h>
+#include       <asm/page.h>
+#include       <asm/cache.h>

 /* flags for kmem_cache_alloc() */
 #define        SLAB_NOFS               GFP_NOFS
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

2. passwd : unknown user root

3. optimize fixed size kmalloc calls

4. REading linux files from dos

5. ALS100: kmalloc params fix

6. Announce: UNIX, shell programming and Awk courseware material

7. ES968: kmalloc params fix

8. Sound on PPC7200/DebianPPC

9. PATCH: fix ALi 32bitisms, fix ALi FIFO, fix ALi IRQ crash

10. How to kmalloc 700kB reliably?

11. [2.5] CIFS cifsfs.c check kmalloc return

12. kmalloc: Size (xxxxxxx) too large ???

13. How to allocate kmalloc/fw_kmalloc/fwhmem ?