| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20090319181116.GA24586@csn.ul.ie>
Date: Thu, 19 Mar 2009 18:11:16 +0000
From: Mel Gorman <mel@....ul.ie>
To: Christoph Lameter <cl@...ux-foundation.org>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
Linux Memory Management List <linux-mm@...ck.org>,
Pekka Enberg <penberg@...helsinki.fi>,
Rik van Riel <riel@...hat.com>,
KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
Johannes Weiner <hannes@...xchg.org>,
Nick Piggin <npiggin@...e.de>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Lin Ming <ming.m.lin@...el.com>,
Zhang Yanmin <yanmin_zhang@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>
Subject: 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@...ux-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists