lkml.org 
[lkml]   [2009]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 24/27] Convert gfp_zone() to use a table of precalculated values
    On Thu, Mar 19, 2009 at 12:53:34PM -0400, Christoph Lameter wrote:
    > On Thu, 19 Mar 2009, Christoph Lameter wrote:
    >
    > > It would work if we could check for BAD_ZONE with a VM_BUG_ON or a
    > > BUILD_BUG_ON. If I get some time I will look into this.
    >
    > Here is such a patch. Boots on my machine and working with that kernel
    > now. There is a slight gcc problem in that the table is likely repeated
    > for each compilation unit. Anyone know how to fix that?
    >

    I ran into exactly that problem and ended up shoving the table into
    page_alloc.c but then there is no benefits from having the table statically
    declared because there is no constant folding.

    Just to confirm: With your patch, gfp_zone_table() does end up in different
    complation units

    $ readelf -s vmlinux | grep gfp_zone_table
    5479: c03a9ea0 64 OBJECT LOCAL DEFAULT 5 gfp_zone_table
    5537: c03a9f20 64 OBJECT LOCAL DEFAULT 5 gfp_zone_table
    5753: c03a9fe0 64 OBJECT LOCAL DEFAULT 5 gfp_zone_table

    > Subject: Use a table lookup for gfp_zone and check for errors in flags passed to the page allocator
    >
    > Use a table to lookup the zone to use given gfp_flags using gfp_zone().
    >
    > This simplifies the code in gfp_zone() and also keeps the ability of the compiler to
    > use constant folding to get rid of gfp_zone processing.
    >
    > One problem with this patch is that we define a static const array in gfp.h. This results
    > in every compilation unit to reserve its own space for the array. There must be some
    > trick to get the compiler to allocate this only once. The contents of the array
    > must be described in the header file otherwise the compiler will not be able to
    > determine the value of a lookup in the table.
    >

    Yep, that is exactly the problem I hit but I didn't find a suitable answer.

    > Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
    >
    > Index: linux-2.6/include/linux/gfp.h
    > ===================================================================
    > --- linux-2.6.orig/include/linux/gfp.h 2009-03-19 11:43:32.000000000 -0500
    > +++ linux-2.6/include/linux/gfp.h 2009-03-19 11:48:38.000000000 -0500
    > @@ -19,7 +19,8 @@
    > #define __GFP_DMA ((__force gfp_t)0x01u)
    > #define __GFP_HIGHMEM ((__force gfp_t)0x02u)
    > #define __GFP_DMA32 ((__force gfp_t)0x04u)
    > -
    > +#define __GFP_MOVABLE ((__force gfp_t)0x08u) /* Page is movable */
    > +#define GFP_ZONEMASK ((__force gfp_t)0x0fu)

    To avoid magic number syndrome, you could define GFP_ZONEMASK as

    (__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | __GFP_MOVABLE)

    > /*
    > * Action modifiers - doesn't change the zoning
    > *
    > @@ -49,7 +50,6 @@
    > #define __GFP_HARDWALL ((__force gfp_t)0x20000u) /* Enforce hardwall cpuset memory allocs */
    > #define __GFP_THISNODE ((__force gfp_t)0x40000u)/* No fallback, no policies */
    > #define __GFP_RECLAIMABLE ((__force gfp_t)0x80000u) /* Page is reclaimable */
    > -#define __GFP_MOVABLE ((__force gfp_t)0x100000u) /* Page is movable */
    >
    > #define __GFP_BITS_SHIFT 21 /* Room for 21 __GFP_FOO bits */
    > #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
    > @@ -111,24 +111,56 @@
    > ((gfp_flags & __GFP_RECLAIMABLE) != 0);
    > }
    >
    > -static inline enum zone_type gfp_zone(gfp_t flags)
    > -{
    > +#ifdef CONFIG_ZONE_HIGHMEM
    > +#define OPT_ZONE_HIGHMEM ZONE_HIGHMEM
    > +#else
    > +#define OPT_ZONE_HIGHMEM ZONE_NORMAL
    > +#endif
    > +
    > #ifdef CONFIG_ZONE_DMA
    > - if (flags & __GFP_DMA)
    > - return ZONE_DMA;
    > +#define OPT_ZONE_DMA ZONE_DMA
    > +#else
    > +#define OPT_ZONE_DMA ZONE_NORMAL
    > #endif
    > +
    > #ifdef CONFIG_ZONE_DMA32
    > - if (flags & __GFP_DMA32)
    > - return ZONE_DMA32;
    > +#define OPT_ZONE_DMA32 ZONE_DMA32
    > +#else
    > +#define OPT_ZONE_DMA32 OPT_ZONE_DMA
    > #endif
    > - if ((flags & (__GFP_HIGHMEM | __GFP_MOVABLE)) ==
    > - (__GFP_HIGHMEM | __GFP_MOVABLE))
    > - return ZONE_MOVABLE;
    > -#ifdef CONFIG_HIGHMEM
    > - if (flags & __GFP_HIGHMEM)
    > - return ZONE_HIGHMEM;
    > +
    > +#define BAD_ZONE MAX_NR_ZONES
    > +
    > +static const enum zone_type gfp_zone_table[GFP_ZONEMASK + 1] = {
    > + ZONE_NORMAL, /* 00 No flags set */
    > + OPT_ZONE_DMA, /* 01 GFP_DMA */
    > + OPT_ZONE_HIGHMEM, /* 02 GFP_HIGHMEM */
    > + BAD_ZONE, /* 03 GFP_HIGHMEM GFP_DMA */
    > + OPT_ZONE_DMA32, /* 04 GFP_DMA32 */
    > + BAD_ZONE, /* 05 GFP_DMA32 GFP_DMA */
    > + BAD_ZONE, /* 06 GFP_DMA32 GFP_HIGHMEM */
    > + BAD_ZONE, /* 07 GFP_DMA32 GFP_HIGHMEM GFP_DMA */
    > + ZONE_NORMAL, /* 08 ZONE_MOVABLE */
    > + OPT_ZONE_DMA, /* 09 MOVABLE + DMA */
    > + ZONE_MOVABLE, /* 0A MOVABLE + HIGHMEM */
    > + BAD_ZONE, /* 0B MOVABLE + HIGHMEM + DMA */
    > + OPT_ZONE_DMA32, /* 0C MOVABLE + DMA32 */
    > + BAD_ZONE, /* 0D MOVABLE + DMA32 + DMA */
    > + BAD_ZONE, /* 0E MOVABLE + DMA32 + HIGHMEM */
    > + BAD_ZONE /* 0F MOVABLE + DMA32 + HIGHMEM + DMA */
    > +};
    > +
    > +static inline enum zone_type gfp_zone(gfp_t flags)
    > +{
    > + enum zone_type zone = gfp_zone_table[flags & 0xf];
    > +

    flags & GFP_ZONEMASK here

    > + if (__builtin_constant_p(zone))
    > + BUILD_BUG_ON(zone == BAD_ZONE);
    > +#ifdef CONFIG_DEBUG_VM
    > + else
    > + BUG_ON(zone == BAD_ZONE);
    > #endif

    That could be made a bit prettier with

    if (__builtin_constant_p(zone))
    BUILD_BUG_ON(zone == BAD_ZONE);
    VM_BUG_ON(zone == BAD_ZONE);

    > - return ZONE_NORMAL;
    > + return zone;
    > }
    >
    > /*
    > Index: linux-2.6/include/linux/mmzone.h
    > ===================================================================
    > --- linux-2.6.orig/include/linux/mmzone.h 2009-03-19 11:47:00.000000000 -0500
    > +++ linux-2.6/include/linux/mmzone.h 2009-03-19 11:47:54.000000000 -0500
    > @@ -240,7 +240,8 @@
    > ZONE_HIGHMEM,
    > #endif
    > ZONE_MOVABLE,
    > - __MAX_NR_ZONES
    > + __MAX_NR_ZONES,
    > + BAD_ZONE
    > };
    >
    > #ifndef __GENERATING_BOUNDS_H
    >

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


    \
     
     \ /
      Last update: 2009-03-19 19:15    [W:0.036 / U:0.388 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site