Re: [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@xxxxxxxxxxxxxxxxxxxx>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages