Re: Page allocator: Single Zone optimizations

Nick Piggin
2006-10-18T11:15:35Z

Christoph Lameter wrote:
> On Tue, 17 Oct 2006, Nick Piggin wrote:
>
>
>>I would give an ack to Kame's approach for lowmem_reserve ;)
>
>

> @@ -922,8 +924,7 @@ int zone_watermark_ok(struct zone *z, in
> min -= min / 2;
> if (alloc_flags & ALLOC_HARDER)
> min -= min / 4;
> -
> - if (free_pages <= min + z->lowmem_reserve[classzone_idx])
> + if (free_pages <= min + MULTI_ZONE * z->lowmem_reserve[classzone_idx])

I think I still prefer having this in a header. It is the normal way of doing it.

> return 0;
> for (o = 0; o < order; o++) {
> /* At the next order, this order's pages become unavailable */
> @@ -1429,8 +1430,6 @@ void show_free_areas(void)
> global_page_state(NR_PAGETABLE));
>
> for_each_zone(zone) {
> - int i;
> -
> if (!populated_zone(zone))
> continue;
>
> @@ -1457,10 +1456,14 @@ void show_free_areas(void)
> zone->pages_scanned,
> (zone->all_unreclaimable ? "yes" : "no")
> );
> - printk("lowmem_reserve[]:");
> - for (i = 0; i < MAX_NR_ZONES; i++)
> - printk(" %lu", zone->lowmem_reserve[i]);
> - printk("\n");
> + if (MULTI_ZONE) {
> + int i;
> +
> + printk("lowmem_reserve[]:");
> + for (i = 0; i < MAX_NR_ZONES; i++)
> + printk(" %lu", zone->lowmem_reserve[i]);
> + printk("\n");
> + }
> }
>
> for_each_zone(zone) {
> @@ -2829,25 +2832,36 @@ void __init page_alloc_init(void)
> * calculate_totalreserve_pages - called when sysctl_lower_zone_reserve_ratio
> * or min_free_kbytes changes.
> */
> +static unsigned long calculate_max_lowmem_reserve(struct zone *zone,
> + enum zone_type start)
> +{
> + unsigned long max;
> + enum zone_type i;
> +
> + if (SINGLE_ZONE)
> + return 0;
> + /* Find valid and maximum lowmem_reserve in the zone */
> + for (i = start; i < MAX_NR_ZONES; i++) {
> + if (zone->lowmem_reserve[i] > max)
> + max = zone->lowmem_reserve[i];
> + }
> + return max;
> +}
> +
> static void calculate_totalreserve_pages(void)
> {
> struct pglist_data *pgdat;
> unsigned long reserve_pages = 0;
> - enum zone_type i, j;
> + enum zone_type i;
>
> for_each_online_pgdat(pgdat) {
> for (i = 0; i < MAX_NR_ZONES; i++) {
> struct zone *zone = pgdat->node_zones + i;
> - unsigned long max = 0;
> -
> - /* Find valid and maximum lowmem_reserve in the zone */
> - for (j = i; j < MAX_NR_ZONES; j++) {
> - if (zone->lowmem_reserve[j] > max)
> - max = zone->lowmem_reserve[j];
> - }
> + unsigned long max;
>
> /* we treat pages_high as reserved pages. */
> - max += zone->pages_high;
> + max = calculate_max_lowmem_reserve(zone, i) + \
> + zone->pages_high;
>
> if (max > zone->present_pages)
> max = zone->present_pages;
> @@ -2868,6 +2882,9 @@ static void setup_per_zone_lowmem_reserv
> struct pglist_data *pgdat;
> enum zone_type j, idx;
>
> + if (SINGLE_ZONE)
> + return 0;
> +
> for_each_online_pgdat(pgdat) {
> for (j = 0; j < MAX_NR_ZONES; j++) {
> struct zone *zone = pgdat->node_zones + j;
> @@ -3044,6 +3061,7 @@ int sysctl_min_slab_ratio_sysctl_handler
> }
> #endif
>
> +#if MULTI_ZONE
> /*
> * lowmem_reserve_ratio_sysctl_handler - just a wrapper around
> * proc_dointvec() so that we can call setup_per_zone_lowmem_reserve()
> @@ -3060,6 +3078,7 @@ int lowmem_reserve_ratio_sysctl_handler(
> setup_per_zone_lowmem_reserve();
> return 0;
> }
> +#endif
>
> /*
> * percpu_pagelist_fraction - changes the pcp->high for each zone on each
> Index: linux-2.6.19-rc1-mm1/kernel/sysctl.c
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/kernel/sysctl.c 2006-10-17 07:27:44.692269445 -0500
> +++ linux-2.6.19-rc1-mm1/kernel/sysctl.c 2006-10-17 07:38:47.977425889 -0500
> @@ -900,6 +900,7 @@ static ctl_table vm_table[] = {
> .proc_handler = &proc_dointvec,
> },
> #endif
> +#if MULTI_ZONE
> {
> .ctl_name = VM_LOWMEM_RESERVE_RATIO,
> .procname = "lowmem_reserve_ratio",
> @@ -909,6 +910,7 @@ static ctl_table vm_table[] = {
> .proc_handler = &lowmem_reserve_ratio_sysctl_handler,
> .strategy = &sysctl_intvec,
> },
> +#endif
> {
> .ctl_name = VM_DROP_PAGECACHE,
> .procname = "drop_caches",
> Index: linux-2.6.19-rc1-mm1/include/linux/mmzone.h
> ===================================================================
> --- linux-2.6.19-rc1-mm1.orig/include/linux/mmzone.h 2006-10-17 07:27:42.478206116 -0500
> +++ linux-2.6.19-rc1-mm1/include/linux/mmzone.h 2006-10-17 07:34:40.134279959 -0500
> @@ -171,6 +171,9 @@ enum zone_type {
> #endif
> #undef __ZONE_COUNT
>
> +#define MULTI_ZONE (ZONES_SHIFT > 0)
> +#define SINGLE_ZONE (ZONES_SHIFT == 0)
> +
> struct zone {
> /* Fields commonly accessed by the page allocator */
> unsigned long free_pages;
> @@ -183,7 +186,7 @@ struct zone {
> * on the higher zones). This array is recalculated at runtime if the
> * sysctl_lowmem_reserve_ratio sysctl changes.
> */
> - unsigned long lowmem_reserve[MAX_NR_ZONES];
> + unsigned long lowmem_reserve[MAX_NR_ZONES - SINGLE_ZONE];

IMO ifdef is much better here. It is more readable and gives better
typechecking. I think ifdef in structures is not much of a problem.

>
> #ifdef CONFIG_NUMA
> int node;
> @@ -420,11 +423,11 @@ unsigned long __init node_memmap_size_by
> /*
> * zone_idx() returns 0 for the ZONE_DMA zone, 1 for the ZONE_NORMAL zone, etc.
> */
> -#define zone_idx(zone) ((zone) - (zone)->zone_pgdat->node_zones)
> +#define zone_idx(zone) (MULTI_ZONE * ((zone) - (zone)->zone_pgdat->node_zones))
>
> static inline int populated_zone(struct zone *zone)
> {
> - return (!!zone->present_pages);
> + return SINGLE_ZONE || (!!zone->present_pages);
> }
>
> static inline int is_highmem_idx(enum zone_type idx)
> @@ -438,7 +441,7 @@ static inline int is_highmem_idx(enum zo
>
> static inline int is_normal_idx(enum zone_type idx)
> {
> - return (idx == ZONE_NORMAL);
> + return SINGLE_ZONE || (idx == ZONE_NORMAL);
> }
>
> /**
> @@ -458,7 +461,8 @@ static inline int is_highmem(struct zone
>
> static inline int is_normal(struct zone *zone)
> {
> - return zone == zone->zone_pgdat->node_zones + ZONE_NORMAL;
> + return SINGLE_ZONE ||
> + zone == zone->zone_pgdat->node_zones + ZONE_NORMAL;
> }

I don't know if these are any better than ifdef elseif endif. I think
the goal is not ifdef removal at any cost, but avoiding ifdefs in
complex functions and within control flow because it makes the code
less readable.

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com