Re: [PATCH 21/25] Use allocation flags as an index to the zone watermark

KOSAKI Motohiro
2009-04-22T00:25:50Z

> ALLOC_WMARK_MIN, ALLOC_WMARK_LOW and ALLOC_WMARK_HIGH determin whether
> pages_min, pages_low or pages_high is used as the zone watermark when
> allocating the pages. Two branches in the allocator hotpath determine which
> watermark to use. This patch uses the flags as an array index and places
> the three watermarks in a union with an array so it can be offset. This
> means the flags can be used as an array index and reduces the branches
> taken.
>
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> Reviewed-by: Christoph Lameter <cl@linux-foundation.org>
> ---
> include/linux/mmzone.h | 8 +++++++-
> mm/page_alloc.c | 18 ++++++++----------
> 2 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index f82bdba..c1fa208 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -275,7 +275,13 @@ struct zone_reclaim_stat {
>
> struct zone {
> /* Fields commonly accessed by the page allocator */
> - unsigned long pages_min, pages_low, pages_high;
> + union {
> + struct {
> + unsigned long pages_min, pages_low, pages_high;
> + };
> + unsigned long pages_mark[3];
> + };
> +

hmmm... I don't like union hack.
Why can't we change all caller to use page_mark?

> /*
> * We don't know if the memory that we're going to allocate will be freeable
> * or/and it will be released eventually, so to avoid totally wasting several
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 376d848..e61867e 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1157,10 +1157,13 @@ failed:
> return NULL;
> }
>
> -#define ALLOC_NO_WATERMARKS 0x01 /* don't check watermarks at all */
> -#define ALLOC_WMARK_MIN 0x02 /* use pages_min watermark */
> -#define ALLOC_WMARK_LOW 0x04 /* use pages_low watermark */
> -#define ALLOC_WMARK_HIGH 0x08 /* use pages_high watermark */
> +/* The WMARK bits are used as an index zone->pages_mark */
> +#define ALLOC_WMARK_MIN 0x00 /* use pages_min watermark */
> +#define ALLOC_WMARK_LOW 0x01 /* use pages_low watermark */
> +#define ALLOC_WMARK_HIGH 0x02 /* use pages_high watermark */
> +#define ALLOC_NO_WATERMARKS 0x08 /* don't check watermarks at all */
> +#define ALLOC_WMARK_MASK 0x07 /* Mask to get the watermark bits */

the mask only use two bit. but mask definition is three bit (0x07), why?

> +
> #define ALLOC_HARDER 0x10 /* try to alloc harder */
> #define ALLOC_HIGH 0x20 /* __GFP_HIGH set */
> #ifdef CONFIG_CPUSETS
> @@ -1463,12 +1466,7 @@ zonelist_scan:
>
> if (!(alloc_flags & ALLOC_NO_WATERMARKS)) {
> unsigned long mark;
> - if (alloc_flags & ALLOC_WMARK_MIN)
> - mark = zone->pages_min;
> - else if (alloc_flags & ALLOC_WMARK_LOW)
> - mark = zone->pages_low;
> - else
> - mark = zone->pages_high;
> + mark = zone->pages_mark[alloc_flags & ALLOC_WMARK_MASK];
> if (!zone_watermark_ok(zone, order, mark,
> classzone_idx, alloc_flags)) {
> if (!zone_reclaim_mode ||
> --
> 1.5.6.5