Skip to contentSkip to conversation

Use one zonelist per node instead of multiple zonelists v9

PATCH In review Mel Gorman 7 participants 34 messages Nov 9, 2007 – Mar 7, 2008

Subsystems (15)
lore
Share & tools
lore

Mel Gorman

author
#0 lore raw

cover Use one zonelist per node instead of multiple zonelists v9

This is basically a rebase to the broken-out -mm tree. Since v8, two fixes have been applied that showed up during testing. Most machines I test -mm on are failing to boot for a variety of reasons but on the two machines that did work, they appeared to work fine.

Changelog since V8
o Rebase to 2.6.24-rc2
o Added ack for the OOM changes
o Behave correctly when GFP_THISNODE and a node ID are specified
o Clear up warning over type of nodes_intersects() function

Changelog since V7
o Rebase to 2.6.23-rc8-mm2

Changelog since V6
o Fix build bug in relation to memory controller combined with one-zonelist
o Use while() instead of a stupid looking for()
o Instead of encoding zone index information in a pointer, this version introduces a structure that stores a zone pointer and its index

Changelog since V5
o Rebase to 2.6.23-rc4-mm1
o Drop patch that replaces inline functions with macros

Changelog since V4
o Rebase to -mm kernel. Host of memoryless patches collisions dealt with
o Do not call wakeup_kswapd() for every zone in a zonelist
o Dropped the FASTCALL removal
o Have cursor in iterator advance earlier
o Use nodes_and in cpuset_nodes_valid_mems_allowed()
o Use defines instead of inlines, noticably better performance on gcc-3.4 No difference on later compilers such as gcc 4.1
o Dropped gfp_skip patch until it is proven to be of benefit. Tests are currently inconclusive but it definitly consumes at least one cache line

Changelog since V3
o Fix compile error in the parisc change
o Calculate gfp_zone only once in __alloc_pages
o Calculate classzone_idx properly in get_page_from_freelist
o Alter check so that zone id embedded may still be used on UP
o Use Kamezawa-sans suggestion for skipping zones in zonelist
o Add __alloc_pages_nodemask() to filter zonelist based on a nodemask. This removes the need for MPOL_BIND to have a custom zonelist
o Move zonelist iterators and helpers to mm.h
o Change _zones from struct zone * to unsigned long

Changelog since V2
o shrink_zones() uses zonelist instead of zonelist->zones
o hugetlb uses zonelist iterator
o zone_idx information is embedded in zonelist pointers
o replace NODE_DATA(nid)->node_zonelist with node_zonelist(nid)

Changelog since V1
o Break up the patch into 3 patches
o Introduce iterators for zonelists
o Performance regression test

The following patches replace multiple zonelists per node with one zonelist that is filtered based on the GFP flags. The patches as a set fix a bug with regard to the use of MPOL_BIND and ZONE_MOVABLE. With this patchset, the MPOL_BIND will apply to the two highest zones when the highest zone is ZONE_MOVABLE. This should be considered as an alternative fix for the MPOL_BIND+ZONE_MOVABLE in 2.6.23 to the previously discussed hack that filters only custom zonelists. As a bonus, the patchset reduces the cache footprint of the kernel and should improve performance in a number of cases.

The first patch cleans up an inconsitency where direct reclaim uses zonelist->zones where other places use zonelist. The second patch introduces a helper function node_zonelist() for looking up the appropriate zonelist for a GFP mask which simplifies patches later in the set.

The third patch replaces multiple zonelists with two zonelists that are filtered. The two zonelists are due to the fact that the memoryless patchset introduces a second set of zonelists for __GFP_THISNODE.

The fourth patch introduces helper macros for retrieving the zone and node indices of entries in a zonelist.

The fifth patch introduces filtering of the zonelists based on a nodemask.

The final patch replaces the two zonelists with one zonelist. A nodemask is created when __GFP_THISNODE is specified to filter the list. The nodelists could be pre-allocated with one-per-node but it's not clear that __GFP_THISNODE is used often enough to be worth the effort.

Performance results varied depending on the machine configuration but were usually small performance gains. In real workloads the gain/loss will depend on how much the userspace portion of the benchmark benefits from having more cache available due to reduced referencing of zonelists.

These are the range of performance losses/gains when running against 2.6.23-rc3-mm1. The set and these machines are a mix of i386, x86_64 and ppc64 both NUMA and non-NUMA.

Total CPU time on Kernbench: -0.67% to  3.05%
Elapsed   time on Kernbench: -0.25% to  2.96%
page_test from aim9:         -6.98% to  5.60%
brk_test  from aim9:         -3.94% to  4.11%
fork_test from aim9:         -5.72% to  4.14%
exec_test from aim9:         -1.02% to  1.56%

The TBench figures were too variable between runs to draw conclusions from but there didn't appear to be any regressions there. The hackbench results for both sockets and pipes were within noise.

Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#1 lore raw

PATCH 1/6 Use zonelists instead of zones when direct reclaiming pages

The allocator deals with zonelists which indicate the order in which zones should be targeted for an allocation. Similarly, direct reclaim of pages iterates over an array of zones. For consistency, this patch converts direct reclaim to use a zonelist. No functionality is changed by this patch. This simplifies zonelist iterators in the next patch.

---

 include/linux/swap.h |    2 +-
 mm/page_alloc.c      |    2 +-
 mm/vmscan.c          |   21 ++++++++++++---------
 3 files changed, 14 insertions(+), 11 deletions(-)
include/linux/swap.h +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-b1106/include/linux/swap.h linux-2.6.24-rc1-mm-005_freepages_zonelist/include/linux/swap.h
@@ -181,7 +181,7 @@ extern int rotate_reclaimable_page(struc
 extern void swap_setup(void);
 
 /* linux/mm/vmscan.c */
-extern unsigned long try_to_free_pages(struct zone **zones, int order,
+extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
 					gfp_t gfp_mask);
 extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,
 							gfp_t gfp_mask);
mm/page_alloc.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-b1106/mm/page_alloc.c linux-2.6.24-rc1-mm-005_freepages_zonelist/mm/page_alloc.c
@@ -1647,7 +1647,7 @@ nofail_alloc:
 	reclaim_state.reclaimed_slab = 0;
 	p->reclaim_state = &reclaim_state;
 
-	did_some_progress = try_to_free_pages(zonelist->zones, order, gfp_mask);
+	did_some_progress = try_to_free_pages(zonelist, order, gfp_mask);
 
 	p->reclaim_state = NULL;
 	p->flags &= ~PF_MEMALLOC;
