lkml.org 
[lkml]   [2015]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 04/10] mm, page_alloc: Use masks and shifts when converting GFP flags to migrate types
On Mon, Sep 21, 2015 at 11:52:36AM +0100, Mel Gorman wrote:
> @@ -14,7 +14,7 @@ struct vm_area_struct;
> #define ___GFP_HIGHMEM 0x02u
> #define ___GFP_DMA32 0x04u
> #define ___GFP_MOVABLE 0x08u
> -#define ___GFP_WAIT 0x10u
> +#define ___GFP_RECLAIMABLE 0x10u
> #define ___GFP_HIGH 0x20u
> #define ___GFP_IO 0x40u
> #define ___GFP_FS 0x80u
> @@ -29,7 +29,7 @@ struct vm_area_struct;
> #define ___GFP_NOMEMALLOC 0x10000u
> #define ___GFP_HARDWALL 0x20000u
> #define ___GFP_THISNODE 0x40000u
> -#define ___GFP_RECLAIMABLE 0x80000u
> +#define ___GFP_WAIT 0x80000u
> #define ___GFP_NOACCOUNT 0x100000u
> #define ___GFP_NOTRACK 0x200000u
> #define ___GFP_NO_KSWAPD 0x400000u
> @@ -126,6 +126,7 @@ struct vm_area_struct;
>
> /* This mask makes up all the page movable related flags */
> #define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
> +#define GFP_MOVABLE_SHIFT 3

This connects the power-of-two gfp bits to the linear migrate type
enum, so shifting back and forth between them works only with up to
two items. A hypothetical ___GFP_FOOABLE would translate to 4, not
3. I'm not expecting new migratetypes to show up anytime soon, but
this implication does not make the code exactly robust and obvious.

> @@ -152,14 +153,15 @@ struct vm_area_struct;
> /* Convert GFP flags to their corresponding migrate type */
> static inline int gfpflags_to_migratetype(const gfp_t gfp_flags)
> {
> - WARN_ON((gfp_flags & GFP_MOVABLE_MASK) == GFP_MOVABLE_MASK);
> + VM_WARN_ON((gfp_flags & GFP_MOVABLE_MASK) == GFP_MOVABLE_MASK);
> + BUILD_BUG_ON((1UL << GFP_MOVABLE_SHIFT) != ___GFP_MOVABLE);
> + BUILD_BUG_ON((___GFP_MOVABLE >> GFP_MOVABLE_SHIFT) != MIGRATE_MOVABLE);
>
> if (unlikely(page_group_by_mobility_disabled))
> return MIGRATE_UNMOVABLE;
>
> /* Group based on mobility */
> - return (((gfp_flags & __GFP_MOVABLE) != 0) << 1) |
> - ((gfp_flags & __GFP_RECLAIMABLE) != 0);
> + return (gfp_flags & GFP_MOVABLE_MASK) >> GFP_MOVABLE_SHIFT;

I'm not sure the simplification of this line is worth the fragile
dependency between those two tables.


\
 
 \ /
  Last update: 2015-09-24 23:01    [W:1.762 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site