Messages in this thread | | | Date | Tue, 24 Feb 2009 17:07:21 +0000 | From | Mel Gorman <> | Subject | Re: [PATCH 04/19] Convert gfp_zone() to use a table of precalculated values |
| |
On Tue, Feb 24, 2009 at 11:43:29AM -0500, Christoph Lameter wrote: > On Tue, 24 Feb 2009, Mel Gorman wrote: > > > static inline enum zone_type gfp_zone(gfp_t flags) > > { > > -#ifdef CONFIG_ZONE_DMA > > - if (flags & __GFP_DMA) > > - return ZONE_DMA; > > -#endif > > -#ifdef CONFIG_ZONE_DMA32 > > - if (flags & __GFP_DMA32) > > - return ZONE_DMA32; > > -#endif > > - if ((flags & (__GFP_HIGHMEM | __GFP_MOVABLE)) == > > - (__GFP_HIGHMEM | __GFP_MOVABLE)) > > - return ZONE_MOVABLE; > > -#ifdef CONFIG_HIGHMEM > > - if (flags & __GFP_HIGHMEM) > > - return ZONE_HIGHMEM; > > -#endif > > - return ZONE_NORMAL; > > + return gfp_zone_table[flags & GFP_ZONEMASK]; > > } > > Aassume > > GFP_DMA = 0x01 > GFP_DMA32 = 0x02 > GFP_MOVABLE = 0x04 > GFP_HIGHMEM = 0x08 > > ZONE_NORMAL = 0 > ZONE_DMA = 1 > ZONE_DMA32 = 2 > ZONE_MOVABLE = 3 > ZONE_HIGHMEM = 4 > > then we could implement gfp_zone simply as: > > static inline enum zone_type gfp_zone(gfp_t flags) > { > return ffs(flags & 0xf); > } >
A few points immediately spring to mind
o What's the cost of ffs? o The altering of zone order is not without consequence. The zonelist walkers for example make the assumion that the higher the zone index, then the "higher" it is. i.e. NORMAL is a bigger index than DMA, HIGHMEM is bigger index than NORMAL etc. o I think movable ends up the wrong "side" of highmem in terms of zone order with that scheme and you'd need to redo how the movable zone is created.
There are probably other consequences too that I haven't thought of yet. Summary - this would not be a trivial way fixing anything.
> However, this would return ZONE_MOVABLE if only GFP_MOVABLE would be > set but not GFP_HIGHMEM. > > If we could make sure that GFP_MOVABLE always includes GFP_HIGHMEM then > this would not be a problem. >
But it wouldn't be right either. It's ok to specify __GFP_MOVABLE without specifying __GFP_HIGHMEM. Quick grep shows it's not amazingly common but it's allowed.
-- Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab
| |