mm/vmscan.c +12 -9
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-b1106/mm/vmscan.c linux-2.6.24-rc1-mm-005_freepages_zonelist/mm/vmscan.c
@@ -1216,10 +1216,11 @@ static unsigned long shrink_zone(int pri
  * If a zone is deemed to be full of pinned pages then just give it a light
  * scan then give up on it.
  */
-static unsigned long shrink_zones(int priority, struct zone **zones,
+static unsigned long shrink_zones(int priority, struct zonelist *zonelist,
 					struct scan_control *sc)
 {
 	unsigned long nr_reclaimed = 0;
+	struct zone **zones = zonelist->zones;
 	int i;
 
 	sc->all_unreclaimable = 1;
@@ -1257,8 +1258,8 @@ static unsigned long shrink_zones(int pr
  * holds filesystem locks which prevent writeout this might not work, and the
  * allocation attempt will fail.
  */
-static unsigned long do_try_to_free_pages(struct zone **zones, gfp_t gfp_mask,
-					  struct scan_control *sc)
+static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
+					gfp_t gfp_mask, struct scan_control *sc)
 {
 	int priority;
 	int ret = 0;
@@ -1266,6 +1267,7 @@ static unsigned long do_try_to_free_page
 	unsigned long nr_reclaimed = 0;
 	struct reclaim_state *reclaim_state = current->reclaim_state;
 	unsigned long lru_pages = 0;
+	struct zone **zones = zonelist->zones;
 	int i;
 
 	count_vm_event(ALLOCSTALL);
@@ -1285,7 +1287,7 @@ static unsigned long do_try_to_free_page
 		sc->nr_io_pages = 0;
 		if (!priority)
 			disable_swap_token();
-		nr_reclaimed += shrink_zones(priority, zones, sc);
+		nr_reclaimed += shrink_zones(priority, zonelist, sc);
 		/*
 		 * Don't shrink slabs when reclaiming memory from
 		 * over limit cgroups
@@ -1344,7 +1346,8 @@ out:
 	return ret;
 }
 
-unsigned long try_to_free_pages(struct zone **zones, int order, gfp_t gfp_mask)
+unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
+								gfp_t gfp_mask)
 {
 	struct scan_control sc = {
 		.gfp_mask = gfp_mask,
@@ -1357,7 +1360,7 @@ unsigned long try_to_free_pages(struct z
 		.isolate_pages = isolate_pages_global,
 	};
 
-	return do_try_to_free_pages(zones, gfp_mask, &sc);
+	return do_try_to_free_pages(zonelist, gfp_mask, &sc);
 }
 
 #ifdef CONFIG_CGROUP_MEM_CONT
@@ -1376,11 +1379,11 @@ unsigned long try_to_free_mem_cgroup_pag
 		.isolate_pages = mem_cgroup_isolate_pages,
 	};
 	int node = numa_node_id();
-	struct zone **zones;
+	struct zonelist *zonelist;
 	int target_zone = gfp_zone(GFP_HIGHUSER_MOVABLE);
 
-	zones = NODE_DATA(node)->node_zonelists[target_zone].zones;
-	if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+	zonelist = &NODE_DATA(node)->node_zonelists[target_zone];
+	if (do_try_to_free_pages(zonelist, sc.gfp_mask, &sc))
 		return 1;
 	return 0;
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#2 lore raw

PATCH 2/6 Introduce node_zonelist() for accessing the zonelist for a GFP mask

In reply to Mel Gorman

This patch introduces a node_zonelist() helper function. It is used to lookup the appropriate zonelist given a node and a GFP mask. The patch on its own is a cleanup but it helps clarify parts of the one-zonelist-per-node patchset. If necessary, it can be merged with the next patch in this set without problems.

---

 drivers/char/sysrq.c      |    3 +--
 fs/buffer.c               |    6 +++---
 include/linux/gfp.h       |   19 +++++++++++++++++--
 include/linux/mempolicy.h |    2 +-
 mm/mempolicy.c            |    6 +++---
 mm/page_alloc.c           |    3 +--
 mm/slab.c                 |    3 +--
 mm/slub.c                 |    3 +--
 8 files changed, 28 insertions(+), 17 deletions(-)
drivers/char/sysrq.c +1 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/drivers/char/sysrq.c linux-2.6.24-rc1-mm-007_node_zonelist/drivers/char/sysrq.c
@@ -271,8 +271,7 @@ static struct sysrq_key_op sysrq_term_op
 
 static void moom_callback(struct work_struct *ignored)
 {
-	out_of_memory(&NODE_DATA(0)->node_zonelists[ZONE_NORMAL],
-			GFP_KERNEL, 0);
+	out_of_memory(node_zonelist(0, GFP_KERNEL), GFP_KERNEL, 0);
 }
 
 static DECLARE_WORK(moom_work, moom_callback);
fs/buffer.c +3 -3
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/fs/buffer.c linux-2.6.24-rc1-mm-007_node_zonelist/fs/buffer.c
@@ -369,13 +369,13 @@ void invalidate_bdev(struct block_device
 static void free_more_memory(void)
 {
 	struct zone **zones;
-	pg_data_t *pgdat;
+	int nid;
 
 	wakeup_pdflush(1024);
 	yield();
 
-	for_each_online_pgdat(pgdat) {
-		zones = pgdat->node_zonelists[gfp_zone(GFP_NOFS)].zones;
+	for_each_online_node(nid) {
+		zones = node_zonelist(nid, GFP_NOFS);
 		if (*zones)
 			try_to_free_pages(zones, 0, GFP_NOFS);
 	}
include/linux/gfp.h +17 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/include/linux/gfp.h linux-2.6.24-rc1-mm-007_node_zonelist/include/linux/gfp.h
@@ -157,13 +157,29 @@ static inline gfp_t set_migrateflags(gfp
  * virtual kernel addresses to the allocated page(s).
  */
 
+static inline enum zone_type gfp_zonelist(gfp_t flags)
+{
+	int base = 0;
+
+	if (NUMA_BUILD && (flags & __GFP_THISNODE))
+		base = 1;
+
+	return base;
+}
+
 /*
  * We get the zone list from the current node and the gfp_mask.
  * This zone list contains a maximum of MAXNODES*MAX_NR_ZONES zones.
+ * There are two zonelists per node, one for all zones with memory and
+ * one containing just zones from the node the zonelist belongs to.
  *
  * For the normal case of non-DISCONTIGMEM systems the NODE_DATA() gets
  * optimized to &contig_page_data at compile-time.
  */
+static inline struct zonelist *node_zonelist(int nid, gfp_t flags)
+{
+	return NODE_DATA(nid)->node_zonelists + gfp_zonelist(flags);
+}
 
 #ifndef HAVE_ARCH_FREE_PAGE
 static inline void arch_free_page(struct page *page, int order) { }
@@ -185,8 +201,7 @@ static inline struct page *alloc_pages_n
 	if (nid < 0)
 		nid = numa_node_id();
 
-	return __alloc_pages(gfp_mask, order,
-		NODE_DATA(nid)->node_zonelists + gfp_zone(gfp_mask));
+	return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
 }
 
 #ifdef CONFIG_NUMA
include/linux/mempolicy.h +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/include/linux/mempolicy.h linux-2.6.24-rc1-mm-007_node_zonelist/include/linux/mempolicy.h
@@ -241,7 +241,7 @@ static inline void mpol_fix_fork_child_f
 static inline struct zonelist *huge_zonelist(struct vm_area_struct *vma,
  		unsigned long addr, gfp_t gfp_flags, struct mempolicy **mpol)
 {
-	return NODE_DATA(0)->node_zonelists + gfp_zone(gfp_flags);
+	return node_zonelist(0, gfp_flags);
 }
 
 static inline int do_migrate_pages(struct mm_struct *mm,
mm/mempolicy.c +3 -3
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/mm/mempolicy.c linux-2.6.24-rc1-mm-007_node_zonelist/mm/mempolicy.c
@@ -1172,7 +1172,7 @@ static struct zonelist *zonelist_policy(
 		nd = 0;
 		BUG();
 	}
-	return NODE_DATA(nd)->node_zonelists + gfp_zone(gfp);
+	return node_zonelist(nd, gfp);
 }
 
 /* Do dynamic interleaving for a process */
@@ -1286,7 +1286,7 @@ struct zonelist *huge_zonelist(struct vm
 
 		nid = interleave_nid(pol, vma, addr, HPAGE_SHIFT);
 		__mpol_free(pol);		/* finished with pol */
-		return NODE_DATA(nid)->node_zonelists + gfp_zone(gfp_flags);
+		return node_zonelist(nid, gfp_flags);
 	}
 
 	zl = zonelist_policy(GFP_HIGHUSER, pol);
@@ -1308,7 +1308,7 @@ static struct page *alloc_page_interleav
 	struct zonelist *zl;
 	struct page *page;
 
-	zl = NODE_DATA(nid)->node_zonelists + gfp_zone(gfp);
+	zl = node_zonelist(nid, gfp);
 	page = __alloc_pages(gfp, order, zl);
 	if (page && page_zone(page) == zl->zones[0])
 		inc_zone_page_state(page, NUMA_INTERLEAVE_HIT);
mm/page_alloc.c +1 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/mm/page_alloc.c linux-2.6.24-rc1-mm-007_node_zonelist/mm/page_alloc.c
@@ -1795,10 +1795,9 @@ EXPORT_SYMBOL(free_pages);
 static unsigned int nr_free_zone_pages(int offset)
 {
 	/* Just pick one node, since fallback list is circular */
-	pg_data_t *pgdat = NODE_DATA(numa_node_id());
 	unsigned int sum = 0;
 
-	struct zonelist *zonelist = pgdat->node_zonelists + offset;
+	struct zonelist *zonelist = node_zonelist(numa_node_id(), GFP_KERNEL);
 	struct zone **zonep = zonelist->zones;
 	struct zone *zone;
 
mm/slab.c +1 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/mm/slab.c linux-2.6.24-rc1-mm-007_node_zonelist/mm/slab.c
@@ -3246,8 +3246,7 @@ static void *fallback_alloc(struct kmem_
 	if (flags & __GFP_THISNODE)
 		return NULL;
 
-	zonelist = &NODE_DATA(slab_node(current->mempolicy))
-			->node_zonelists[gfp_zone(flags)];
+	zonelist = node_zonelist(slab_node(current->mempolicy), flags);
 	local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
 
 retry:
mm/slub.c +1 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-005_freepages_zonelist/mm/slub.c linux-2.6.24-rc1-mm-007_node_zonelist/mm/slub.c
@@ -1349,8 +1349,7 @@ static unsigned long get_any_partial(str
 			get_cycles() % 1024 > s->remote_node_defrag_ratio)
 		return 0;
 
-	zonelist = &NODE_DATA(slab_node(current->mempolicy))
-					->node_zonelists[gfp_zone(flags)];
+	zonelist = node_zonelist(slab_node(current->mempolicy), flags);
 	for (z = zonelist->zones; *z; z++) {
 		struct kmem_cache_node *n;
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#4 lore raw

PATCH 3/6 Use two zonelist that are filtered by GFP mask

In reply to Mel Gorman

Currently a node has a number of zonelists, one for each zone type in the system and a second set for GFP_THISNODE allocations. Based on the zones allowed by a gfp mask, one of these zonelists is selected. All of these zonelists consume memory and occupy cache lines.

This patch replaces the multiple zonelists per-node with two zonelists. The first contains all populated zones in the system and the second contains all populated zones in node suitable for GFP_THISNODE allocations. An iterator macro is introduced called for_each_zone_zonelist() interates through each zone in the zonelist that is allowed by the GFP flags.

---

 arch/parisc/mm/init.c  |   11 +-
 fs/buffer.c            |    6 +
 include/linux/gfp.h    |   17 +---
 include/linux/mmzone.h |   65 +++++++++++-----
 mm/hugetlb.c           |    8 +-
 mm/oom_kill.c          |    8 +-
 mm/page_alloc.c        |  169 +++++++++++++++++++-------------------------
 mm/slab.c              |    8 +-
 mm/slub.c              |    8 +-
 mm/vmscan.c            |   20 ++---
 10 files changed, 159 insertions(+), 161 deletions(-)
arch/parisc/mm/init.c +7 -4
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/arch/parisc/mm/init.c linux-2.6.24-rc1-mm-010_use_two_zonelists/arch/parisc/mm/init.c
@@ -603,15 +603,18 @@ void show_mem(void)
 #ifdef CONFIG_DISCONTIGMEM
 	{
 		struct zonelist *zl;
-		int i, j, k;
+		int i, j;
 
 		for (i = 0; i < npmem_ranges; i++) {
+			zl = node_zonelist(i);
 			for (j = 0; j < MAX_NR_ZONES; j++) {
-				zl = NODE_DATA(i)->node_zonelists + j;
+				struct zone **z;
+				struct zone *zone;
 
 				printk("Zone list for zone %d on node %d: ", j, i);
-				for (k = 0; zl->zones[k] != NULL; k++) 
-					printk("[%d/%s] ", zone_to_nid(zl->zones[k]), zl->zones[k]->name);
+				for_each_zone_zonelist(zone, z, zl, j)
+					printk("[%d/%s] ", zone_to_nid(zone),
+								zone->name);
 				printk("\n");
 			}
 		}
fs/buffer.c +4 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/fs/buffer.c linux-2.6.24-rc1-mm-010_use_two_zonelists/fs/buffer.c
@@ -375,9 +375,11 @@ static void free_more_memory(void)
 	yield();
 
 	for_each_online_node(nid) {
-		zones = node_zonelist(nid, GFP_NOFS);
+		zones = first_zones_zonelist(node_zonelist(nid, GFP_NOFS),
+						gfp_zone(GFP_NOFS));
 		if (*zones)
-			try_to_free_pages(zones, 0, GFP_NOFS);
+			try_to_free_pages(node_zonelist(nid, GFP_NOFS), 0,
+						GFP_NOFS);
 	}
 }
 
include/linux/gfp.h +5 -12
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/include/linux/gfp.h linux-2.6.24-rc1-mm-010_use_two_zonelists/include/linux/gfp.h
@@ -119,29 +119,22 @@ static inline int allocflags_to_migratet
 
 static inline enum zone_type gfp_zone(gfp_t flags)
 {
-	int base = 0;
-
-#ifdef CONFIG_NUMA
-	if (flags & __GFP_THISNODE)
-		base = MAX_NR_ZONES;
-#endif
-
 #ifdef CONFIG_ZONE_DMA
 	if (flags & __GFP_DMA)
-		return base + ZONE_DMA;
+		return ZONE_DMA;
 #endif
 #ifdef CONFIG_ZONE_DMA32
 	if (flags & __GFP_DMA32)
-		return base + ZONE_DMA32;
+		return ZONE_DMA32;
 #endif
 	if ((flags & (__GFP_HIGHMEM | __GFP_MOVABLE)) ==
 			(__GFP_HIGHMEM | __GFP_MOVABLE))
-		return base + ZONE_MOVABLE;
+		return ZONE_MOVABLE;
 #ifdef CONFIG_HIGHMEM
 	if (flags & __GFP_HIGHMEM)
-		return base + ZONE_HIGHMEM;
+		return ZONE_HIGHMEM;
 #endif
-	return base + ZONE_NORMAL;
+	return ZONE_NORMAL;
 }
 
 static inline gfp_t set_migrateflags(gfp_t gfp, gfp_t migrate_flags)
include/linux/mmzone.h +43 -22

90 lines not on this page · raw message

mm/hugetlb.c +4 -4
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/mm/hugetlb.c linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/hugetlb.c
@@ -78,11 +78,11 @@ static struct page *dequeue_huge_page(st
 	struct mempolicy *mpol;
 	struct zonelist *zonelist = huge_zonelist(vma, address,
 					htlb_alloc_mask, &mpol);
-	struct zone **z;
+	struct zone *zone, **z;
 
-	for (z = zonelist->zones; *z; z++) {
-		nid = zone_to_nid(*z);
-		if (cpuset_zone_allowed_softwall(*z, htlb_alloc_mask) &&
+	for_each_zone_zonelist(zone, z, zonelist, MAX_NR_ZONES - 1) {
+		nid = zone_to_nid(zone);
+		if (cpuset_zone_allowed_softwall(zone, htlb_alloc_mask) &&
 		    !list_empty(&hugepage_freelists[nid])) {
 			page = list_entry(hugepage_freelists[nid].next,
 					  struct page, lru);
mm/oom_kill.c +5 -3
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/mm/oom_kill.c linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/oom_kill.c
@@ -175,12 +175,14 @@ static inline enum oom_constraint constr
 						    gfp_t gfp_mask)
 {
 #ifdef CONFIG_NUMA
+	struct zone *zone;
 	struct zone **z;
+	enum zone_type high_zoneidx = gfp_zone(gfp_mask);
 	nodemask_t nodes = node_states[N_HIGH_MEMORY];
 
-	for (z = zonelist->zones; *z; z++)
-		if (cpuset_zone_allowed_softwall(*z, gfp_mask))
-			node_clear(zone_to_nid(*z), nodes);
+	for_each_zone_zonelist(zone, z, zonelist, high_zoneidx)
+		if (cpuset_zone_allowed_softwall(zone, gfp_mask))
+			node_clear(zone_to_nid(zone), nodes);
 		else
 			return CONSTRAINT_CPUSET;
 
mm/page_alloc.c +73 -96

299 lines not on this page · raw message

mm/slab.c +5 -3
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/mm/slab.c linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/slab.c
@@ -3240,6 +3240,8 @@ static void *fallback_alloc(struct kmem_
 	struct zonelist *zonelist;
 	gfp_t local_flags;
 	struct zone **z;
+	struct zone *zone;
+	enum zone_type high_zoneidx = gfp_zone(flags);
 	void *obj = NULL;
 	int nid;
 
@@ -3254,10 +3256,10 @@ retry:
 	 * Look through allowed nodes for objects available
 	 * from existing per node queues.
 	 */
-	for (z = zonelist->zones; *z && !obj; z++) {
-		nid = zone_to_nid(*z);
+	for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
+		nid = zone_to_nid(zone);
 
-		if (cpuset_zone_allowed_hardwall(*z, flags) &&
+		if (cpuset_zone_allowed_hardwall(zone, flags) &&
 			cache->nodelists[nid] &&
 			cache->nodelists[nid]->free_objects)
 				obj = ____cache_alloc_node(cache,
mm/slub.c +5 -3
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/mm/slub.c linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/slub.c
@@ -1325,6 +1325,8 @@ static unsigned long get_any_partial(str
 #ifdef CONFIG_NUMA
 	struct zonelist *zonelist;
 	struct zone **z;
+	struct zone *zone;
+	enum zone_type high_zoneidx = gfp_zone(flags);
 	unsigned long state;
 
 	/*
@@ -1350,12 +1352,12 @@ static unsigned long get_any_partial(str
 		return 0;
 
 	zonelist = node_zonelist(slab_node(current->mempolicy), flags);
-	for (z = zonelist->zones; *z; z++) {
+	for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
 		struct kmem_cache_node *n;
 
-		n = get_node(s, zone_to_nid(*z));
+		n = get_node(s, zone_to_nid(zone));
 
-		if (n && cpuset_zone_allowed_hardwall(*z, flags) &&
+		if (n && cpuset_zone_allowed_hardwall(zone, flags) &&
 				n->nr_partial > MIN_PARTIAL) {
 			state = get_partial_node(n, c);
 			if (state)
mm/vmscan.c +8 -12
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-007_node_zonelist/mm/vmscan.c linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/vmscan.c
@@ -1220,13 +1220,11 @@ static unsigned long shrink_zones(int pr
 					struct scan_control *sc)
 {
 	unsigned long nr_reclaimed = 0;
-	struct zone **zones = zonelist->zones;
-	int i;
+	struct zone **z;
+	struct zone *zone;
 
 	sc->all_unreclaimable = 1;
-	for (i = 0; zones[i] != NULL; i++) {
-		struct zone *zone = zones[i];
-
+	for_each_zone_zonelist(zone, z, zonelist, MAX_NR_ZONES - 1) {
 		if (!populated_zone(zone))
 			continue;
 
@@ -1267,14 +1265,13 @@ static unsigned long do_try_to_free_page
 	unsigned long nr_reclaimed = 0;
 	struct reclaim_state *reclaim_state = current->reclaim_state;
 	unsigned long lru_pages = 0;
-	struct zone **zones = zonelist->zones;
-	int i;
+	struct zone **z;
+	struct zone *zone;
+	enum zone_type high_zoneidx = gfp_zone(gfp_mask);
 
 	count_vm_event(ALLOCSTALL);
 
-	for (i = 0; zones[i] != NULL; i++) {
-		struct zone *zone = zones[i];
-
+	for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
 		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
 			continue;
 
@@ -1335,9 +1332,8 @@ out:
 	 */
 	if (priority < 0)
 		priority = 0;
-	for (i = 0; zones[i] != NULL; i++) {
-		struct zone *zone = zones[i];
 
+	for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
 		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
 			continue;
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#5 lore raw

PATCH 4/6 Have zonelist contains structs with both a zone pointer and zone_idx

In reply to Mel Gorman

Filtering zonelists requires very frequent use of zone_idx(). This is costly as it involves a lookup of another structure and a substraction operation. As the zone_idx is often required, it should be quickly accessible. The node idx could also be stored here if it was found that accessing zone->node is significant which may be the case on workloads where nodemasks are heavily used.

This patch introduces a struct zoneref to store a zone pointer and a zone index. The zonelist then consists of an array of this struct zonerefs which are looked up as necessary. Helpers are given for accessing the zone index as well as the node index.

[[email protected]: Suggested struct zoneref instead of embedding information in pointers]

---

 arch/parisc/mm/init.c  |    2 -
 fs/buffer.c            |    6 ++--
 include/linux/mmzone.h |   64 +++++++++++++++++++++++++++++++++++++-------
 include/linux/oom.h    |    4 +-
 kernel/cpuset.c        |    4 +-
 mm/hugetlb.c           |    3 +-
 mm/mempolicy.c         |   35 ++++++++++++++----------
 mm/oom_kill.c          |   45 +++++++++++++++---------------
 mm/page_alloc.c        |   59 ++++++++++++++++++++--------------------
 mm/slab.c              |    2 -
 mm/slub.c              |    2 -
 mm/vmscan.c            |    7 ++--
 mm/vmstat.c            |    5 ++-
 13 files changed, 145 insertions(+), 93 deletions(-)
arch/parisc/mm/init.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/arch/parisc/mm/init.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/arch/parisc/mm/init.c
@@ -608,7 +608,7 @@ void show_mem(void)
 		for (i = 0; i < npmem_ranges; i++) {
 			zl = node_zonelist(i);
 			for (j = 0; j < MAX_NR_ZONES; j++) {
-				struct zone **z;
+				struct zoneref *z;
 				struct zone *zone;
 
 				printk("Zone list for zone %d on node %d: ", j, i);
fs/buffer.c +3 -3
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/fs/buffer.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/fs/buffer.c
@@ -368,16 +368,16 @@ void invalidate_bdev(struct block_device
  */
 static void free_more_memory(void)
 {
-	struct zone **zones;
+	struct zoneref *zrefs;
 	int nid;
 
 	wakeup_pdflush(1024);
 	yield();
 
 	for_each_online_node(nid) {
-		zones = first_zones_zonelist(node_zonelist(nid, GFP_NOFS),
+		zrefs = first_zones_zonelist(node_zonelist(nid, GFP_NOFS),
 						gfp_zone(GFP_NOFS));
-		if (*zones)
+		if (zrefs->zone)
 			try_to_free_pages(node_zonelist(nid, GFP_NOFS), 0,
 						GFP_NOFS);
 	}
include/linux/mmzone.h +54 -10

106 lines not on this page · raw message

include/linux/oom.h +2 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/include/linux/oom.h linux-2.6.24-rc1-mm-020_zoneid_zonelist/include/linux/oom.h
@@ -23,8 +23,8 @@ enum oom_constraint {
 	CONSTRAINT_MEMORY_POLICY,
 };
 
-extern int try_set_zone_oom(struct zonelist *zonelist);
-extern void clear_zonelist_oom(struct zonelist *zonelist);
+extern int try_set_zone_oom(struct zonelist *zonelist, gfp_t gfp_flags);
+extern void clear_zonelist_oom(struct zonelist *zonelist, gfp_t gfp_flags);
 
 extern void out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask, int order);
 extern int register_oom_notifier(struct notifier_block *nb);
kernel/cpuset.c +2 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/kernel/cpuset.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/kernel/cpuset.c
@@ -1877,8 +1877,8 @@ int cpuset_zonelist_valid_mems_allowed(s
 {
 	int i;
 
-	for (i = 0; zl->zones[i]; i++) {
-		int nid = zone_to_nid(zl->zones[i]);
+	for (i = 0; zl->_zonerefs[i].zone; i++) {
+		int nid = zonelist_node_idx(zl->_zonerefs[i]);
 
 		if (node_isset(nid, current->mems_allowed))
 			return 1;
mm/hugetlb.c +2 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/hugetlb.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/mm/hugetlb.c
@@ -78,7 +78,8 @@ static struct page *dequeue_huge_page(st
 	struct mempolicy *mpol;
 	struct zonelist *zonelist = huge_zonelist(vma, address,
 					htlb_alloc_mask, &mpol);
-	struct zone *zone, **z;
+	struct zone *zone;
+	struct zoneref *z;
 
 	for_each_zone_zonelist(zone, z, zonelist, MAX_NR_ZONES - 1) {
 		nid = zone_to_nid(zone);
mm/mempolicy.c +21 -14
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/mempolicy.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/mm/mempolicy.c
@@ -157,7 +157,7 @@ static struct zonelist *bind_zonelist(no
 		for_each_node_mask(nd, *nodes) { 
 			struct zone *z = &NODE_DATA(nd)->node_zones[k];
 			if (z->present_pages > 0) 
-				zl->zones[num++] = z;
+				encode_zoneref(z, &zl->_zonerefs[num++]);
 		}
 		if (k == 0)
 			break;
@@ -167,7 +167,7 @@ static struct zonelist *bind_zonelist(no
 		kfree(zl);
 		return ERR_PTR(-EINVAL);
 	}
-	zl->zones[num] = NULL;
+	zl->_zonerefs[num].zone = NULL;
 	return zl;
 }
 
@@ -489,9 +489,11 @@ static void get_zonemask(struct mempolic
 	nodes_clear(*nodes);
 	switch (p->policy) {
 	case MPOL_BIND:
-		for (i = 0; p->v.zonelist->zones[i]; i++)
-			node_set(zone_to_nid(p->v.zonelist->zones[i]),
-				*nodes);
+		for (i = 0; p->v.zonelist->_zonerefs[i].zone; i++) {
+			struct zoneref *zref;
+			zref = &p->v.zonelist->_zonerefs[i];
+			node_set(zonelist_node_idx(zref), *nodes);
+		}
 		break;
 	case MPOL_DEFAULT:
 		break;
@@ -1201,12 +1203,13 @@ unsigned slab_node(struct mempolicy *pol
 	case MPOL_INTERLEAVE:
 		return interleave_nodes(policy);
 
-	case MPOL_BIND:
+	case MPOL_BIND: {
 		/*
 		 * Follow bind policy behavior and start allocation at the
 		 * first node.
 		 */
-		return zone_to_nid(policy->v.zonelist->zones[0]);
+		return zonelist_node_idx(policy->v.zonelist->_zonerefs);
+	}
 
 	case MPOL_PREFERRED:
 		if (policy->v.preferred_node >= 0)
@@ -1310,7 +1313,7 @@ static struct page *alloc_page_interleav
 
 	zl = node_zonelist(nid, gfp);
 	page = __alloc_pages(gfp, order, zl);
-	if (page && page_zone(page) == zl->zones[0])
+	if (page && page_zone(page) == zonelist_zone(&zl->_zonerefs[0]))
 		inc_zone_page_state(page, NUMA_INTERLEAVE_HIT);
 	return page;
 }
@@ -1447,10 +1450,14 @@ int __mpol_equal(struct mempolicy *a, st
 		return a->v.preferred_node == b->v.preferred_node;
 	case MPOL_BIND: {
 		int i;
-		for (i = 0; a->v.zonelist->zones[i]; i++)
-			if (a->v.zonelist->zones[i] != b->v.zonelist->zones[i])
+		for (i = 0; a->v.zonelist->_zonerefs[i].zone; i++) {
+			struct zone *za, *zb;
+			za = zonelist_zone(&a->v.zonelist->_zonerefs[i]);
+			zb = zonelist_zone(&b->v.zonelist->_zonerefs[i]);
+			if (za != zb)
 				return 0;
-		return b->v.zonelist->zones[i] == NULL;
+		}
+		return b->v.zonelist->_zonerefs[i].zone == NULL;
 	}
 	default:
 		BUG();
@@ -1769,12 +1776,12 @@ static void mpol_rebind_policy(struct me
 		break;
 	case MPOL_BIND: {
 		nodemask_t nodes;
-		struct zone **z;
+		struct zoneref *z;
 		struct zonelist *zonelist;
 
 		nodes_clear(nodes);
-		for (z = pol->v.zonelist->zones; *z; z++)
-			node_set(zone_to_nid(*z), nodes);
+		for (z = pol->v.zonelist->_zonerefs; z->zone; z++)
+			node_set(zonelist_node_idx(z), nodes);
 		nodes_remap(tmp, nodes, *mpolmask, *newmask);
 		nodes = tmp;
 
mm/oom_kill.c +22 -23

74 lines not on this page · raw message

mm/page_alloc.c +30 -29

205 lines not on this page · raw message

mm/slab.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/slab.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/mm/slab.c
@@ -3239,7 +3239,7 @@ static void *fallback_alloc(struct kmem_
 {
 	struct zonelist *zonelist;
 	gfp_t local_flags;
-	struct zone **z;
+	struct zoneref *z;
 	struct zone *zone;
 	enum zone_type high_zoneidx = gfp_zone(flags);
 	void *obj = NULL;
mm/slub.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/slub.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/mm/slub.c
@@ -1324,7 +1324,7 @@ static unsigned long get_any_partial(str
 {
 #ifdef CONFIG_NUMA
 	struct zonelist *zonelist;
-	struct zone **z;
+	struct zoneref *z;
 	struct zone *zone;
 	enum zone_type high_zoneidx = gfp_zone(flags);
 	unsigned long state;
mm/vmscan.c +3 -4
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/vmscan.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/mm/vmscan.c
@@ -1220,7 +1220,7 @@ static unsigned long shrink_zones(int pr
 					struct scan_control *sc)
 {
 	unsigned long nr_reclaimed = 0;
-	struct zone **z;
+	struct zoneref *z;
 	struct zone *zone;
 
 	sc->all_unreclaimable = 1;
@@ -1265,7 +1265,7 @@ static unsigned long do_try_to_free_page
 	unsigned long nr_reclaimed = 0;
 	struct reclaim_state *reclaim_state = current->reclaim_state;
 	unsigned long lru_pages = 0;
-	struct zone **z;
+	struct zoneref *z;
 	struct zone *zone;
 	enum zone_type high_zoneidx = gfp_zone(gfp_mask);
 
@@ -1376,9 +1376,8 @@ unsigned long try_to_free_mem_cgroup_pag
 	};
 	int node = numa_node_id();
 	struct zonelist *zonelist;
-	int target_zone = gfp_zone(GFP_HIGHUSER_MOVABLE);
 
-	zonelist = &NODE_DATA(node)->node_zonelists[target_zone];
+	zonelist = &NODE_DATA(node)->node_zonelists[0];
 	if (do_try_to_free_pages(zonelist, sc.gfp_mask, &sc))
 		return 1;
 	return 0;
mm/vmstat.c +3 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/mm/vmstat.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/mm/vmstat.c
@@ -365,11 +365,12 @@ void refresh_cpu_vm_stats(int cpu)
  */
 void zone_statistics(struct zonelist *zonelist, struct zone *z)
 {
-	if (z->zone_pgdat == zonelist->zones[0]->zone_pgdat) {
+	if (z->zone_pgdat == zonelist_zone(&zonelist->_zonerefs[0])->zone_pgdat) {
 		__inc_zone_state(z, NUMA_HIT);
 	} else {
 		__inc_zone_state(z, NUMA_MISS);
-		__inc_zone_state(zonelist->zones[0], NUMA_FOREIGN);
+		__inc_zone_state(zonelist_zone(&zonelist->_zonerefs[0]),
+								NUMA_FOREIGN);
 	}
 	if (z->node == numa_node_id())
 		__inc_zone_state(z, NUMA_LOCAL);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#6 lore raw

Re: [PATCH 4/6] Have zonelist contains structs with both a zone pointer and zone_idx

Quoted 33 lines · Mel Gorman

Filtering zonelists requires very frequent use of zone_idx(). This is costly as it involves a lookup of another structure and a substraction operation. As the zone_idx is often required, it should be quickly accessible. The node idx could also be stored here if it was found that accessing zone->node is significant which may be the case on workloads where nodemasks are heavily used.

This patch introduces a struct zoneref to store a zone pointer and a zone index. The zonelist then consists of an array of this struct zonerefs which are looked up as necessary. Helpers are given for accessing the zone index as well as the node index.

[[email protected]: Suggested struct zoneref instead of embedding information in pointers]

---

 arch/parisc/mm/init.c  |    2 -
 fs/buffer.c            |    6 ++--
 include/linux/mmzone.h |   64 +++++++++++++++++++++++++++++++++++++-------
 include/linux/oom.h    |    4 +-
 kernel/cpuset.c        |    4 +-
 mm/hugetlb.c           |    3 +-
 mm/mempolicy.c         |   35 ++++++++++++++----------
 mm/oom_kill.c          |   45 +++++++++++++++---------------
 mm/page_alloc.c        |   59 ++++++++++++++++++++--------------------
 mm/slab.c              |    2 -
 mm/slub.c              |    2 -
 mm/vmscan.c            |    7 ++--
 mm/vmstat.c            |    5 ++-
 13 files changed, 145 insertions(+), 93 deletions(-)

<snip>

kernel/cpuset.c int cpuset_zonelist_valid_mems_allowed(s · line 1881

Quoted 11 lines

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-010_use_two_zonelists/kernel/cpuset.c linux-2.6.24-rc1-mm-020_zoneid_zonelist/kernel/cpuset.c

kernel/cpuset.c +2 -2
@@ -1877,8 +1877,8 @@ int cpuset_zonelist_valid_mems_allowed(s
 {
 	int i;
 
-	for (i = 0; zl->zones[i]; i++) {
-		int nid = zone_to_nid(zl->zones[i]);
+	for (i = 0; zl->_zonerefs[i].zone; i++) {
+		int nid = zonelist_node_idx(zl->_zonerefs[i]);

Should be:

 +		  int nid = zonelist_node_idx(&zl->_zonerefs[i]);
                                              ^

else doesn't build. Stand by for testing...

kernel/cpuset.c int cpuset_zonelist_valid_mems_allowed(s · line 1884

Quoted 3 lines
kernel/cpuset.c +0 -0
 		if (node_isset(nid, current->mems_allowed))
 			return 1;

<snip>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#7 lore raw

PATCH 5/6 Filter based on a nodemask as well as a gfp_mask

In reply to Mel Gorman

The MPOL_BIND policy creates a zonelist that is used for allocations belonging to that thread that can use the policy_zone. As the per-node zonelist is already being filtered based on a zone id, this patch adds a version of __alloc_pages() that takes a nodemask for further filtering. This eliminates the need for MPOL_BIND to create a custom zonelist. A positive benefit of this is that allocations using MPOL_BIND now use the local-node-ordered zonelist instead of a custom node-id-ordered zonelist.

---

 fs/buffer.c               |    2 
 include/linux/cpuset.h    |    4 -
 include/linux/gfp.h       |    4 +
 include/linux/mempolicy.h |    3 
 include/linux/mmzone.h    |   58 +++++++++++++---
 kernel/cpuset.c           |   18 +----
 mm/mempolicy.c            |  144 +++++++++++------------------------------
 mm/page_alloc.c           |   40 +++++++----
 8 files changed, 131 insertions(+), 142 deletions(-)
fs/buffer.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-020_zoneid_zonelist/fs/buffer.c linux-2.6.24-rc1-mm-030_filter_nodemask/fs/buffer.c
@@ -376,7 +376,7 @@ static void free_more_memory(void)
 
 	for_each_online_node(nid) {
 		zrefs = first_zones_zonelist(node_zonelist(nid, GFP_NOFS),
-						gfp_zone(GFP_NOFS));
+						NULL, gfp_zone(GFP_NOFS));
 		if (zrefs->zone)
 			try_to_free_pages(node_zonelist(nid, GFP_NOFS), 0,
 						GFP_NOFS);
include/linux/cpuset.h +2 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-020_zoneid_zonelist/include/linux/cpuset.h linux-2.6.24-rc1-mm-030_filter_nodemask/include/linux/cpuset.h
@@ -28,7 +28,7 @@ void cpuset_init_current_mems_allowed(vo
 void cpuset_update_task_memory_state(void);
 #define cpuset_nodes_subset_current_mems_allowed(nodes) \
 		nodes_subset((nodes), current->mems_allowed)
-int cpuset_zonelist_valid_mems_allowed(struct zonelist *zl);
+int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask);
 
 extern int __cpuset_zone_allowed_softwall(struct zone *z, gfp_t gfp_mask);
 extern int __cpuset_zone_allowed_hardwall(struct zone *z, gfp_t gfp_mask);
@@ -103,7 +103,7 @@ static inline void cpuset_init_current_m
 static inline void cpuset_update_task_memory_state(void) {}
 #define cpuset_nodes_subset_current_mems_allowed(nodes) (1)
 
-static inline int cpuset_zonelist_valid_mems_allowed(struct zonelist *zl)
+static inline int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask)
 {
 	return 1;
 }
include/linux/gfp.h +4 -0
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-020_zoneid_zonelist/include/linux/gfp.h linux-2.6.24-rc1-mm-030_filter_nodemask/include/linux/gfp.h
@@ -184,6 +184,10 @@ static inline void arch_alloc_page(struc
 extern struct page *
 FASTCALL(__alloc_pages(gfp_t, unsigned int, struct zonelist *));
 
+extern struct page *
+FASTCALL(__alloc_pages_nodemask(gfp_t, unsigned int,
+				struct zonelist *, nodemask_t *nodemask));
+
 static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
 						unsigned int order)
 {
include/linux/mempolicy.h +1 -2
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-020_zoneid_zonelist/include/linux/mempolicy.h linux-2.6.24-rc1-mm-030_filter_nodemask/include/linux/mempolicy.h
@@ -64,9 +64,8 @@ struct mempolicy {
 	atomic_t refcnt;
 	short policy; 	/* See MPOL_* above */
 	union {
-		struct zonelist  *zonelist;	/* bind */
 		short 		 preferred_node; /* preferred */
-		nodemask_t	 nodes;		/* interleave */
+		nodemask_t	 nodes;		/* interleave/bind */
 		/* undefined for default */
 	} v;
 	nodemask_t cpuset_mems_allowed;	/* mempolicy relative to these nodes */
include/linux/mmzone.h +48 -10

95 lines not on this page · raw message

kernel/cpuset.c +5 -13
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-020_zoneid_zonelist/kernel/cpuset.c linux-2.6.24-rc1-mm-030_filter_nodemask/kernel/cpuset.c
@@ -1868,22 +1868,14 @@ nodemask_t cpuset_mems_allowed(struct ta
 }
 
 /**
- * cpuset_zonelist_valid_mems_allowed - check zonelist vs. curremt mems_allowed
- * @zl: the zonelist to be checked
+ * cpuset_nodemask_valid_mems_allowed - check nodemask vs. curremt mems_allowed
+ * @nodemask: the nodemask to be checked
  *
- * Are any of the nodes on zonelist zl allowed in current->mems_allowed?
+ * Are any of the nodes in the nodemask allowed in current->mems_allowed?
  */
-int cpuset_zonelist_valid_mems_allowed(struct zonelist *zl)
+int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask)
 {
-	int i;
-
-	for (i = 0; zl->_zonerefs[i].zone; i++) {
-		int nid = zonelist_node_idx(zl->_zonerefs[i]);
-
-		if (node_isset(nid, current->mems_allowed))
-			return 1;
-	}
-	return 0;
+	return nodes_intersects(*nodemask, current->mems_allowed);
 }
 
 /*
mm/mempolicy.c +42 -102

235 lines not on this page · raw message

mm/page_alloc.c +28 -12
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-020_zoneid_zonelist/mm/page_alloc.c linux-2.6.24-rc1-mm-030_filter_nodemask/mm/page_alloc.c
@@ -1399,7 +1399,7 @@ static void zlc_mark_zone_full(struct zo
  * a page.
  */
 static struct page *
-get_page_from_freelist(gfp_t gfp_mask, unsigned int order,
+get_page_from_freelist(gfp_t gfp_mask, nodemask_t *nodemask, unsigned int order,
 		struct zonelist *zonelist, int high_zoneidx, int alloc_flags)
 {
 	struct zoneref *z;
@@ -1410,7 +1410,7 @@ get_page_from_freelist(gfp_t gfp_mask, u
 	int zlc_active = 0;		/* set if using zonelist_cache */
 	int did_zlc_setup = 0;		/* just call zlc_setup() one time */
 
-	z = first_zones_zonelist(zonelist, high_zoneidx);
+	z = first_zones_zonelist(zonelist, nodemask, high_zoneidx);
 	classzone_idx = zonelist_zone_idx(z);
 
 zonelist_scan:
@@ -1418,7 +1418,8 @@ zonelist_scan:
 	 * Scan zonelist, looking for a zone with enough free.
 	 * See also cpuset_zone_allowed() comment in kernel/cpuset.c.
 	 */
-	for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
+	for_each_zone_zonelist_nodemask(zone, z, zonelist,
+						high_zoneidx, nodemask) {
 		if (NUMA_BUILD && zlc_active &&
 			!zlc_zone_worth_trying(zonelist, z, allowednodes))
 				continue;
@@ -1524,9 +1525,9 @@ static void set_page_owner(struct page *
 /*
  * This is the 'heart' of the zoned buddy allocator.
  */
-struct page * fastcall
-__alloc_pages(gfp_t gfp_mask, unsigned int order,
-		struct zonelist *zonelist)
+static struct page *
+__alloc_pages_internal(gfp_t gfp_mask, unsigned int order,
+			struct zonelist *zonelist, nodemask_t *nodemask)
 {
 	const gfp_t wait = gfp_mask & __GFP_WAIT;
 	enum zone_type high_zoneidx = gfp_zone(gfp_mask);
@@ -1555,7 +1556,7 @@ restart:
 		return NULL;
 	}
 
-	page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, order,
+	page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
 			zonelist, high_zoneidx, ALLOC_WMARK_LOW|ALLOC_CPUSET);
 	if (page)
 		goto got_pg;
@@ -1600,7 +1601,7 @@ restart:
 	 * Ignore cpuset if GFP_ATOMIC (!wait) rather than fail alloc.
 	 * See also cpuset_zone_allowed() comment in kernel/cpuset.c.
 	 */
-	page = get_page_from_freelist(gfp_mask, order, zonelist,
+	page = get_page_from_freelist(gfp_mask, nodemask, order, zonelist,
 						high_zoneidx, alloc_flags);
 	if (page)
 		goto got_pg;
@@ -1613,7 +1614,7 @@ rebalance:
 		if (!(gfp_mask & __GFP_NOMEMALLOC)) {
 nofail_alloc:
 			/* go through the zonelist yet again, ignoring mins */
-			page = get_page_from_freelist(gfp_mask, order,
+			page = get_page_from_freelist(gfp_mask, nodemask, order,
 				zonelist, high_zoneidx, ALLOC_NO_WATERMARKS);
 			if (page)
 				goto got_pg;
@@ -1648,7 +1649,7 @@ nofail_alloc:
 		drain_all_local_pages();
 
 	if (likely(did_some_progress)) {
-		page = get_page_from_freelist(gfp_mask, order,
+		page = get_page_from_freelist(gfp_mask, nodemask, order,
 					zonelist, high_zoneidx, alloc_flags);
 		if (page)
 			goto got_pg;
@@ -1664,8 +1665,9 @@ nofail_alloc:
 		 * a parallel oom killing, we must fail if we're still
 		 * under heavy pressure.
 		 */
-		page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, order,
-			zonelist, high_zoneidx, ALLOC_WMARK_HIGH|ALLOC_CPUSET);
+		page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask,
+			order, zonelist, high_zoneidx,
+			ALLOC_WMARK_HIGH|ALLOC_CPUSET);
 		if (page) {
 			clear_zonelist_oom(zonelist, gfp_mask);
 			goto got_pg;
@@ -1718,6 +1720,20 @@ got_pg:
 	return page;
 }
 
+struct page * fastcall
+__alloc_pages(gfp_t gfp_mask, unsigned int order,
+		struct zonelist *zonelist)
+{
+	return __alloc_pages_internal(gfp_mask, order, zonelist, NULL);
+}
+
+struct page * fastcall
+__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
+		struct zonelist *zonelist, nodemask_t *nodemask)
+{
+	return __alloc_pages_internal(gfp_mask, order, zonelist, nodemask);
+}
+
 EXPORT_SYMBOL(__alloc_pages);
 
 /*

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#8 lore raw

Re: [PATCH 5/6] Filter based on a nodemask as well as a gfp_mask

Quoted 3 lines · Mel

A positive benefit of
this is that allocations using MPOL_BIND now use the local-node-ordered zonelist instead of a custom node-id-ordered zonelist.

Could you update the now obsolete documentation (perhaps just delete the no longer correct remark):

Documentation/vm/numa_memory_policy.txt:

MPOL_BIND: This mode specifies that memory must come from the set of nodes specified by the policy.

The memory policy APIs do not specify an order in which the nodes will be searched. However, unlike "local allocation", the Bind policy does not consider the distance between the nodes. Rather, allocations will fallback to the nodes specified by the policy in order of numeric node id. Like everything in Linux, this is subject to change.

Signature · 9 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.940.382.4214

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#9 lore raw

Re: [PATCH 5/6] Filter based on a nodemask as well as a gfp_mask

Quoted 13 lines · Paul Jackson
Quoted 3 lines · Mel

A positive benefit of
this is that allocations using MPOL_BIND now use the local-node-ordered zonelist instead of a custom node-id-ordered zonelist.

Could you update the now obsolete documentation (perhaps just delete the no longer correct remark):

Documentation/vm/numa_memory_policy.txt:

MPOL_BIND: This mode specifies that memory must come from the set of nodes specified by the policy.

Quoted 7 lines

The memory policy APIs do not specify an order in which the nodes will be searched. However, unlike "local allocation", the Bind policy does not consider the distance between the nodes. Rather, allocations will fallback to the nodes specified by the policy in order of numeric node id. Like everything in Linux, this is subject to change.

Yes, will do.

Thanks, Lee

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#10 lore raw

PATCH 1/1 2.6.25-rc3-mm1 - Mempolicy - update stale documentation and comments

In reply to Paul Jackson

Was Re: [PATCH 5/6] Filter based on a nodemask as well as a gfp_mask

Quoted 13 lines · Paul Jackson
Quoted 3 lines · Mel

A positive benefit of
this is that allocations using MPOL_BIND now use the local-node-ordered zonelist instead of a custom node-id-ordered zonelist.

Could you update the now obsolete documentation (perhaps just delete the no longer correct remark):

Documentation/vm/numa_memory_policy.txt:

MPOL_BIND: This mode specifies that memory must come from the set of nodes specified by the policy.

Quoted 7 lines

The memory policy APIs do not specify an order in which the nodes will be searched. However, unlike "local allocation", the Bind policy does not consider the distance between the nodes. Rather, allocations will fallback to the nodes specified by the policy in order of numeric node id. Like everything in Linux, this is subject to change.

How's this:

PATCH Mempolicy: update documentation and comments

Address stale comments and numa_memory_policy.txt discussion based on Mel Gorman's changes to page allocation zonelist handling.

Specifically: mpol_free() and mpol_copy() no longer need to deal with a custom zonelist for MPOL_BIND policy, and MPOL_BIND now allocates memory from the nearest node with available memory in the specified nodemask.

In "fixing" the mpol_free() and mpol_copy() comments in mempolicy.h, I wanted to replace them with something, rather than just deleting them. So, I described the reference counting. Not directly related to the zonelist changes, but useful, IMO.

 Documentation/vm/numa_memory_policy.txt |   11 +++--------
 include/linux/mempolicy.h               |    8 +++++---
 2 files changed, 8 insertions(+), 11 deletions(-)
Documentation/vm/numa_memory_policy.txt +3 -8
Index: linux-2.6.25-rc3-mm1/Documentation/vm/numa_memory_policy.txt
===================================================================
@@ -182,14 +182,9 @@ Components of Memory Policies
 	    The Default mode does not use the optional set of nodes.
 
 	MPOL_BIND:  This mode specifies that memory must come from the
-	set of nodes specified by the policy.
-
-	    The memory policy APIs do not specify an order in which the nodes
-	    will be searched.  However, unlike "local allocation", the Bind
-	    policy does not consider the distance between the nodes.  Rather,
-	    allocations will fallback to the nodes specified by the policy in
-	    order of numeric node id.  Like everything in Linux, this is subject
-	    to change.
+	set of nodes specified by the policy.  Memory will be allocated from
+	the node in the set with sufficient free memory that is closest to
+	the node where the allocation takes place.
 
 	MPOL_PREFERRED:  This mode specifies that the allocation should be
 	attempted from the single node specified in the policy.  If that
include/linux/mempolicy.h +5 -3
Index: linux-2.6.25-rc3-mm1/include/linux/mempolicy.h
===================================================================
@@ -54,11 +54,13 @@ struct mm_struct;
  * mmap_sem.
  *
  * Freeing policy:
- * When policy is MPOL_BIND v.zonelist is kmalloc'ed and must be kfree'd.
- * All other policies don't have any external state. mpol_free() handles this.
+ * Mempolicy objects are reference counted.  A mempolicy will be freed when
+ * mpol_free() decrements the reference count to zero.
  *
  * Copying policy objects:
- * For MPOL_BIND the zonelist must be always duplicated. mpol_clone() does this.
+ * mpol_copy() allocates a new mempolicy and copies the specified mempolicy
+ * to the new storage.  The reference count of the new object is initialized
+ * to 1, representing the caller of mpol_copy().
  */
 struct mempolicy {
 	atomic_t refcnt;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Acked-by on v1 1/1
#11 lore raw

Re: [PATCH] 2.6.25-rc3-mm1 - Mempolicy - update stale documentation and comments

Quoted 3 lines · Lee

How's this:

PATCH Mempolicy: update documentation and comments

I like it - thanks.

Signature · 9 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.940.382.4214

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author Acked-by on v1 1/1
#12 lore raw

Re: [PATCH] 2.6.25-rc3-mm1 - Mempolicy - update stale documentation and comments

In reply to Lee Schermerhorn

On (04/03/08 15:20), Lee Schermerhorn didst pronounce:

Quoted 36 lines

Was Re: [PATCH 5/6] Filter based on a nodemask as well as a gfp_mask

Quoted 13 lines · Paul Jackson
Quoted 3 lines · Mel

A positive benefit of
this is that allocations using MPOL_BIND now use the local-node-ordered zonelist instead of a custom node-id-ordered zonelist.

Could you update the now obsolete documentation (perhaps just delete the no longer correct remark):

Documentation/vm/numa_memory_policy.txt:

MPOL_BIND: This mode specifies that memory must come from the set of nodes specified by the policy.

Quoted 7 lines

The memory policy APIs do not specify an order in which the nodes will be searched. However, unlike "local allocation", the Bind policy does not consider the distance between the nodes. Rather, allocations will fallback to the nodes specified by the policy in order of numeric node id. Like everything in Linux, this is subject to change.

How's this:

PATCH Mempolicy: update documentation and comments

Address stale comments and numa_memory_policy.txt discussion based on Mel Gorman's changes to page allocation zonelist handling.

Specifically: mpol_free() and mpol_copy() no longer need to deal with a custom zonelist for MPOL_BIND policy, and MPOL_BIND now allocates memory from the nearest node with available memory in the specified nodemask.

Quoted 7 lines

In "fixing" the mpol_free() and mpol_copy() comments in mempolicy.h, I wanted to replace them with something, rather than just deleting them. So, I described the reference counting. Not directly related to the zonelist changes, but useful, IMO.

Looks great.

Quoted 49 lines
 Documentation/vm/numa_memory_policy.txt |   11 +++--------
 include/linux/mempolicy.h               |    8 +++++---
 2 files changed, 8 insertions(+), 11 deletions(-)

Index: linux-2.6.25-rc3-mm1/Documentation/vm/numa_memory_policy.txt
===================================================================

Documentation/vm/numa_memory_policy.txt +3 -8
@@ -182,14 +182,9 @@ Components of Memory Policies
 	    The Default mode does not use the optional set of nodes.
 
 	MPOL_BIND:  This mode specifies that memory must come from the
-	set of nodes specified by the policy.
-
-	    The memory policy APIs do not specify an order in which the nodes
-	    will be searched.  However, unlike "local allocation", the Bind
-	    policy does not consider the distance between the nodes.  Rather,
-	    allocations will fallback to the nodes specified by the policy in
-	    order of numeric node id.  Like everything in Linux, this is subject
-	    to change.
+	set of nodes specified by the policy.  Memory will be allocated from
+	the node in the set with sufficient free memory that is closest to
+	the node where the allocation takes place.
 
 	MPOL_PREFERRED:  This mode specifies that the allocation should be
 	attempted from the single node specified in the policy.  If that

Index: linux-2.6.25-rc3-mm1/include/linux/mempolicy.h
===================================================================

include/linux/mempolicy.h +5 -3
@@ -54,11 +54,13 @@ struct mm_struct;
  * mmap_sem.
  *
  * Freeing policy:
- * When policy is MPOL_BIND v.zonelist is kmalloc'ed and must be kfree'd.
- * All other policies don't have any external state. mpol_free() handles this.
+ * Mempolicy objects are reference counted.  A mempolicy will be freed when
+ * mpol_free() decrements the reference count to zero.
  *
  * Copying policy objects:
- * For MPOL_BIND the zonelist must be always duplicated. mpol_clone() does this.
+ * mpol_copy() allocates a new mempolicy and copies the specified mempolicy
+ * to the new storage.  The reference count of the new object is initialized
+ * to 1, representing the caller of mpol_copy().
  */
 struct mempolicy {
 	atomic_t refcnt;
Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#13 lore raw

PATCH 6/6 Use one zonelist that is filtered by nodemask

In reply to Mel Gorman

Two zonelists exist so that GFP_THISNODE allocations will be guaranteed to use memory only from a node local to the CPU. As we can now filter the zonelist based on a nodemask, we filter the standard node zonelist for zones on the local node when GFP_THISNODE is specified.

When GFP_THISNODE is used, a temporary nodemask is created. By default, it will only contain zones local to the CPU. If a node ID is specified by the caller, that node is used. This allows us to eliminate the second zonelist.

---

 drivers/char/sysrq.c      |    2 -
 fs/buffer.c               |    5 +--
 include/linux/gfp.h       |   30 +++++++++-----------
 include/linux/mempolicy.h |    2 -
 include/linux/mmzone.h    |   14 ---------
 mm/mempolicy.c            |    8 ++---
 mm/page_alloc.c           |   59 ++++++++++++++++++++++-------------------
 mm/slab.c                 |    2 -
 mm/slub.c                 |    2 -
 mm/vmscan.c               |    2 -
 10 files changed, 58 insertions(+), 68 deletions(-)
drivers/char/sysrq.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/drivers/char/sysrq.c linux-2.6.24-rc1-mm-040_use_one_zonelist/drivers/char/sysrq.c
@@ -271,7 +271,7 @@ static struct sysrq_key_op sysrq_term_op
 
 static void moom_callback(struct work_struct *ignored)
 {
-	out_of_memory(node_zonelist(0, GFP_KERNEL), GFP_KERNEL, 0);
+	out_of_memory(node_zonelist(0), GFP_KERNEL, 0);
 }
 
 static DECLARE_WORK(moom_work, moom_callback);
fs/buffer.c +2 -3
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/fs/buffer.c linux-2.6.24-rc1-mm-040_use_one_zonelist/fs/buffer.c
@@ -375,11 +375,10 @@ static void free_more_memory(void)
 	yield();
 
 	for_each_online_node(nid) {
-		zrefs = first_zones_zonelist(node_zonelist(nid, GFP_NOFS),
+		zrefs = first_zones_zonelist(node_zonelist(nid),
 						NULL, gfp_zone(GFP_NOFS));
 		if (zrefs->zone)
-			try_to_free_pages(node_zonelist(nid, GFP_NOFS), 0,
-						GFP_NOFS);
+			try_to_free_pages(node_zonelist(nid), 0, GFP_NOFS);
 	}
 }
 
include/linux/gfp.h +14 -16
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/include/linux/gfp.h linux-2.6.24-rc1-mm-040_use_one_zonelist/include/linux/gfp.h
@@ -150,28 +150,16 @@ static inline gfp_t set_migrateflags(gfp
  * virtual kernel addresses to the allocated page(s).
  */
 
-static inline enum zone_type gfp_zonelist(gfp_t flags)
-{
-	int base = 0;
-
-	if (NUMA_BUILD && (flags & __GFP_THISNODE))
-		base = 1;
-
-	return base;
-}
-
 /*
- * We get the zone list from the current node and the gfp_mask.
+ * We get the zone list based on a node ID as there is one zone list per node.
  * This zone list contains a maximum of MAXNODES*MAX_NR_ZONES zones.
- * There are two zonelists per node, one for all zones with memory and
- * one containing just zones from the node the zonelist belongs to.
  *
  * For the normal case of non-DISCONTIGMEM systems the NODE_DATA() gets
  * optimized to &contig_page_data at compile-time.
  */
-static inline struct zonelist *node_zonelist(int nid, gfp_t flags)
+static inline struct zonelist *node_zonelist(int nid)
 {
-	return NODE_DATA(nid)->node_zonelists + gfp_zonelist(flags);
+	return &NODE_DATA(nid)->node_zonelist;
 }
 
 #ifndef HAVE_ARCH_FREE_PAGE
@@ -187,6 +175,7 @@ FASTCALL(__alloc_pages(gfp_t, unsigned i
 extern struct page *
 FASTCALL(__alloc_pages_nodemask(gfp_t, unsigned int,
 				struct zonelist *, nodemask_t *nodemask));
+extern nodemask_t *nodemask_thisnode(int nid, nodemask_t *nodemask);
 
 static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
 						unsigned int order)
@@ -198,7 +187,16 @@ static inline struct page *alloc_pages_n
 	if (nid < 0)
 		nid = numa_node_id();
 
-	return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
+	/* Use a temporary nodemask for __GFP_THISNODE allocations */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;
+
+		return __alloc_pages_nodemask(gfp_mask, order,
+				node_zonelist(nid),
+				nodemask_thisnode(nid, &nodemask));
+	}
+
+	return __alloc_pages(gfp_mask, order, node_zonelist(nid));
 }
 
 #ifdef CONFIG_NUMA
include/linux/mempolicy.h +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/include/linux/mempolicy.h linux-2.6.24-rc1-mm-040_use_one_zonelist/include/linux/mempolicy.h
@@ -240,7 +240,7 @@ static inline void mpol_fix_fork_child_f
 static inline struct zonelist *huge_zonelist(struct vm_area_struct *vma,
  		unsigned long addr, gfp_t gfp_flags, struct mempolicy **mpol)
 {
-	return node_zonelist(0, gfp_flags);
+	return node_zonelist(0);
 }
 
 static inline int do_migrate_pages(struct mm_struct *mm,
include/linux/mmzone.h +1 -13
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/include/linux/mmzone.h linux-2.6.24-rc1-mm-040_use_one_zonelist/include/linux/mmzone.h
@@ -388,17 +388,6 @@ static inline int zone_is_oom_locked(con
 #define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
 
 #ifdef CONFIG_NUMA
-
-/*
- * The NUMA zonelists are doubled becausse we need zonelists that restrict the
- * allocations to a single node for GFP_THISNODE.
- *
- * [0]	: Zonelist with fallback
- * [1]	: No fallback (GFP_THISNODE)
- */
-#define MAX_ZONELISTS 2
-
-
 /*
  * We cache key information from each zonelist for smaller cache
  * footprint when scanning for free pages in get_page_from_freelist().
@@ -464,7 +453,6 @@ struct zonelist_cache {
 	unsigned long last_full_zap;		/* when last zap'd (jiffies) */
 };
 #else
-#define MAX_ZONELISTS 1
 struct zonelist_cache;
 #endif
 
@@ -529,7 +517,7 @@ extern struct page *mem_map;
 struct bootmem_data;
 typedef struct pglist_data {
 	struct zone node_zones[MAX_NR_ZONES];
-	struct zonelist node_zonelists[MAX_ZONELISTS];
+	struct zonelist node_zonelist;
 	int nr_zones;
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 	struct page *node_mem_map;
mm/mempolicy.c +4 -4
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/mm/mempolicy.c linux-2.6.24-rc1-mm-040_use_one_zonelist/mm/mempolicy.c
@@ -1153,7 +1153,7 @@ static struct zonelist *zonelist_policy(
 		nd = 0;
 		BUG();
 	}
-	return node_zonelist(nd, gfp);
+	return node_zonelist(nd);
 }
 
 /* Do dynamic interleaving for a process */
@@ -1190,7 +1190,7 @@ unsigned slab_node(struct mempolicy *pol
 		struct zonelist *zonelist;
 		struct zoneref *z;
 		enum zone_type highest_zoneidx = gfp_zone(GFP_KERNEL);
-		zonelist = &NODE_DATA(numa_node_id())->node_zonelists[0];
+		zonelist = &NODE_DATA(numa_node_id())->node_zonelist;
 		z = first_zones_zonelist(zonelist, &policy->v.nodes,
 							highest_zoneidx);
 		return zonelist_node_idx(z);
@@ -1274,7 +1274,7 @@ struct zonelist *huge_zonelist(struct vm
 
 		nid = interleave_nid(pol, vma, addr, HPAGE_SHIFT);
 		__mpol_free(pol);		/* finished with pol */
-		return node_zonelist(nid, gfp_flags);
+		return node_zonelist(nid);
 	}
 
 	zl = zonelist_policy(GFP_HIGHUSER, pol);
@@ -1296,7 +1296,7 @@ static struct page *alloc_page_interleav
 	struct zonelist *zl;
 	struct page *page;
 
-	zl = node_zonelist(nid, gfp);
+	zl = node_zonelist(nid);
 	page = __alloc_pages(gfp, order, zl);
 	if (page && page_zone(page) == zonelist_zone(&zl->_zonerefs[0]))
 		inc_zone_page_state(page, NUMA_INTERLEAVE_HIT);
mm/page_alloc.c +32 -27

135 lines not on this page · raw message

mm/slab.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/mm/slab.c linux-2.6.24-rc1-mm-040_use_one_zonelist/mm/slab.c
@@ -3248,7 +3248,7 @@ static void *fallback_alloc(struct kmem_
 	if (flags & __GFP_THISNODE)
 		return NULL;
 
-	zonelist = node_zonelist(slab_node(current->mempolicy), flags);
+	zonelist = node_zonelist(slab_node(current->mempolicy));
 	local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
 
 retry:
mm/slub.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/mm/slub.c linux-2.6.24-rc1-mm-040_use_one_zonelist/mm/slub.c
@@ -1351,7 +1351,7 @@ static unsigned long get_any_partial(str
 			get_cycles() % 1024 > s->remote_node_defrag_ratio)
 		return 0;
 
-	zonelist = node_zonelist(slab_node(current->mempolicy), flags);
+	zonelist = node_zonelist(slab_node(current->mempolicy));
 	for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
 		struct kmem_cache_node *n;
 
mm/vmscan.c +1 -1
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc1-mm-030_filter_nodemask/mm/vmscan.c linux-2.6.24-rc1-mm-040_use_one_zonelist/mm/vmscan.c
@@ -1377,7 +1377,7 @@ unsigned long try_to_free_mem_cgroup_pag
 	int node = numa_node_id();
 	struct zonelist *zonelist;
 
-	zonelist = &NODE_DATA(node)->node_zonelists[0];
+	zonelist = &NODE_DATA(node)->node_zonelist;
 	if (do_try_to_free_pages(zonelist, sc.gfp_mask, &sc))
 		return 1;
 	return 0;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#14 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

mm/page_alloc.c got_pg: · line 1742

Quoted 11 lines · Mel Gorman

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

mm/page_alloc.c +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

mm/page_alloc.c got_pg: · line 1745

Quoted 3 lines
mm/page_alloc.c +3 -0
+
+		return __alloc_pages_internal(gfp_mask, order,
+			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));

Argh.... GFP_THISNODE must use the nid passed to alloc_pages_node and *not* the local numa node id. Only if the node specified to alloc_pages nodes is -1 will this work.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#15 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

On (09/11/07 07:45), Christoph Lameter didst pronounce:

Quoted 18 lines · Mel Gorman

mm/page_alloc.c got_pg: · line 1742

Quoted 12 lines

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

mm/page_alloc.c +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

That is what I was hinting at in the comment as a possible solution.

Quoted 8 lines

mm/page_alloc.c got_pg: · line 1745

Quoted 3 lines
mm/page_alloc.c +3 -0
+
+		return __alloc_pages_internal(gfp_mask, order,
+			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));

Argh.... GFP_THISNODE must use the nid passed to alloc_pages_node and *not* the local numa node id. Only if the node specified to alloc_pages nodes is -1 will this work.

alloc_pages_node() calls __alloc_pages_nodemask() though where in this function if I'm reading it right is called without a node id. Given no other details on the nid, the current one seemed a logical choice.

What I did notice when rechecking is I left the warning about THISNODE in by accident :(

Signature · 10 lines
-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#16 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 8 lines · Mel Gorman
Quoted 4 lines

Argh.... GFP_THISNODE must use the nid passed to alloc_pages_node and *not* the local numa node id. Only if the node specified to alloc_pages nodes is -1 will this work.

alloc_pages_node() calls __alloc_pages_nodemask() though where in this function if I'm reading it right is called without a node id. Given no other details on the nid, the current one seemed a logical choice.

If the function only called when the first zone == numa_node_id then the use of numa_node_id here is okay.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#17 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

In reply to Mel Gorman
Quoted 31 lines · Mel Gorman

On (09/11/07 07:45), Christoph Lameter didst pronounce:

Quoted 18 lines · Mel Gorman

mm/page_alloc.c got_pg: · line 1742

Quoted 12 lines

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

mm/page_alloc.c +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

That is what I was hinting at in the comment as a possible solution.

Quoted 8 lines

mm/page_alloc.c got_pg: · line 1745

Quoted 3 lines
mm/page_alloc.c +3 -0
+
+		return __alloc_pages_internal(gfp_mask, order,
+			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));

Argh.... GFP_THISNODE must use the nid passed to alloc_pages_node and *not* the local numa node id. Only if the node specified to alloc_pages nodes is -1 will this work.

Quoted 3 lines

alloc_pages_node() calls __alloc_pages_nodemask() though where in this function if I'm reading it right is called without a node id. Given no other details on the nid, the current one seemed a logical choice.

Yeah, I guess the context here matters (and is a little hard to follow because thare are a few places that change in different ways here):

For allocating pages from a particular node (GFP_THISNODE with nid), the nid clearly must be specified. This only happens with alloc_pages_node(), AFAICT. So, in that interface, the right thing is done and the appropriate nodemask will be built.

On the other hand, if we call alloc_pages() with GFP_THISNODE set, there is no nid to base the allocation on, so we "fallback" to numa_node_id() [ almost like the nid had been specified as -1 ].

So I guess this is logical -- but I wonder, do we have any callers of alloc_pages(GFP_THISNODE) ? It seems like an odd thing to do, when alloc_pages_node() exists?

Thanks,
Nish

Signature · 8 lines
Nishanth Aravamudan <[email protected]>
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#18 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 36 lines · Nishanth Aravamudan
Quoted 31 lines · Mel Gorman

On (09/11/07 07:45), Christoph Lameter didst pronounce:

Quoted 18 lines · Mel Gorman

mm/page_alloc.c got_pg: · line 1742

Quoted 12 lines

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

mm/page_alloc.c +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

That is what I was hinting at in the comment as a possible solution.

Quoted 8 lines

mm/page_alloc.c got_pg: · line 1745

Quoted 3 lines
mm/page_alloc.c +3 -0
+
+		return __alloc_pages_internal(gfp_mask, order,
+			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));

Argh.... GFP_THISNODE must use the nid passed to alloc_pages_node and *not* the local numa node id. Only if the node specified to alloc_pages nodes is -1 will this work.

Quoted 3 lines

alloc_pages_node() calls __alloc_pages_nodemask() though where in this function if I'm reading it right is called without a node id. Given no other details on the nid, the current one seemed a logical choice.

Quoted 7 lines

Yeah, I guess the context here matters (and is a little hard to follow because thare are a few places that change in different ways here):

For allocating pages from a particular node (GFP_THISNODE with nid), the nid clearly must be specified. This only happens with alloc_pages_node(), AFAICT. So, in that interface, the right thing is done and the appropriate nodemask will be built.

I agree. In an earlier patch, Mel was ignoring nid and using numa_node_id() here. This was causing your [Nish's] hugetlb pool allocation patches to fail. Mel fixed that ~9oct07.

Quoted 8 lines

On the other hand, if we call alloc_pages() with GFP_THISNODE set, there is no nid to base the allocation on, so we "fallback" to numa_node_id() [ almost like the nid had been specified as -1 ].

So I guess this is logical -- but I wonder, do we have any callers of alloc_pages(GFP_THISNODE) ? It seems like an odd thing to do, when alloc_pages_node() exists?

I don't know if we have any current callers that do this, but absent any documentation specifying otherwise, Mel's implementation matches what I'd expect the behavior to be if I DID call alloc_pages with 'THISNODE. However, we could specify that THISNODE is ignored in __alloc_pages() and recommend the use of alloc_pages_node() passing numa_node_id() as the nid parameter to achieve the behavior. This would eliminate the check for 'THISNODE in __alloc_pages(). Just mask it off before calling down to __alloc_pages_internal().

Does this make sense?

Lee

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#19 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 40 lines · Lee Schermerhorn
Quoted 39 lines · Nishanth Aravamudan
Quoted 31 lines · Mel Gorman

On (09/11/07 07:45), Christoph Lameter didst pronounce:

Quoted 18 lines · Mel Gorman

mm/page_alloc.c got_pg: · line 1742

Quoted 12 lines

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

mm/page_alloc.c +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

That is what I was hinting at in the comment as a possible solution.

Quoted 8 lines

mm/page_alloc.c got_pg: · line 1745

Quoted 3 lines
mm/page_alloc.c +3 -0
+
+		return __alloc_pages_internal(gfp_mask, order,
+			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));

Argh.... GFP_THISNODE must use the nid passed to alloc_pages_node and *not* the local numa node id. Only if the node specified to alloc_pages nodes is -1 will this work.

Quoted 3 lines

alloc_pages_node() calls __alloc_pages_nodemask() though where in this function if I'm reading it right is called without a node id. Given no other details on the nid, the current one seemed a logical choice.

Yeah, I guess the context here matters (and is a little hard to follow because thare are a few places that change in different ways here):

Quoted 8 lines
Quoted 4 lines

For allocating pages from a particular node (GFP_THISNODE with nid), the nid clearly must be specified. This only happens with alloc_pages_node(), AFAICT. So, in that interface, the right thing is done and the appropriate nodemask will be built.

I agree. In an earlier patch, Mel was ignoring nid and using numa_node_id() here. This was causing your [Nish's] hugetlb pool allocation patches to fail. Mel fixed that ~9oct07.

Yep, and that's why I'm on the Cc, I think :)

Quoted 17 lines
Quoted 7 lines

On the other hand, if we call alloc_pages() with GFP_THISNODE set, there is no nid to base the allocation on, so we "fallback" to numa_node_id() [ almost like the nid had been specified as -1 ].

So I guess this is logical -- but I wonder, do we have any callers of alloc_pages(GFP_THISNODE) ? It seems like an odd thing to do, when alloc_pages_node() exists?

I don't know if we have any current callers that do this, but absent any documentation specifying otherwise, Mel's implementation matches what I'd expect the behavior to be if I DID call alloc_pages with 'THISNODE. However, we could specify that THISNODE is ignored in __alloc_pages() and recommend the use of alloc_pages_node() passing numa_node_id() as the nid parameter to achieve the behavior. This would eliminate the check for 'THISNODE in __alloc_pages(). Just mask it off before calling down to __alloc_pages_internal().

Quoted 1 line

Does this make sense?

The caller could also just use -1 as the nid, since then alloc_pages_node() should do the numa_node_id() for the caller... But I agree, there is no documentation saying GFP_THISNODE is *not* allowed for alloc_pages(), so we should probably handle it

-Nish

Signature · 8 lines
Nishanth Aravamudan <[email protected]>
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#20 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

In reply to Lee Schermerhorn
Quoted 17 lines · Lee Schermerhorn
Quoted 7 lines

On the other hand, if we call alloc_pages() with GFP_THISNODE set, there is no nid to base the allocation on, so we "fallback" to numa_node_id() [ almost like the nid had been specified as -1 ].

So I guess this is logical -- but I wonder, do we have any callers of alloc_pages(GFP_THISNODE) ? It seems like an odd thing to do, when alloc_pages_node() exists?

I don't know if we have any current callers that do this, but absent any documentation specifying otherwise, Mel's implementation matches what I'd expect the behavior to be if I DID call alloc_pages with 'THISNODE. However, we could specify that THISNODE is ignored in __alloc_pages() and recommend the use of alloc_pages_node() passing numa_node_id() as the nid parameter to achieve the behavior. This would eliminate the check for 'THISNODE in __alloc_pages(). Just mask it off before calling down to __alloc_pages_internal().

Quoted 1 line

Does this make sense?

I like consistency. If someone absolutely wants a local page then specifying GFP_THISNODE to __alloc_pages is okay. Leave as is I guess.

What happens though if an MPOL_BIND policy is in effect? The node used must then be the nearest node from the policy mask....

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#21 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 19 lines · Christoph Lameter
Quoted 17 lines · Lee Schermerhorn
Quoted 7 lines

On the other hand, if we call alloc_pages() with GFP_THISNODE set, there is no nid to base the allocation on, so we "fallback" to numa_node_id() [ almost like the nid had been specified as -1 ].

So I guess this is logical -- but I wonder, do we have any callers of alloc_pages(GFP_THISNODE) ? It seems like an odd thing to do, when alloc_pages_node() exists?

I don't know if we have any current callers that do this, but absent any documentation specifying otherwise, Mel's implementation matches what I'd expect the behavior to be if I DID call alloc_pages with 'THISNODE. However, we could specify that THISNODE is ignored in __alloc_pages() and recommend the use of alloc_pages_node() passing numa_node_id() as the nid parameter to achieve the behavior. This would eliminate the check for 'THISNODE in __alloc_pages(). Just mask it off before calling down to __alloc_pages_internal().

Quoted 4 lines
Quoted 1 line

Does this make sense?

I like consistency. If someone absolutely wants a local page then specifying GFP_THISNODE to __alloc_pages is okay. Leave as is I guess.

Fair enough.

Quoted 2 lines

What happens though if an MPOL_BIND policy is in effect? The node used must then be the nearest node from the policy mask....

Indeed, this probably needs to be validated... Sigh, more interleaving of policies and everything else...

-Nish

Signature · 8 lines
Nishanth Aravamudan <[email protected]>
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#22 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 25 lines · Nishanth Aravamudan
Quoted 19 lines · Christoph Lameter
Quoted 17 lines · Lee Schermerhorn
Quoted 7 lines

On the other hand, if we call alloc_pages() with GFP_THISNODE set, there is no nid to base the allocation on, so we "fallback" to numa_node_id() [ almost like the nid had been specified as -1 ].

So I guess this is logical -- but I wonder, do we have any callers of alloc_pages(GFP_THISNODE) ? It seems like an odd thing to do, when alloc_pages_node() exists?

I don't know if we have any current callers that do this, but absent any documentation specifying otherwise, Mel's implementation matches what I'd expect the behavior to be if I DID call alloc_pages with 'THISNODE. However, we could specify that THISNODE is ignored in __alloc_pages() and recommend the use of alloc_pages_node() passing numa_node_id() as the nid parameter to achieve the behavior. This would eliminate the check for 'THISNODE in __alloc_pages(). Just mask it off before calling down to __alloc_pages_internal().

Quoted 4 lines
Quoted 1 line

Does this make sense?

I like consistency. If someone absolutely wants a local page then specifying GFP_THISNODE to __alloc_pages is okay. Leave as is I guess.

Quoted 7 lines

Fair enough.

Quoted 2 lines

What happens though if an MPOL_BIND policy is in effect? The node used must then be the nearest node from the policy mask....

Indeed, this probably needs to be validated... Sigh, more interleaving of policies and everything else...

Hrm, more importantly, isn't this an existing issue? Maybe should be resolved separately from the one zonelist patches.

-Nish

Signature · 8 lines
Nishanth Aravamudan <[email protected]>
IBM Linux Technology Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#23 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 5 lines · Nishanth Aravamudan
Quoted 2 lines

Indeed, this probably needs to be validated... Sigh, more interleaving of policies and everything else...

Hrm, more importantly, isn't this an existing issue? Maybe should be resolved separately from the one zonelist patches.

GFP_THISNODE with alloc_pages() currently yields an allocation from the first node of the MPOL_BIND zonelist. So its the lowest node of the set.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#24 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

In reply to Christoph Lameter

On (09/11/07 09:26), Christoph Lameter didst pronounce:

Quoted 19 lines · Lee Schermerhorn
Quoted 18 lines
Quoted 8 lines

On the other hand, if we call alloc_pages() with GFP_THISNODE set, there is no nid to base the allocation on, so we "fallback" to numa_node_id() [ almost like the nid had been specified as -1 ].

So I guess this is logical -- but I wonder, do we have any callers of alloc_pages(GFP_THISNODE) ? It seems like an odd thing to do, when alloc_pages_node() exists?

I don't know if we have any current callers that do this, but absent any documentation specifying otherwise, Mel's implementation matches what I'd expect the behavior to be if I DID call alloc_pages with 'THISNODE. However, we could specify that THISNODE is ignored in __alloc_pages() and recommend the use of alloc_pages_node() passing numa_node_id() as the nid parameter to achieve the behavior. This would eliminate the check for 'THISNODE in __alloc_pages(). Just mask it off before calling down to __alloc_pages_internal().

Quoted 5 lines
Quoted 1 line

Does this make sense?

I like consistency. If someone absolutely wants a local page then specifying GFP_THISNODE to __alloc_pages is okay. Leave as is I guess.

Agreed.

Quoted 3 lines

What happens though if an MPOL_BIND policy is in effect? The node used must then be the nearest node from the policy mask....

If MPOL_BIND is in effect, the allocation will be filtered based on the current allowed nodemask. If they specify THISNODE and the specified node or current node is not in the mask, I would expect the allocation to fail. Is that unexpected to anybody?

Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#25 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 4 lines · Mel Gorman

If MPOL_BIND is in effect, the allocation will be filtered based on the current allowed nodemask. If they specify THISNODE and the specified node or current node is not in the mask, I would expect the allocation to fail. Is that unexpected to anybody?

Currently GFP_THISNODE with MPOL_BIND results an allocation on the first node.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#26 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

In reply to Christoph Lameter

On (09/11/07 07:45), Christoph Lameter didst pronounce:

Quoted 17 lines · Mel Gorman

Review

Quoted 12 lines

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

(quoted diff) +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

Went back and revisited this. Allocating them at boot-time is below but essentially it is a silly and it makes sense to just have two zonelists where one of them is for __GFP_THISNODE. Implementation wise, this involves dropping the last patch in the set and the overall result is still a reduction in the number of zonelists.

include/linux/gfp.h +1 -5
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/include/linux/gfp.h linux-2.6.24-rc2-mm1-045_use_static_nodemask/include/linux/gfp.h
@@ -175,7 +175,6 @@ FASTCALL(__alloc_pages(gfp_t, unsigned i
 extern struct page *
 FASTCALL(__alloc_pages_nodemask(gfp_t, unsigned int,
 				struct zonelist *, nodemask_t *nodemask));
-extern nodemask_t *nodemask_thisnode(int nid, nodemask_t *nodemask);
 
 static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
 						unsigned int order)
@@ -187,13 +186,10 @@ static inline struct page *alloc_pages_n
 	if (nid < 0)
 		nid = numa_node_id();
 
-	/* Use a temporary nodemask for __GFP_THISNODE allocations */
 	if (unlikely(gfp_mask & __GFP_THISNODE)) {
-		nodemask_t nodemask;
-
 		return __alloc_pages_nodemask(gfp_mask, order,
 				node_zonelist(nid),
-				nodemask_thisnode(nid, &nodemask));
+				NODE_DATA(nid)->nodemask_thisnode);
 	}
 
 	return __alloc_pages(gfp_mask, order, node_zonelist(nid));
include/linux/mmzone.h +3 -0
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/include/linux/mmzone.h linux-2.6.24-rc2-mm1-045_use_static_nodemask/include/linux/mmzone.h
@@ -519,6 +519,9 @@ typedef struct pglist_data {
 	struct zone node_zones[MAX_NR_ZONES];
 	struct zonelist node_zonelist;
 	int nr_zones;
+
+	/* nodemask suitable for __GFP_THISNODE */
+	nodemask_t *nodemask_thisnode;
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 	struct page *node_mem_map;
 #endif
mm/page_alloc.c +22 -13
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/mm/page_alloc.c linux-2.6.24-rc2-mm1-045_use_static_nodemask/mm/page_alloc.c
@@ -1695,28 +1695,36 @@ got_pg:
 }
 
 /* Creates a nodemask suitable for GFP_THISNODE allocations */
-nodemask_t *nodemask_thisnode(int nid, nodemask_t *nodemask)
+static inline void alloc_node_nodemask_thisnode(pg_data_t *pgdat)
 {
-	nodes_clear(*nodemask);
-	node_set(nid, *nodemask);
+	nodemask_t *nodemask_thisnode;
 
-	return nodemask;
+	/* Only a machine with multiple nodes needs the nodemask */
+	if (!NUMA_BUILD || num_online_nodes() == 1)
+		return;
+	
+	/* Allocate the nodemask. Serious if it fails, but not world ending */
+	nodemask_thisnode = alloc_bootmem_node(pgdat, sizeof(nodemask_t));
+	if (!nodemask_thisnode) {
+		printk(KERN_WARNING
+			"thisnode nodemask allocation failed."
+			"There may be sub-optimal NUMA placement.\n");
+		return;
+	}
+
+	/* Initialise the nodemask to only cover the current node */
+	nodes_clear(*nodemask_thisnode);
+	node_set(pgdat->node_id, *nodemask_thisnode);
+	pgdat->nodemask_thisnode = nodemask_thisnode;
 }
 
 struct page * fastcall
 __alloc_pages(gfp_t gfp_mask, unsigned int order,
 		struct zonelist *zonelist)
 {
-	/*
-	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
-	 * cost of allocating on the stack or the stack usage becomes
-	 * noticable, allocate the nodemasks per node at boot or compile time
-	 */
 	if (unlikely(gfp_mask & __GFP_THISNODE)) {
-		nodemask_t nodemask;
-
-		return __alloc_pages_internal(gfp_mask, order,
-			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));
+		return __alloc_pages_internal(gfp_mask, order, zonelist,
+			NODE_DATA(numa_node_id())->nodemask_thisnode);
 	}
 
 	return __alloc_pages_internal(gfp_mask, order, zonelist, NULL);
@@ -3501,6 +3509,7 @@ void __meminit free_area_init_node(int n
 	calculate_node_totalpages(pgdat, zones_size, zholes_size);
 
 	alloc_node_mem_map(pgdat);
+	alloc_node_nodemask_thisnode(pgdat);
 
 	free_area_init_core(pgdat, zones_size, zholes_size);
 }
Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#27 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 19 lines · Mel Gorman

On (09/11/07 07:45), Christoph Lameter didst pronounce:

Quoted 17 lines · Mel Gorman

Review

Quoted 12 lines

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

(quoted diff) +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

Quoted 5 lines

Went back and revisited this. Allocating them at boot-time is below but essentially it is a silly and it makes sense to just have two zonelists where one of them is for __GFP_THISNODE. Implementation wise, this involves dropping the last patch in the set and the overall result is still a reduction in the number of zonelists.

Hi, Mel:

I'll try this out [n 24-rc2-mm1 or later]. I have a series with yet another rework of policy reference counting that will be easier/cleaner atop your series w/o the external zonelist hung off 'BIND policies. So, I'm hoping your series goes into -mm "real soon now".

Question: Just wondering why you didn't embed the '_THISNODE nodemask in the pgdat_t--initialized at boot/node-hot-add time? Perhaps under #ifdef CONFIG_NUMA. The size is known at build time, right? And wouldn't that be way smaller than an additional zonelist?

Lee

Quoted 95 lines

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/include/linux/gfp.h linux-2.6.24-rc2-mm1-045_use_static_nodemask/include/linux/gfp.h

include/linux/gfp.h +1 -5
@@ -175,7 +175,6 @@ FASTCALL(__alloc_pages(gfp_t, unsigned i
 extern struct page *
 FASTCALL(__alloc_pages_nodemask(gfp_t, unsigned int,
 				struct zonelist *, nodemask_t *nodemask));
-extern nodemask_t *nodemask_thisnode(int nid, nodemask_t *nodemask);
 
 static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
 						unsigned int order)
@@ -187,13 +186,10 @@ static inline struct page *alloc_pages_n
 	if (nid < 0)
 		nid = numa_node_id();
 
-	/* Use a temporary nodemask for __GFP_THISNODE allocations */
 	if (unlikely(gfp_mask & __GFP_THISNODE)) {
-		nodemask_t nodemask;
-
 		return __alloc_pages_nodemask(gfp_mask, order,
 				node_zonelist(nid),
-				nodemask_thisnode(nid, &nodemask));
+				NODE_DATA(nid)->nodemask_thisnode);
 	}
 
 	return __alloc_pages(gfp_mask, order, node_zonelist(nid));

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/include/linux/mmzone.h linux-2.6.24-rc2-mm1-045_use_static_nodemask/include/linux/mmzone.h

include/linux/mmzone.h +3 -0
@@ -519,6 +519,9 @@ typedef struct pglist_data {
 	struct zone node_zones[MAX_NR_ZONES];
 	struct zonelist node_zonelist;
 	int nr_zones;
+
+	/* nodemask suitable for __GFP_THISNODE */
+	nodemask_t *nodemask_thisnode;
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 	struct page *node_mem_map;
 #endif

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/mm/page_alloc.c linux-2.6.24-rc2-mm1-045_use_static_nodemask/mm/page_alloc.c

mm/page_alloc.c +21 -13
@@ -1695,28 +1695,36 @@ got_pg:
 }
 
 /* Creates a nodemask suitable for GFP_THISNODE allocations */
-nodemask_t *nodemask_thisnode(int nid, nodemask_t *nodemask)
+static inline void alloc_node_nodemask_thisnode(pg_data_t *pgdat)
 {
-	nodes_clear(*nodemask);
-	node_set(nid, *nodemask);
+	nodemask_t *nodemask_thisnode;
 
-	return nodemask;
+	/* Only a machine with multiple nodes needs the nodemask */
+	if (!NUMA_BUILD || num_online_nodes() == 1)
+		return;
+	
+	/* Allocate the nodemask. Serious if it fails, but not world ending */
+	nodemask_thisnode = alloc_bootmem_node(pgdat, sizeof(nodemask_t));
+	if (!nodemask_thisnode) {
+		printk(KERN_WARNING
+			"thisnode nodemask allocation failed."
+			"There may be sub-optimal NUMA placement.\n");
+		return;
+	}
+
+	/* Initialise the nodemask to only cover the current node */
+	nodes_clear(*nodemask_thisnode);
+	node_set(pgdat->node_id, *nodemask_thisnode);
+	pgdat->nodemask_thisnode = nodemask_thisnode;
 }
 
 struct page * fastcall
 __alloc_pages(gfp_t gfp_mask, unsigned int order,
 		struct zonelist *zonelist)
 {
-	/*
-	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
-	 * cost of allocating on the stack or the stack usage becomes
-	 * noticable, allocate the nodemasks per node at boot or compile time
-	 */
 	if (unlikely(gfp_mask & __GFP_THISNODE)) {
-		nodemask_t nodemask;
-
-		return __alloc_pages_internal(gfp_mask, order,
-			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));
+		return __alloc_pages_internal(gfp_mask, order, zonelist,
+			NODE_DATA(numa_node_id())->nodemask_thisnode);
 	}
 
 	return __alloc_pages_internal(gfp_mask, order, zonelist, NULL);

mm/page_alloc.c void __meminit free_area_init_node(int n · line 3515

Quoted 7 lines
mm/page_alloc.c +1 -0
@@ -3501,6 +3509,7 @@ void __meminit free_area_init_node(int n
 	calculate_node_totalpages(pgdat, zones_size, zholes_size);
 
 	alloc_node_mem_map(pgdat);
+	alloc_node_nodemask_thisnode(pgdat);
 
 	free_area_init_core(pgdat, zones_size, zholes_size);
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#28 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

On (20/11/07 10:14), Lee Schermerhorn didst pronounce:

Quoted 26 lines · Mel Gorman
Quoted 19 lines

On (09/11/07 07:45), Christoph Lameter didst pronounce:

Quoted 17 lines · Mel Gorman

Review

Quoted 12 lines

struct page * fastcall __alloc_pages(gfp_t gfp_mask, unsigned int order,

(quoted diff) +7 -0
 		struct zonelist *zonelist)
 {
+	/*
+	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
+	 * cost of allocating on the stack or the stack usage becomes
+	 * noticable, allocate the nodemasks per node at boot or compile time
+	 */
+	if (unlikely(gfp_mask & __GFP_THISNODE)) {
+		nodemask_t nodemask;

Hmmm.. This places a potentially big structure on the stack. nodemask can contain up to 1024 bits which means 128 bytes. Maybe keep an array of gfp_thisnode nodemasks (node_nodemask?) and use node_nodemask[nid]?

Quoted 5 lines

Went back and revisited this. Allocating them at boot-time is below but essentially it is a silly and it makes sense to just have two zonelists where one of them is for __GFP_THISNODE. Implementation wise, this involves dropping the last patch in the set and the overall result is still a reduction in the number of zonelists.

Quoted 3 lines

Hi, Mel:

I'll try this out [n 24-rc2-mm1 or later].

Hold off testing for the moment. Getting all the corner cases right for __GFP_THISNODE has turned too complicated to be considered as part of a larger patchset. I believe it makes sense to drop the final patch and settle with having two zonelists. One of these will be for __GFP_THISNODE allocations. We can then tackle removing that zonelist at a later date.

This will still remove the hack, reduce the number of zonelists and improve how MPOL_BIND policies are applied so it is still an overall win.

I should have a revised patchset with the final one dropped posted in a few hours.

Quoted 5 lines

I have a series with yet
another rework of policy reference counting that will be easier/cleaner atop your series w/o the external zonelist hung off 'BIND policies. So, I'm hoping your series goes into -mm "real soon now".

Sounds good.

Quoted 2 lines

Question: Just wondering why you didn't embed the '_THISNODE nodemask in the pgdat_t--initialized at boot/node-hot-add time?

At the time, to save space in the pg_data_t structure but it is not worth the complexity.

Quoted 4 lines

Perhaps under
#ifdef CONFIG_NUMA. The size is known at build time, right? And wouldn't that be way smaller than an additional zonelist?

Yes it would.

Quoted 105 lines

Lee

Quoted 102 lines

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/include/linux/gfp.h linux-2.6.24-rc2-mm1-045_use_static_nodemask/include/linux/gfp.h

include/linux/gfp.h +1 -5
@@ -175,7 +175,6 @@ FASTCALL(__alloc_pages(gfp_t, unsigned i
 extern struct page *
 FASTCALL(__alloc_pages_nodemask(gfp_t, unsigned int,
 				struct zonelist *, nodemask_t *nodemask));
-extern nodemask_t *nodemask_thisnode(int nid, nodemask_t *nodemask);
 
 static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
 						unsigned int order)
@@ -187,13 +186,10 @@ static inline struct page *alloc_pages_n
 	if (nid < 0)
 		nid = numa_node_id();
 
-	/* Use a temporary nodemask for __GFP_THISNODE allocations */
 	if (unlikely(gfp_mask & __GFP_THISNODE)) {
-		nodemask_t nodemask;
-
 		return __alloc_pages_nodemask(gfp_mask, order,
 				node_zonelist(nid),
-				nodemask_thisnode(nid, &nodemask));
+				NODE_DATA(nid)->nodemask_thisnode);
 	}
 
 	return __alloc_pages(gfp_mask, order, node_zonelist(nid));

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/include/linux/mmzone.h linux-2.6.24-rc2-mm1-045_use_static_nodemask/include/linux/mmzone.h

include/linux/mmzone.h +3 -0
@@ -519,6 +519,9 @@ typedef struct pglist_data {
 	struct zone node_zones[MAX_NR_ZONES];
 	struct zonelist node_zonelist;
 	int nr_zones;
+
+	/* nodemask suitable for __GFP_THISNODE */
+	nodemask_t *nodemask_thisnode;
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
 	struct page *node_mem_map;
 #endif

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.24-rc2-mm1-040_use_one_zonelist/mm/page_alloc.c linux-2.6.24-rc2-mm1-045_use_static_nodemask/mm/page_alloc.c

mm/page_alloc.c +22 -13
@@ -1695,28 +1695,36 @@ got_pg:
 }
 
 /* Creates a nodemask suitable for GFP_THISNODE allocations */
-nodemask_t *nodemask_thisnode(int nid, nodemask_t *nodemask)
+static inline void alloc_node_nodemask_thisnode(pg_data_t *pgdat)
 {
-	nodes_clear(*nodemask);
-	node_set(nid, *nodemask);
+	nodemask_t *nodemask_thisnode;
 
-	return nodemask;
+	/* Only a machine with multiple nodes needs the nodemask */
+	if (!NUMA_BUILD || num_online_nodes() == 1)
+		return;
+	
+	/* Allocate the nodemask. Serious if it fails, but not world ending */
+	nodemask_thisnode = alloc_bootmem_node(pgdat, sizeof(nodemask_t));
+	if (!nodemask_thisnode) {
+		printk(KERN_WARNING
+			"thisnode nodemask allocation failed."
+			"There may be sub-optimal NUMA placement.\n");
+		return;
+	}
+
+	/* Initialise the nodemask to only cover the current node */
+	nodes_clear(*nodemask_thisnode);
+	node_set(pgdat->node_id, *nodemask_thisnode);
+	pgdat->nodemask_thisnode = nodemask_thisnode;
 }
 
 struct page * fastcall
 __alloc_pages(gfp_t gfp_mask, unsigned int order,
 		struct zonelist *zonelist)
 {
-	/*
-	 * Use a temporary nodemask for __GFP_THISNODE allocations. If the
-	 * cost of allocating on the stack or the stack usage becomes
-	 * noticable, allocate the nodemasks per node at boot or compile time
-	 */
 	if (unlikely(gfp_mask & __GFP_THISNODE)) {
-		nodemask_t nodemask;
-
-		return __alloc_pages_internal(gfp_mask, order,
-			zonelist, nodemask_thisnode(numa_node_id(), &nodemask));
+		return __alloc_pages_internal(gfp_mask, order, zonelist,
+			NODE_DATA(numa_node_id())->nodemask_thisnode);
 	}
 
 	return __alloc_pages_internal(gfp_mask, order, zonelist, NULL);
@@ -3501,6 +3509,7 @@ void __meminit free_area_init_node(int n
 	calculate_node_totalpages(pgdat, zones_size, zholes_size);
 
 	alloc_node_mem_map(pgdat);
+	alloc_node_nodemask_thisnode(pgdat);
 
 	free_area_init_core(pgdat, zones_size, zholes_size);
 }
Signature · 10 lines
-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#29 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 5 lines · Mel Gorman

Hold off testing for the moment. Getting all the corner cases right for __GFP_THISNODE has turned too complicated to be considered as part of a larger patchset. I believe it makes sense to drop the final patch and settle with having two zonelists. One of these will be for __GFP_THISNODE allocations. We can then tackle removing that zonelist at a later date.

Ack.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#30 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

In reply to Mel Gorman
Quoted 5 lines · Mel Gorman

Went back and revisited this. Allocating them at boot-time is below but essentially it is a silly and it makes sense to just have two zonelists where one of them is for __GFP_THISNODE. Implementation wise, this involves dropping the last patch in the set and the overall result is still a reduction in the number of zonelists.

Allright with me. Andrew could we get this patchset merged?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#31 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

On (20/11/07 12:18), Christoph Lameter didst pronounce:

Quoted 10 lines · Mel Gorman
Quoted 6 lines

Went back and revisited this. Allocating them at boot-time is below but essentially it is a silly and it makes sense to just have two zonelists where one of them is for __GFP_THISNODE. Implementation wise, this involves dropping the last patch in the set and the overall result is still a reduction in the number of zonelists.

Allright with me. Andrew could we get this patchset merged?

They will not merge cleanly with a recent tree. I expect to be ready to post a new set when regression tests complete later this evening.

Signature · 9 lines
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
maintainer
#32 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

In reply to Christoph Lameter
Quoted 9 lines · On Tue, 20 Nov 2007 12:18:10 -0800 (PST) Christoph Lameter <[email protected]>…
Quoted 5 lines · Mel Gorman

Went back and revisited this. Allocating them at boot-time is below but essentially it is a silly and it makes sense to just have two zonelists where one of them is for __GFP_THISNODE. Implementation wise, this involves dropping the last patch in the set and the overall result is still a reduction in the number of zonelists.

Allright with me. Andrew could we get this patchset merged?

uhm, maybe. It's getting toward the time when we should try to get -mm vaguely compiling and booting on some machines, which means stopping merging new stuff. I left that too late in the 2.6.23 cycle.

otoh it'd be nice to get mainline fixed up a bit too :(

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#33 lore raw

Re: [PATCH 6/6] Use one zonelist that is filtered by nodemask

Quoted 3 lines · Andrew Morton

uhm, maybe. It's getting toward the time when we should try to get -mm vaguely compiling and booting on some machines, which means stopping merging new stuff. I left that too late in the 2.6.23 cycle.

Huh? mm1 works fine here.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>