hugetlb: numa control of persistent huge pages alloc/free

[PATCH 1/5] hugetlb: rework hstate_next_node* functions

Against: 2.6.31-rc6-mmotm-090820-1918

V2:
+ cleaned up comments, removed some deemed unnecessary,
add some suggested by review
+ removed check for !current in huge_mpol_nodes_allowed().
+ added 'current->comm' to warning message in huge_mpol_nodes_allowed().
+ added VM_BUG_ON() assertion in hugetlb.c next_node_allowed() to catch out of range node id.
+ add examples to patch description

V3:
+ factored this "cleanup" patch out of V2 patch 2/3
+ moved ahead of patch to add nodes_allowed mask to alloc funcs as this patch is somewhat independent from using task mempolicy to control huge page allocation and freeing.

Modify the hstate_next_node* functions to allow them to be called to obtain the "start_nid". Then, whereas prior to this patch we unconditionally called hstate_next_node_to_{alloc|free}(), whether or not we successfully allocated/freed a huge page on the node, now we only call these functions on failure to alloc/free to advance to next allowed node.

Factor out the next_node_allowed() function to handle wrap at end of node_online_map. In this version, the allowed nodes include all of the online nodes.

Reviewed-by Mel Gorman <[email protected]>

Signed-off-by Lee Schermerhorn <[email protected]>

[PATCH 2/4] hugetlb: add nodemask arg to huge page alloc, free and surplus adjust fcns

Against: 2.6.31-rc6-mmotm-090820-1918

V3:
+ moved this patch to after the "rework" of hstate_next_node_to_... functions as this patch is more specific to using task mempolicy to control huge page allocation and freeing.

In preparation for constraining huge page allocation and freeing by the controlling task's numa mempolicy, add a "nodes_allowed" nodemask pointer to the allocate, free and surplus adjustment functions. For now, pass NULL to indicate default behavior--i.e., use node_online_map. A subsqeuent patch will derive a non-default mask from the controlling task's numa mempolicy.

Reviewed-by Mel Gorman <[email protected]>

Signed-off-by Lee Schermerhorn <[email protected]>

 mm/hugetlb.c |  102 ++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 67 insertions(+), 35 deletions(-)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

mm/hugetlb.c +8 -5

@@ -622,19 +622,29 @@ static struct page *alloc_fresh_huge_pag
 }
 
 /*
- * common helper function for hstate_next_node_to_{alloc|free}.
- * return next node in node_online_map, wrapping at end.
+ * common helper functions for hstate_next_node_to_{alloc|free}.
+ * We may have allocated or freed a huge pages based on a different
+ * nodes_allowed, previously, so h->next_node_to_{alloc|free} might
+ * be outside of *nodes_allowed.  Ensure that we use the next
+ * allowed node for alloc or free.
  */
-static int next_node_allowed(int nid)
+static int next_node_allowed(int nid, nodemask_t *nodes_allowed)
 {
-	nid = next_node(nid, node_online_map);
+	nid = next_node(nid, *nodes_allowed);
 	if (nid == MAX_NUMNODES)
-		nid = first_node(node_online_map);
+		nid = first_node(*nodes_allowed);
 	VM_BUG_ON(nid >= MAX_NUMNODES);

mm/hugetlb.c +6 -6

+
 /*
  * Use a helper variable to find the next node and then
  * copy it back to next_nid_to_alloc afterwards:

@@ -642,28 +652,34 @@ static int next_node_allowed(int nid)
  * pass invalid nid MAX_NUMNODES to alloc_pages_exact_node.
  * But we don't need to use a spin_lock here: it really
  * doesn't matter if occasionally a racer chooses the
- * same nid as we do.  Move nid forward in the mask even
- * if we just successfully allocated a hugepage so that
- * the next caller gets hugepages on the next node.
+ * same nid as we do.  Move nid forward in the mask whether
+ * or not we just successfully allocated a hugepage so that
+ * the next allocation addresses the next node.
  */
-static int hstate_next_node_to_alloc(struct hstate *h)
+static int hstate_next_node_to_alloc(struct hstate *h,
+					nodemask_t *nodes_allowed)
 {
 	int nid, next_nid;
 
-	nid = h->next_nid_to_alloc;
-	next_nid = next_node_allowed(nid);

mm/hugetlb.c +4 -6

-static int alloc_fresh_huge_page(struct hstate *h)
+static int alloc_fresh_huge_page(struct hstate *h, nodemask_t *nodes_allowed)
 {
 	struct page *page;
 	int start_nid;
 	int next_nid;
 	int ret = 0;
 
-	start_nid = hstate_next_node_to_alloc(h);
+	start_nid = hstate_next_node_to_alloc(h, nodes_allowed);
 	next_nid = start_nid;
 
 	do {

@@ -672,7 +688,7 @@ static int alloc_fresh_huge_page(struct
 			ret = 1;
 			break;
 		}
-		next_nid = hstate_next_node_to_alloc(h);
+		next_nid = hstate_next_node_to_alloc(h, nodes_allowed);
 	} while (next_nid != start_nid);
 
 	if (ret)

@@ -689,13 +705,18 @@ static int alloc_fresh_huge_page(struct
  * whether or not we find a free huge page to free so that the
  * next attempt to free addresses the next node.
  */
-static int hstate_next_node_to_free(struct hstate *h)
+static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed)
 {
 	int nid, next_nid;
 
-	nid = h->next_nid_to_free;
-	next_nid = next_node_allowed(nid);

Quoted 51 lines · Lee Schermerhorn

[PATCH 2/4] hugetlb: add nodemask arg to huge page alloc, free and surplus adjust fcns

Against: 2.6.31-rc6-mmotm-090820-1918

V3:
+ moved this patch to after the "rework" of hstate_next_node_to_... functions as this patch is more specific to using task mempolicy to control huge page allocation and freeing.

In preparation for constraining huge page allocation and freeing by the controlling task's numa mempolicy, add a "nodes_allowed" nodemask pointer to the allocate, free and surplus adjustment functions. For now, pass NULL to indicate default behavior--i.e., use node_online_map. A subsqeuent patch will derive a non-default mask from the controlling task's numa mempolicy.

Reviewed-by Mel Gorman <[email protected]>

Signed-off-by Lee Schermerhorn <[email protected]>

 mm/hugetlb.c |  102 ++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 67 insertions(+), 35 deletions(-)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

mm/hugetlb.c +8 -5

@@ -622,19 +622,29 @@ static struct page *alloc_fresh_huge_pag
 }
 
 /*
- * common helper function for hstate_next_node_to_{alloc|free}.
- * return next node in node_online_map, wrapping at end.
+ * common helper functions for hstate_next_node_to_{alloc|free}.
+ * We may have allocated or freed a huge pages based on a different
+ * nodes_allowed, previously, so h->next_node_to_{alloc|free} might
+ * be outside of *nodes_allowed.  Ensure that we use the next
+ * allowed node for alloc or free.
  */
-static int next_node_allowed(int nid)
+static int next_node_allowed(int nid, nodemask_t *nodes_allowed)
 {
-	nid = next_node(nid, node_online_map);
+	nid = next_node(nid, *nodes_allowed);
 	if (nid == MAX_NUMNODES)
-		nid = first_node(node_online_map);
+		nid = first_node(*nodes_allowed);
 	VM_BUG_ON(nid >= MAX_NUMNODES);
 
 	return nid;
 }
 

mm/hugetlb.c static struct page *alloc_fresh_huge_pag · line 646

Quoted 6 lines

mm/hugetlb.c +6 -0

+static int this_node_allowed(int nid, nodemask_t *nodes_allowed)
+{
+	if (!node_isset(nid, *nodes_allowed))
+		nid = next_node_allowed(nid, nodes_allowed);
+	return nid;
+}

Awkward name considering this doesn't simply return true or false as expected, it returns a nid.

Quoted 22 lines

mm/hugetlb.c +6 -4

+
 /*
  * Use a helper variable to find the next node and then
  * copy it back to next_nid_to_alloc afterwards:

@@ -642,28 +652,34 @@ static int next_node_allowed(int nid)
  * pass invalid nid MAX_NUMNODES to alloc_pages_exact_node.
  * But we don't need to use a spin_lock here: it really
  * doesn't matter if occasionally a racer chooses the
- * same nid as we do.  Move nid forward in the mask even
- * if we just successfully allocated a hugepage so that
- * the next caller gets hugepages on the next node.
+ * same nid as we do.  Move nid forward in the mask whether
+ * or not we just successfully allocated a hugepage so that
+ * the next allocation addresses the next node.
  */
-static int hstate_next_node_to_alloc(struct hstate *h)
+static int hstate_next_node_to_alloc(struct hstate *h,
+					nodemask_t *nodes_allowed)
 {
 	int nid, next_nid;
 

mm/hugetlb.c static int next_node_allowed(int nid) · line 673

Quoted 12 lines

mm/hugetlb.c +7 -2

-	nid = h->next_nid_to_alloc;
-	next_nid = next_node_allowed(nid);
+	if (!nodes_allowed)
+		nodes_allowed = &node_online_map;
+
+	nid = this_node_allowed(h->next_nid_to_alloc, nodes_allowed);
+
+	next_nid = next_node_allowed(nid, nodes_allowed);
 	h->next_nid_to_alloc = next_nid;
+
 	return nid;
 }

Don't need next_nid.

Quoted 32 lines

mm/hugetlb.c +4 -4

-static int alloc_fresh_huge_page(struct hstate *h)
+static int alloc_fresh_huge_page(struct hstate *h, nodemask_t *nodes_allowed)
 {
 	struct page *page;
 	int start_nid;
 	int next_nid;
 	int ret = 0;
 
-	start_nid = hstate_next_node_to_alloc(h);
+	start_nid = hstate_next_node_to_alloc(h, nodes_allowed);
 	next_nid = start_nid;
 
 	do {

@@ -672,7 +688,7 @@ static int alloc_fresh_huge_page(struct
 			ret = 1;
 			break;
 		}
-		next_nid = hstate_next_node_to_alloc(h);
+		next_nid = hstate_next_node_to_alloc(h, nodes_allowed);
 	} while (next_nid != start_nid);
 
 	if (ret)

@@ -689,13 +705,18 @@ static int alloc_fresh_huge_page(struct
  * whether or not we find a free huge page to free so that the
  * next attempt to free addresses the next node.
  */
-static int hstate_next_node_to_free(struct hstate *h)
+static int hstate_next_node_to_free(struct hstate *h, nodemask_t *nodes_allowed)
 {
 	int nid, next_nid;
 

mm/hugetlb.c static int alloc_fresh_huge_page(struct · line 721

Quoted 12 lines

mm/hugetlb.c +7 -2

-	nid = h->next_nid_to_free;
-	next_nid = next_node_allowed(nid);
+	if (!nodes_allowed)
+		nodes_allowed = &node_online_map;
+
+	nid = this_node_allowed(h->next_nid_to_free, nodes_allowed);
+
+	next_nid = next_node_allowed(nid, nodes_allowed);
 	h->next_nid_to_free = next_nid;
+
 	return nid;
 }

Same.

Quoted 25 lines

Quoted 25 lines

(quoted diff) +8 -5

@@ -622,19 +622,29 @@ static struct page *alloc_fresh_huge_pag
 }
 
 /*
- * common helper function for hstate_next_node_to_{alloc|free}.
- * return next node in node_online_map, wrapping at end.
+ * common helper functions for hstate_next_node_to_{alloc|free}.
+ * We may have allocated or freed a huge pages based on a different
+ * nodes_allowed, previously, so h->next_node_to_{alloc|free} might
+ * be outside of *nodes_allowed.  Ensure that we use the next
+ * allowed node for alloc or free.
  */
-static int next_node_allowed(int nid)
+static int next_node_allowed(int nid, nodemask_t *nodes_allowed)
 {
-	nid = next_node(nid, node_online_map);
+	nid = next_node(nid, *nodes_allowed);
 	if (nid == MAX_NUMNODES)
-		nid = first_node(node_online_map);
+		nid = first_node(*nodes_allowed);
 	VM_BUG_ON(nid >= MAX_NUMNODES);
 
 	return nid;
 }
 

Quoted 9 lines

Review static struct page *alloc_fresh_huge_pag · line 646

Quoted 6 lines

(quoted diff) +6 -0

+static int this_node_allowed(int nid, nodemask_t *nodes_allowed)
+{
+	if (!node_isset(nid, *nodes_allowed))
+		nid = next_node_allowed(nid, nodes_allowed);
+	return nid;
+}

Awkward name considering this doesn't simply return true or false as expected, it returns a nid.

Well, it's not a predicate function so I wouldn't expect true or false return, but I can see how the trailing "allowed" can sound like we're asking the question "Is this node allowed?". Maybe, "get_this_node_allowed()" or "get_start_node_allowed" [we return the nid to "startnid"], ... Or, do you have a suggestion?

Quoted 22 lines

Quoted 22 lines

(quoted diff) +6 -4

+
 /*
  * Use a helper variable to find the next node and then
  * copy it back to next_nid_to_alloc afterwards:

@@ -642,28 +652,34 @@ static int next_node_allowed(int nid)
  * pass invalid nid MAX_NUMNODES to alloc_pages_exact_node.
  * But we don't need to use a spin_lock here: it really
  * doesn't matter if occasionally a racer chooses the
- * same nid as we do.  Move nid forward in the mask even
- * if we just successfully allocated a hugepage so that
- * the next caller gets hugepages on the next node.
+ * same nid as we do.  Move nid forward in the mask whether
+ * or not we just successfully allocated a hugepage so that
+ * the next allocation addresses the next node.
  */
-static int hstate_next_node_to_alloc(struct hstate *h)
+static int hstate_next_node_to_alloc(struct hstate *h,
+					nodemask_t *nodes_allowed)
 {
 	int nid, next_nid;
 

Quoted 14 lines

Review static int next_node_allowed(int nid) · line 673

Quoted 12 lines

(quoted diff) +7 -2

-	nid = h->next_nid_to_alloc;
-	next_nid = next_node_allowed(nid);
+	if (!nodes_allowed)
+		nodes_allowed = &node_online_map;
+
+	nid = this_node_allowed(h->next_nid_to_alloc, nodes_allowed);
+
+	next_nid = next_node_allowed(nid, nodes_allowed);
 	h->next_nid_to_alloc = next_nid;
+
 	return nid;
 }

Don't need next_nid.

Well, the pre-existing comment block indicated that the use of the apparently spurious next_nid variable is necessary to close a race. Not sure whether that comment still applies with this rework. What do you think?

Quoted 6 lines

Quoted 6 lines

<SNIP>

mm/hugetlb.c +2 -0

+static int hstate_next_node_to_alloc(struct hstate *h,
+					nodemask_t *nodes_allowed)
 {
 	int nid, next_nid;

Quoted 14 lines

mm/hugetlb.c static int next_node_allowed(int nid) · line 673

Quoted 12 lines

mm/hugetlb.c +7 -2

-	nid = h->next_nid_to_alloc;
-	next_nid = next_node_allowed(nid);
+	if (!nodes_allowed)
+		nodes_allowed = &node_online_map;
+
+	nid = this_node_allowed(h->next_nid_to_alloc, nodes_allowed);
+
+	next_nid = next_node_allowed(nid, nodes_allowed);
 	h->next_nid_to_alloc = next_nid;
+
 	return nid;
 }

Don't need next_nid.

Well, the pre-existing comment block indicated that the use of the apparently spurious next_nid variable is necessary to close a race. Not sure whether that comment still applies with this rework. What do you think?

This patch derives a "nodes_allowed" node mask from the numa mempolicy of the task modifying the number of persistent huge pages to control the allocation, freeing and adjusting of surplus huge pages. This mask is derived as follows:

* For "default" [NULL] task mempolicy, a NULL nodemask_t pointer is produced. This will cause the hugetlb subsystem to use node_online_map as the "nodes_allowed". This preserves the behavior before this patch.
* For "preferred" mempolicy, including explicit local allocation, a nodemask with the single preferred node will be produced. "local" policy will NOT track any internode migrations of the task adjusting nr_hugepages.
* For "bind" and "interleave" policy, the mempolicy's nodemask will be used.
* Other than to inform the construction of the nodes_allowed node mask, the actual mempolicy mode is ignored. That is, all modes behave like interleave over the resulting nodes_allowed mask with no "fallback".

Notes:

1) This patch introduces a subtle change in behavior: huge page allocation and freeing will be constrained by any mempolicy that the task adjusting the huge page pool inherits from its parent. This policy could come from a distant ancestor. The adminstrator adjusting the huge page pool without explicitly specifying a mempolicy via numactl might be surprised by this. Additionaly, any mempolicy specified by numactl will be constrained by the cpuset in which numactl is invoked.

2) Hugepages allocated at boot time use the node_online_map. An additional patch could implement a temporary boot time huge pages nodes_allowed command line parameter.

3) Using mempolicy to control persistent huge page allocation and freeing requires no change to hugeadm when invoking it via numactl, as shown in the examples below. However, hugeadm could be enhanced to take the allowed nodes as an argument and set its task mempolicy itself. This would allow it to detect and warn about any non-default mempolicy that it inherited from its parent, thus alleviating the issue described in Note 1 above.

See the updated documentation [next patch] for more information about the implications of this patch.

Examples:

Starting with:

	Node 0 HugePages_Total:     0
	Node 1 HugePages_Total:     0
	Node 2 HugePages_Total:     0
	Node 3 HugePages_Total:     0

Default behavior [with or without this patch] balances persistent hugepage allocation across nodes [with sufficient contiguous memory]:

	hugeadm --pool-pages-min=2048Kb:32

yields:

	Node 0 HugePages_Total:     8
	Node 1 HugePages_Total:     8
	Node 2 HugePages_Total:     8
	Node 3 HugePages_Total:     8

Applying mempolicy--e.g., with numactl [using '-m' a.k.a. '--membind' because it allows multiple nodes to be specified and it's easy to type]--we can allocate huge pages on individual nodes or sets of nodes. So, starting from the condition above, with 8 huge pages per node:

	numactl -m 2 hugeadm --pool-pages-min=2048Kb:+8

yields:

Node 0 HugePages_Total: 8
Node 1 HugePages_Total: 8
Node 2 HugePages_Total: 16
Node 3 HugePages_Total: 8

The incremental 8 huge pages were restricted to node 2 by the specified mempolicy.

Similarly, we can use mempolicy to free persistent huge pages from specified nodes:

	numactl -m 0,1 hugeadm --pool-pages-min=2048Kb:-8

yields:

Node 0 HugePages_Total: 4
Node 1 HugePages_Total: 4
Node 2 HugePages_Total: 16
Node 3 HugePages_Total: 8

The 8 huge pages freed were balanced over nodes 0 and 1.

Signed-off-by Lee Schermerhorn <[email protected]>

 include/linux/mempolicy.h |    3 ++
 mm/hugetlb.c              |   14 ++++++----
 mm/mempolicy.c            |   61 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 5 deletions(-)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/mempolicy.c
===================================================================

mm/mempolicy.c +23 -0

@@ -1564,6 +1564,67 @@ struct zonelist *huge_zonelist(struct vm
 	}
 	return zl;
 }
+
+/*
+ * huge_mpol_nodes_allowed -- mempolicy extension for huge pages.
+ *
+ * Returns a [pointer to a] nodelist based on the current task's mempolicy
+ * to constraing the allocation and freeing of persistent huge pages
+ * 'Preferred', 'local' and 'interleave' mempolicy will behave more like
+ * 'bind' policy in this context.  An attempt to allocate a persistent huge
+ * page will never "fallback" to another node inside the buddy system
+ * allocator.
+ *
+ * If the task's mempolicy is "default" [NULL], just return NULL for
+ * default behavior.  Otherwise, extract the policy nodemask for 'bind'
+ * or 'interleave' policy or construct a nodemask for 'preferred' or
+ * 'local' policy and return a pointer to a kmalloc()ed nodemask_t.
+ *
+ * N.B., it is the caller's responsibility to free a returned nodemask.
+ */
+nodemask_t *huge_mpol_nodes_allowed(void)
+{
+	nodemask_t *nodes_allowed = NULL;
+	struct mempolicy *mempolicy;
+	int nid;

mm/mempolicy.c +18 -0

+		goto out;
+	}
+	nodes_clear(*nodes_allowed);
+
+	mempolicy = current->mempolicy;
+	switch (mempolicy->mode) {
+	case MPOL_PREFERRED:
+		if (mempolicy->flags & MPOL_F_LOCAL)
+			nid = numa_node_id();
+		else
+			nid = mempolicy->v.preferred_node;
+		node_set(nid, *nodes_allowed);
+		break;
+
+	case MPOL_BIND:
+		/* Fall through */
+	case MPOL_INTERLEAVE:
+		*nodes_allowed =  mempolicy->v.nodes;

mm/mempolicy.c +0 -0

 #endif
 
 /* Allocate a page in interleaved policy.

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/mempolicy.h
===================================================================

include/linux/mempolicy.h +3 -0

@@ -201,6 +201,7 @@ extern void mpol_fix_fork_child_flag(str
 extern struct zonelist *huge_zonelist(struct vm_area_struct *vma,
 				unsigned long addr, gfp_t gfp_flags,
 				struct mempolicy **mpol, nodemask_t **nodemask);
+extern nodemask_t *huge_mpol_nodes_allowed(void);
 extern unsigned slab_node(struct mempolicy *policy);
 
 extern enum zone_type policy_zone;

@@ -328,6 +329,8 @@ static inline struct zonelist *huge_zone
 	return node_zonelist(0, gfp_flags);
 }
 
+static inline nodemask_t *huge_mpol_nodes_allowed(void) { return NULL; }
+
 static inline int do_migrate_pages(struct mm_struct *mm,
 			const nodemask_t *from_nodes,
 			const nodemask_t *to_nodes, int flags)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

Quoted 142 lines · Lee Schermerhorn

This patch derives a "nodes_allowed" node mask from the numa mempolicy of the task modifying the number of persistent huge pages to control the allocation, freeing and adjusting of surplus huge pages. This mask is derived as follows:

* For "default" [NULL] task mempolicy, a NULL nodemask_t pointer is produced. This will cause the hugetlb subsystem to use node_online_map as the "nodes_allowed". This preserves the behavior before this patch.
* For "preferred" mempolicy, including explicit local allocation, a nodemask with the single preferred node will be produced. "local" policy will NOT track any internode migrations of the task adjusting nr_hugepages.
* For "bind" and "interleave" policy, the mempolicy's nodemask will be used.
* Other than to inform the construction of the nodes_allowed node mask, the actual mempolicy mode is ignored. That is, all modes behave like interleave over the resulting nodes_allowed mask with no "fallback".

Notes:

1) This patch introduces a subtle change in behavior: huge page allocation and freeing will be constrained by any mempolicy that the task adjusting the huge page pool inherits from its parent. This policy could come from a distant ancestor. The adminstrator adjusting the huge page pool without explicitly specifying a mempolicy via numactl might be surprised by this. Additionaly, any mempolicy specified by numactl will be constrained by the cpuset in which numactl is invoked.

2) Hugepages allocated at boot time use the node_online_map. An additional patch could implement a temporary boot time huge pages nodes_allowed command line parameter.

3) Using mempolicy to control persistent huge page allocation and freeing requires no change to hugeadm when invoking it via numactl, as shown in the examples below. However, hugeadm could be enhanced to take the allowed nodes as an argument and set its task mempolicy itself. This would allow it to detect and warn about any non-default mempolicy that it inherited from its parent, thus alleviating the issue described in Note 1 above.

See the updated documentation [next patch] for more information about the implications of this patch.

Examples:

Starting with:

	Node 0 HugePages_Total:     0
	Node 1 HugePages_Total:     0
	Node 2 HugePages_Total:     0
	Node 3 HugePages_Total:     0

Default behavior [with or without this patch] balances persistent hugepage allocation across nodes [with sufficient contiguous memory]:

	hugeadm --pool-pages-min=2048Kb:32

yields:

	Node 0 HugePages_Total:     8
	Node 1 HugePages_Total:     8
	Node 2 HugePages_Total:     8
	Node 3 HugePages_Total:     8

Applying mempolicy--e.g., with numactl [using '-m' a.k.a. '--membind' because it allows multiple nodes to be specified and it's easy to type]--we can allocate huge pages on individual nodes or sets of nodes. So, starting from the condition above, with 8 huge pages per node:

	numactl -m 2 hugeadm --pool-pages-min=2048Kb:+8

yields:

Node 0 HugePages_Total: 8
Node 1 HugePages_Total: 8
Node 2 HugePages_Total: 16
Node 3 HugePages_Total: 8

The incremental 8 huge pages were restricted to node 2 by the specified mempolicy.

Similarly, we can use mempolicy to free persistent huge pages from specified nodes:

	numactl -m 0,1 hugeadm --pool-pages-min=2048Kb:-8

yields:

Node 0 HugePages_Total: 4
Node 1 HugePages_Total: 4
Node 2 HugePages_Total: 16
Node 3 HugePages_Total: 8

The 8 huge pages freed were balanced over nodes 0 and 1.

Signed-off-by Lee Schermerhorn <[email protected]>

 include/linux/mempolicy.h |    3 ++
 mm/hugetlb.c              |   14 ++++++----
 mm/mempolicy.c            |   61 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 5 deletions(-)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/mempolicy.c
===================================================================

mm/mempolicy.c +27 -0

@@ -1564,6 +1564,67 @@ struct zonelist *huge_zonelist(struct vm
 	}
 	return zl;
 }
+
+/*
+ * huge_mpol_nodes_allowed -- mempolicy extension for huge pages.
+ *
+ * Returns a [pointer to a] nodelist based on the current task's mempolicy
+ * to constraing the allocation and freeing of persistent huge pages
+ * 'Preferred', 'local' and 'interleave' mempolicy will behave more like
+ * 'bind' policy in this context.  An attempt to allocate a persistent huge
+ * page will never "fallback" to another node inside the buddy system
+ * allocator.
+ *
+ * If the task's mempolicy is "default" [NULL], just return NULL for
+ * default behavior.  Otherwise, extract the policy nodemask for 'bind'
+ * or 'interleave' policy or construct a nodemask for 'preferred' or
+ * 'local' policy and return a pointer to a kmalloc()ed nodemask_t.
+ *
+ * N.B., it is the caller's responsibility to free a returned nodemask.
+ */
+nodemask_t *huge_mpol_nodes_allowed(void)
+{
+	nodemask_t *nodes_allowed = NULL;
+	struct mempolicy *mempolicy;
+	int nid;
+
+	if (!current->mempolicy)
+		return NULL;
+

mm/mempolicy.c struct zonelist *huge_zonelist(struct vm · line 1599

Quoted 6 lines

mm/mempolicy.c +6 -0

+	mpol_get(current->mempolicy);
+	nodes_allowed = kmalloc(sizeof(*nodes_allowed), GFP_KERNEL);
+	if (!nodes_allowed) {
+		printk(KERN_WARNING "%s unable to allocate nodes allowed mask "
+			"for huge page allocation.\nFalling back to default.\n",
+			current->comm);

I don't think using '\n' inside printk's is allowed anymore.

Quoted 19 lines

mm/mempolicy.c +18 -0

+		goto out;
+	}
+	nodes_clear(*nodes_allowed);
+
+	mempolicy = current->mempolicy;
+	switch (mempolicy->mode) {
+	case MPOL_PREFERRED:
+		if (mempolicy->flags & MPOL_F_LOCAL)
+			nid = numa_node_id();
+		else
+			nid = mempolicy->v.preferred_node;
+		node_set(nid, *nodes_allowed);
+		break;
+
+	case MPOL_BIND:
+		/* Fall through */
+	case MPOL_INTERLEAVE:
+		*nodes_allowed =  mempolicy->v.nodes;

mm/mempolicy.c struct zonelist *huge_zonelist(struct vm · line 1627

Quoted 10 lines

mm/mempolicy.c +10 -0

+		break;
+
+	default:
+		BUG();
+	}
+
+out:
+	mpol_put(current->mempolicy);
+	return nodes_allowed;
+}

This should be all unnecessary, see below.

Quoted 29 lines

mm/mempolicy.c +0 -0

 #endif
 
 /* Allocate a page in interleaved policy.

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/mempolicy.h
===================================================================

include/linux/mempolicy.h +3 -0

@@ -201,6 +201,7 @@ extern void mpol_fix_fork_child_flag(str
 extern struct zonelist *huge_zonelist(struct vm_area_struct *vma,
 				unsigned long addr, gfp_t gfp_flags,
 				struct mempolicy **mpol, nodemask_t **nodemask);
+extern nodemask_t *huge_mpol_nodes_allowed(void);
 extern unsigned slab_node(struct mempolicy *policy);
 
 extern enum zone_type policy_zone;

@@ -328,6 +329,8 @@ static inline struct zonelist *huge_zone
 	return node_zonelist(0, gfp_flags);
 }
 
+static inline nodemask_t *huge_mpol_nodes_allowed(void) { return NULL; }
+
 static inline int do_migrate_pages(struct mm_struct *mm,
 			const nodemask_t *from_nodes,
 			const nodemask_t *to_nodes, int flags)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

mm/hugetlb.c static int adjust_pool_surplus(struct hs · line 1264

Quoted 8 lines

mm/hugetlb.c +1 -0

@@ -1257,10 +1257,13 @@ static int adjust_pool_surplus(struct hs
 static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count)
 {
 	unsigned long min_count, ret;
+	nodemask_t *nodes_allowed;
 
 	if (h->order >= MAX_ORDER)
 		return h->max_huge_pages;
 

Why can't you simply do this?

	struct mempolicy *pol = NULL;
	nodemask_t *nodes_allowed = &node_online_map;

	local_irq_disable();
	pol = current->mempolicy;
	mpol_get(pol);
	local_irq_enable();
	if (pol) {
		switch (pol->mode) {
		case MPOL_BIND:
		case MPOL_INTERLEAVE:
			nodes_allowed = pol->v.nodes;
			break;
		case MPOL_PREFERRED:
			... use NODEMASK_SCRATCH() ...
		default:
			BUG();
		}
	}
	mpol_put(pol);

and then use nodes_allowed throughout set_max_huge_pages()?

Quoted 37 lines

mm/hugetlb.c static int adjust_pool_surplus(struct hs · line 1264

Quoted 13 lines

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

mm/hugetlb.c +1 -0

@@ -1257,10 +1257,13 @@ static int adjust_pool_surplus(struct hs
 static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count)
 {
 	unsigned long min_count, ret;
+	nodemask_t *nodes_allowed;
 
 	if (h->order >= MAX_ORDER)
 		return h->max_huge_pages;
 

Why can't you simply do this?

	struct mempolicy *pol = NULL;
	nodemask_t *nodes_allowed = &node_online_map;

	local_irq_disable();
	pol = current->mempolicy;
	mpol_get(pol);
	local_irq_enable();
	if (pol) {
		switch (pol->mode) {
		case MPOL_BIND:
		case MPOL_INTERLEAVE:
			nodes_allowed = pol->v.nodes;
			break;
		case MPOL_PREFERRED:
			... use NODEMASK_SCRATCH() ...
		default:
			BUG();
		}
	}
	mpol_put(pol);

Quoted 1 line

and then use nodes_allowed throughout set_max_huge_pages()?

Well, I do use nodes_allowed [pointer] throughout set_max_huge_pages().

NODEMASK_SCRATCH() didn't exist when I wrote this, and I can't be sure it will return a kmalloc()'d nodemask, which I need because a NULL nodemask pointer means "all online nodes" [really all nodes with memory, I suppose] and I need a pointer to kmalloc()'d nodemask to return from huge_mpol_nodes_allowed(). I want to keep the access to the internals of mempolicy in mempolicy.[ch], thus the call out to huge_mpol_nodes_allowed(), instead of open coding it.

[PATCH 3/4] hugetlb: derive huge pages nodes allowed from task mempolicy

Against: 2.6.31-rc6-mmotm-090820-1918

V2:
+ cleaned up comments, removed some deemed unnecessary,
add some suggested by review
+ removed check for !current in huge_mpol_nodes_allowed().
+ added 'current->comm' to warning message in huge_mpol_nodes_allowed().
+ added VM_BUG_ON() assertion in hugetlb.c next_node_allowed() to catch out of range node id.
+ add examples to patch description

V3: Factored this patch from V2 patch 2/3

V4: added back missing "kfree(nodes_allowed)" in set_max_nr_hugepages()

This patch derives a "nodes_allowed" node mask from the numa mempolicy of the task modifying the number of persistent huge pages to control the allocation, freeing and adjusting of surplus huge pages. This mask is derived as follows:

* For "default" [NULL] task mempolicy, a NULL nodemask_t pointer is produced. This will cause the hugetlb subsystem to use node_online_map as the "nodes_allowed". This preserves the behavior before this patch.
* For "preferred" mempolicy, including explicit local allocation, a nodemask with the single preferred node will be produced. "local" policy will NOT track any internode migrations of the task adjusting nr_hugepages.
* For "bind" and "interleave" policy, the mempolicy's nodemask will be used.
* Other than to inform the construction of the nodes_allowed node mask, the actual mempolicy mode is ignored. That is, all modes behave like interleave over the resulting nodes_allowed mask with no "fallback".

Notes:

1) This patch introduces a subtle change in behavior: huge page allocation and freeing will be constrained by any mempolicy that the task adjusting the huge page pool inherits from its parent. This policy could come from a distant ancestor. The adminstrator adjusting the huge page pool without explicitly specifying a mempolicy via numactl might be surprised by this. Additionaly, any mempolicy specified by numactl will be constrained by the cpuset in which numactl is invoked.

2) Hugepages allocated at boot time use the node_online_map. An additional patch could implement a temporary boot time huge pages nodes_allowed command line parameter.

3) Using mempolicy to control persistent huge page allocation and freeing requires no change to hugeadm when invoking it via numactl, as shown in the examples below. However, hugeadm could be enhanced to take the allowed nodes as an argument and set its task mempolicy itself. This would allow it to detect and warn about any non-default mempolicy that it inherited from its parent, thus alleviating the issue described in Note 1 above.

See the updated documentation [next patch] for more information about the implications of this patch.

Examples:

Starting with:

	Node 0 HugePages_Total:     0
	Node 1 HugePages_Total:     0
	Node 2 HugePages_Total:     0
	Node 3 HugePages_Total:     0

Default behavior [with or without this patch] balances persistent hugepage allocation across nodes [with sufficient contiguous memory]:

	hugeadm --pool-pages-min=2048Kb:32

yields:

	Node 0 HugePages_Total:     8
	Node 1 HugePages_Total:     8
	Node 2 HugePages_Total:     8
	Node 3 HugePages_Total:     8

Applying mempolicy--e.g., with numactl [using '-m' a.k.a. '--membind' because it allows multiple nodes to be specified and it's easy to type]--we can allocate huge pages on individual nodes or sets of nodes. So, starting from the condition above, with 8 huge pages per node:

	numactl -m 2 hugeadm --pool-pages-min=2048Kb:+8

yields:

Node 0 HugePages_Total: 8
Node 1 HugePages_Total: 8
Node 2 HugePages_Total: 16
Node 3 HugePages_Total: 8

The incremental 8 huge pages were restricted to node 2 by the specified mempolicy.

Similarly, we can use mempolicy to free persistent huge pages from specified nodes:

	numactl -m 0,1 hugeadm --pool-pages-min=2048Kb:-8

yields:

Node 0 HugePages_Total: 4
Node 1 HugePages_Total: 4
Node 2 HugePages_Total: 16
Node 3 HugePages_Total: 8

The 8 huge pages freed were balanced over nodes 0 and 1.

Signed-off-by Lee Schermerhorn <[email protected]>

 include/linux/mempolicy.h |    3 ++
 mm/hugetlb.c              |   14 ++++++----
 mm/mempolicy.c            |   61 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 5 deletions(-)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/mempolicy.c
===================================================================

mm/mempolicy.c +61 -0

@@ -1564,6 +1564,67 @@ struct zonelist *huge_zonelist(struct vm
 	}
 	return zl;
 }
+
+/*
+ * huge_mpol_nodes_allowed -- mempolicy extension for huge pages.
+ *
+ * Returns a [pointer to a] nodelist based on the current task's mempolicy
+ * to constraing the allocation and freeing of persistent huge pages
+ * 'Preferred', 'local' and 'interleave' mempolicy will behave more like
+ * 'bind' policy in this context.  An attempt to allocate a persistent huge
+ * page will never "fallback" to another node inside the buddy system
+ * allocator.
+ *
+ * If the task's mempolicy is "default" [NULL], just return NULL for
+ * default behavior.  Otherwise, extract the policy nodemask for 'bind'
+ * or 'interleave' policy or construct a nodemask for 'preferred' or
+ * 'local' policy and return a pointer to a kmalloc()ed nodemask_t.
+ *
+ * N.B., it is the caller's responsibility to free a returned nodemask.
+ */
+nodemask_t *huge_mpol_nodes_allowed(void)
+{
+	nodemask_t *nodes_allowed = NULL;
+	struct mempolicy *mempolicy;
+	int nid;
+
+	if (!current->mempolicy)
+		return NULL;
+
+	mpol_get(current->mempolicy);
+	nodes_allowed = kmalloc(sizeof(*nodes_allowed), GFP_KERNEL);
+	if (!nodes_allowed) {
+		printk(KERN_WARNING "%s unable to allocate nodes allowed mask "
+			"for huge page allocation.\nFalling back to default.\n",
+			current->comm);
+		goto out;
+	}
+	nodes_clear(*nodes_allowed);
+
+	mempolicy = current->mempolicy;
+	switch (mempolicy->mode) {
+	case MPOL_PREFERRED:
+		if (mempolicy->flags & MPOL_F_LOCAL)
+			nid = numa_node_id();
+		else
+			nid = mempolicy->v.preferred_node;
+		node_set(nid, *nodes_allowed);
+		break;
+
+	case MPOL_BIND:
+		/* Fall through */
+	case MPOL_INTERLEAVE:
+		*nodes_allowed =  mempolicy->v.nodes;
+		break;
+
+	default:
+		BUG();
+	}
+
+out:
+	mpol_put(current->mempolicy);
+	return nodes_allowed;
+}
 #endif
 
 /* Allocate a page in interleaved policy.

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/mempolicy.h
===================================================================

include/linux/mempolicy.h +3 -0

@@ -201,6 +201,7 @@ extern void mpol_fix_fork_child_flag(str
 extern struct zonelist *huge_zonelist(struct vm_area_struct *vma,
 				unsigned long addr, gfp_t gfp_flags,
 				struct mempolicy **mpol, nodemask_t **nodemask);
+extern nodemask_t *huge_mpol_nodes_allowed(void);
 extern unsigned slab_node(struct mempolicy *policy);
 
 extern enum zone_type policy_zone;

@@ -328,6 +329,8 @@ static inline struct zonelist *huge_zone
 	return node_zonelist(0, gfp_flags);
 }
 
+static inline nodemask_t *huge_mpol_nodes_allowed(void) { return NULL; }
+
 static inline int do_migrate_pages(struct mm_struct *mm,
 			const nodemask_t *from_nodes,
 			const nodemask_t *to_nodes, int flags)

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

mm/hugetlb.c +9 -5

@@ -1257,10 +1257,13 @@ static int adjust_pool_surplus(struct hs
 static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count)
 {
 	unsigned long min_count, ret;
+	nodemask_t *nodes_allowed;
 
 	if (h->order >= MAX_ORDER)
 		return h->max_huge_pages;
 
+	nodes_allowed = huge_mpol_nodes_allowed();
+
 	/*
 	 * Increase the pool size
 	 * First take pages out of surplus state.  Then make up the

@@ -1274,7 +1277,7 @@ static unsigned long set_max_huge_pages(
 	 */
 	spin_lock(&hugetlb_lock);
 	while (h->surplus_huge_pages && count > persistent_huge_pages(h)) {
-		if (!adjust_pool_surplus(h, NULL, -1))
+		if (!adjust_pool_surplus(h, nodes_allowed, -1))
 			break;
 	}
 

@@ -1285,7 +1288,7 @@ static unsigned long set_max_huge_pages(
 		 * and reducing the surplus.
 		 */
 		spin_unlock(&hugetlb_lock);
-		ret = alloc_fresh_huge_page(h, NULL);
+		ret = alloc_fresh_huge_page(h, nodes_allowed);
 		spin_lock(&hugetlb_lock);
 		if (!ret)
 			goto out;

@@ -1309,18 +1312,19 @@ static unsigned long set_max_huge_pages(
 	 */
 	min_count = h->resv_huge_pages + h->nr_huge_pages - h->free_huge_pages;
 	min_count = max(count, min_count);
-	try_to_free_low(h, min_count, NULL);
+	try_to_free_low(h, min_count, nodes_allowed);
 	while (min_count < persistent_huge_pages(h)) {
-		if (!free_pool_huge_page(h, NULL, 0))
+		if (!free_pool_huge_page(h, nodes_allowed, 0))
 			break;
 	}
 	while (count < persistent_huge_pages(h)) {
-		if (!adjust_pool_surplus(h, NULL, 1))
+		if (!adjust_pool_surplus(h, nodes_allowed, 1))
 			break;
 	}
 out:
 	ret = persistent_huge_pages(h);
 	spin_unlock(&hugetlb_lock);
+	kfree(nodes_allowed);
 	return ret;
 }
 

PATCH/RFC 5/4 hugetlb: register per node hugepages attributes

Against: 2.6.31-rc6-mmotm-090820-1918

V2: remove dependency on kobject private bitfield. Search
global hstates then all per node hstates for kobject match in attribute show/store functions.

V3: rebase atop the mempolicy-based hugepage alloc/free;
use custom "nodes_allowed" to restrict alloc/free to a specific node via per node attributes. Per node attribute overrides mempolicy. I.e., mempolicy only applies to global attributes.

To demonstrate feasibility--if not advisability--of supporting both mempolicy-based persistent huge page management with per node "override" attributes.

This patch adds the per huge page size control/query attributes to the per node sysdevs:

/sys/devices/system/node/node<ID>/hugepages/hugepages-<size>/
nr_hugepages - r/w
free_huge_pages - r/o
surplus_huge_pages - r/o

The patch attempts to re-use/share as much of the existing global hstate attribute initialization and handling, and the "nodes_allowed" constraint processing as possible. In set_max_huge_pages(), a node id < 0 indicates a change to global hstate parameters. In this case, any non-default task mempolicy will be used to generate the nodes_allowed mask. A node id > 0 indicates a node specific update and the count argument specifies the target count for the node. From this info, we compute the target global count for the hstate and construct a nodes_allowed node mask contain only the specified node. Thus, setting the node specific nr_hugepages via the per node attribute effectively overrides any task mempolicy.

Issue: dependency of base driver [node] dependency on hugetlbfs module. We want to keep all of the hstate attribute registration and handling in the hugetlb module. However, we need to call into this code to register the per node hstate attributes on node hot plug.

With this patch:

(me):ls /sys/devices/system/node/node0/hugepages/hugepages-2048kB ./ ../ free_hugepages nr_hugepages surplus_hugepages

Starting from:

Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0
Node 1 HugePages_Total:     0
Node 1 HugePages_Free:      0
Node 1 HugePages_Surp:      0
Node 2 HugePages_Total:     0
Node 2 HugePages_Free:      0
Node 2 HugePages_Surp:      0
Node 3 HugePages_Total:     0
Node 3 HugePages_Free:      0
Node 3 HugePages_Surp:      0

vm.nr_hugepages = 0

Allocate 16 persistent huge pages on node 2:
(me):echo 16 >/sys/devices/system/node/node2/hugepages/hugepages-2048kB/nr_hugepages

[Note that this is equivalent to:

	numactl -m 2 hugeadmin --pool-pages-min 2M:+16

]

Yields:

Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0
Node 1 HugePages_Total:     0
Node 1 HugePages_Free:      0
Node 1 HugePages_Surp:      0
Node 2 HugePages_Total:    16
Node 2 HugePages_Free:     16
Node 2 HugePages_Surp:      0
Node 3 HugePages_Total:     0
Node 3 HugePages_Free:      0
Node 3 HugePages_Surp:      0

vm.nr_hugepages = 16

Global controls work as expected--reduce pool to 8 persistent huge pages:
(me):echo 8 >/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages

Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0
Node 1 HugePages_Total:     0
Node 1 HugePages_Free:      0
Node 1 HugePages_Surp:      0
Node 2 HugePages_Total:     8
Node 2 HugePages_Free:      8
Node 2 HugePages_Surp:      0
Node 3 HugePages_Total:     0
Node 3 HugePages_Free:      0
Node 3 HugePages_Surp:      0

Signed-off-by Lee Schermerhorn <[email protected]>

 drivers/base/node.c     |    2 
 include/linux/hugetlb.h |    6 +
 include/linux/node.h    |    3 
 mm/hugetlb.c            |  213 +++++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 197 insertions(+), 27 deletions(-)

Index: linux-2.6.31-rc6-mmotm-090820-1918/drivers/base/node.c
===================================================================

drivers/base/node.c +2 -0

@@ -200,6 +200,7 @@ int register_node(struct node *node, int
 		sysdev_create_file(&node->sysdev, &attr_distance);
 
 		scan_unevictable_register_node(node);
+		hugetlb_register_node(node);
 	}
 	return error;
 }

@@ -220,6 +221,7 @@ void unregister_node(struct node *node)
 	sysdev_remove_file(&node->sysdev, &attr_distance);
 
 	scan_unevictable_unregister_node(node);
+	hugetlb_unregister_node(node);
 
 	sysdev_unregister(&node->sysdev);
 }

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/hugetlb.h
===================================================================

include/linux/hugetlb.h +4 -0

@@ -278,6 +278,10 @@ static inline struct hstate *page_hstate
 	return size_to_hstate(PAGE_SIZE << compound_order(page));
 }
 
+struct node;
+extern void hugetlb_register_node(struct node *);
+extern void hugetlb_unregister_node(struct node *);
+
 #else
 struct hstate {};
 #define alloc_bootmem_huge_page(h) NULL

include/linux/hugetlb.h +0 -0

 #endif /* _LINUX_HUGETLB_H */

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

mm/hugetlb.c +1 -0

@@ -24,6 +24,7 @@
 #include <asm/io.h>
 
 #include <linux/hugetlb.h>
+#include <linux/node.h>
 #include "internal.h"
 
 const unsigned long hugetlb_zero = 0, hugetlb_infinity = ~0UL;

mm/hugetlb.c +15 -1

+	nodemask_t *nodes_allowed;
+	nodes_allowed = kmalloc(sizeof(*nodes_allowed), GFP_KERNEL);
+	if (!nodes_allowed) {
+		printk(KERN_WARNING "%s unable to allocate nodes allowed mask "
+			"for huge page allocation.\nFalling back to default.\n",
+			current->comm);
+	} else {
+		nodes_clear(*nodes_allowed);
+		node_set(nid, *nodes_allowed);
+	}
+	return nodes_allowed;
+}
+
 #define persistent_huge_pages(h) (h->nr_huge_pages - h->surplus_huge_pages)
-static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count)
+static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
+								int nid)
 {
 	unsigned long min_count, ret;
 	nodemask_t *nodes_allowed;

mm/hugetlb.c +9 -0

+	else {
+		/*
+		 * incoming 'count' is for node 'nid' only, so
+		 * adjust count to global, but restrict alloc/free
+		 * to the specified node.
+		 */
+		count += h->nr_huge_pages - h->nr_huge_pages_node[nid];
+		nodes_allowed = nodes_allowed_from_node(nid);
+	}
 
 	/*
 	 * Increase the pool size

@@ -1338,34 +1365,69 @@ out:
 static struct kobject *hugepages_kobj;
 static struct kobject *hstate_kobjs[HUGE_MAX_HSTATE];

mm/hugetlb.c +13 -3

+
+	BUG();
+	return NULL;
+}
+
+static struct hstate *kobj_to_hstate(struct kobject *kobj, int *nidp)
 {
 	int i;
+
 	for (i = 0; i < HUGE_MAX_HSTATE; i++)
-		if (hstate_kobjs[i] == kobj)
+		if (hstate_kobjs[i] == kobj) {
+			if (nidp)
+				*nidp = -1;
 			return &hstates[i];
-	BUG();
-	return NULL;
+		}
+
+	return kobj_to_node_hstate(kobj, nidp);
 }
 
 static ssize_t nr_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)

mm/hugetlb.c +5 -1

+	else
+		nr_huge_pages = h->nr_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", nr_huge_pages);
 }
+
 static ssize_t nr_hugepages_store(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
-	int err;
 	unsigned long input;

mm/hugetlb.c +115 -16

+	h = kobj_to_hstate(kobj, &nid);
+	h->max_huge_pages = set_max_huge_pages(h, input, nid);
 
 	return count;
 }

@@ -1374,15 +1436,17 @@ HSTATE_ATTR(nr_hugepages);
 static ssize_t nr_overcommit_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
+
 	return sprintf(buf, "%lu\n", h->nr_overcommit_huge_pages);
 }
+
 static ssize_t nr_overcommit_hugepages_store(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
 	int err;
 	unsigned long input;
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
 
 	err = strict_strtoul(buf, 10, &input);
 	if (err)

@@ -1399,15 +1463,24 @@ HSTATE_ATTR(nr_overcommit_hugepages);
 static ssize_t free_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->free_huge_pages);
+	struct hstate *h;
+	unsigned long free_huge_pages;
+	int nid;
+
+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		free_huge_pages = h->free_huge_pages;
+	else
+		free_huge_pages = h->free_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", free_huge_pages);
 }
 HSTATE_ATTR_RO(free_hugepages);
 
 static ssize_t resv_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
 	return sprintf(buf, "%lu\n", h->resv_huge_pages);
 }
 HSTATE_ATTR_RO(resv_hugepages);

@@ -1415,8 +1488,17 @@ HSTATE_ATTR_RO(resv_hugepages);
 static ssize_t surplus_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->surplus_huge_pages);
+	struct hstate *h;
+	unsigned long surplus_huge_pages;
+	int nid;
+
+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		surplus_huge_pages = h->surplus_huge_pages;
+	else
+		surplus_huge_pages = h->surplus_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", surplus_huge_pages);
 }
 HSTATE_ATTR_RO(surplus_hugepages);
 

@@ -1433,19 +1515,21 @@ static struct attribute_group hstate_att
 	.attrs = hstate_attrs,
 };
 
-static int __init hugetlb_sysfs_add_hstate(struct hstate *h)
+static int __init hugetlb_sysfs_add_hstate(struct hstate *h,
+				struct kobject *parent,
+				struct kobject **hstate_kobjs,
+				struct attribute_group *hstate_attr_group)
 {
 	int retval;
+	int hi = h - hstates;
 
-	hstate_kobjs[h - hstates] = kobject_create_and_add(h->name,
-							hugepages_kobj);
-	if (!hstate_kobjs[h - hstates])
+	hstate_kobjs[hi] = kobject_create_and_add(h->name, parent);
+	if (!hstate_kobjs[hi])
 		return -ENOMEM;
 
-	retval = sysfs_create_group(hstate_kobjs[h - hstates],
-							&hstate_attr_group);
+	retval = sysfs_create_group(hstate_kobjs[hi], hstate_attr_group);
 	if (retval)
-		kobject_put(hstate_kobjs[h - hstates]);
+		kobject_put(hstate_kobjs[hi]);
 
 	return retval;
 }

@@ -1460,17 +1544,90 @@ static void __init hugetlb_sysfs_init(vo
 		return;
 
 	for_each_hstate(h) {
-		err = hugetlb_sysfs_add_hstate(h);
+		err = hugetlb_sysfs_add_hstate(h, hugepages_kobj,
+					 hstate_kobjs, &hstate_attr_group);
 		if (err)
 			printk(KERN_ERR "Hugetlb: Unable to add hstate %s",
 								h->name);
 	}
 }
 
+#ifdef CONFIG_NUMA
+static struct attribute *per_node_hstate_attrs[] = {
+	&nr_hugepages_attr.attr,
+	&free_hugepages_attr.attr,
+	&surplus_hugepages_attr.attr,
+	NULL,
+};
+
+static struct attribute_group per_node_hstate_attr_group = {
+	.attrs = per_node_hstate_attrs,
+};
+
+
+void hugetlb_unregister_node(struct node *node)
+{
+	struct hstate *h;
+
+	for_each_hstate(h) {
+		kobject_put(node->hstate_kobjs[h - hstates]);
+		node->hstate_kobjs[h - hstates] = NULL;
+	}
+
+	kobject_put(node->hugepages_kobj);
+	node->hugepages_kobj = NULL;
+}
+
+static void hugetlb_unregister_all_nodes(void)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++)
+		hugetlb_unregister_node(&node_devices[nid]);
+}
+
+void hugetlb_register_node(struct node *node)
+{
+	struct hstate *h;
+	int err;
+
+	if (!hugepages_kobj)
+		return;		/* too early */
+
+	node->hugepages_kobj = kobject_create_and_add("hugepages",
+							&node->sysdev.kobj);
+	if (!node->hugepages_kobj)
+		return;
+
+	for_each_hstate(h) {
+		err = hugetlb_sysfs_add_hstate(h, node->hugepages_kobj,
+						node->hstate_kobjs,
+						&per_node_hstate_attr_group);
+		if (err)
+			printk(KERN_ERR "Hugetlb: Unable to add hstate %s"
+					" for node %d\n",
+						h->name, node->sysdev.id);
+	}
+}
+
+static void hugetlb_register_all_nodes(void)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++) {
+		struct node *node = &node_devices[nid];
+		if (node->sysdev.id == nid && !node->hugepages_kobj)
+			hugetlb_register_node(node);
+	}
+}
+#endif
+
 static void __exit hugetlb_exit(void)
 {
 	struct hstate *h;
 
+	hugetlb_unregister_all_nodes();
+
 	for_each_hstate(h) {
 		kobject_put(hstate_kobjs[h - hstates]);
 	}

@@ -1505,6 +1662,8 @@ static int __init hugetlb_init(void)
 
 	hugetlb_sysfs_init();
 
+	hugetlb_register_all_nodes();
+
 	return 0;
 }
 module_init(hugetlb_init);

@@ -1607,7 +1766,7 @@ int hugetlb_sysctl_handler(struct ctl_ta
 	proc_doulongvec_minmax(table, write, file, buffer, length, ppos);
 
 	if (write)
-		h->max_huge_pages = set_max_huge_pages(h, tmp);
+		h->max_huge_pages = set_max_huge_pages(h, tmp, -1);
 
 	return 0;
 }

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/node.h
===================================================================

include/linux/node.h +0 -0

@@ -21,9 +21,12 @@
 
 #include <linux/sysdev.h>

Quoted 148 lines · Lee Schermerhorn

PATCH/RFC 5/4 hugetlb: register per node hugepages attributes

Against: 2.6.31-rc6-mmotm-090820-1918

V2: remove dependency on kobject private bitfield. Search
global hstates then all per node hstates for kobject match in attribute show/store functions.

V3: rebase atop the mempolicy-based hugepage alloc/free;
use custom "nodes_allowed" to restrict alloc/free to a specific node via per node attributes. Per node attribute overrides mempolicy. I.e., mempolicy only applies to global attributes.

To demonstrate feasibility--if not advisability--of supporting both mempolicy-based persistent huge page management with per node "override" attributes.

This patch adds the per huge page size control/query attributes to the per node sysdevs:

/sys/devices/system/node/node<ID>/hugepages/hugepages-<size>/
nr_hugepages - r/w
free_huge_pages - r/o
surplus_huge_pages - r/o

The patch attempts to re-use/share as much of the existing global hstate attribute initialization and handling, and the "nodes_allowed" constraint processing as possible. In set_max_huge_pages(), a node id < 0 indicates a change to global hstate parameters. In this case, any non-default task mempolicy will be used to generate the nodes_allowed mask. A node id > 0 indicates a node specific update and the count argument specifies the target count for the node. From this info, we compute the target global count for the hstate and construct a nodes_allowed node mask contain only the specified node. Thus, setting the node specific nr_hugepages via the per node attribute effectively overrides any task mempolicy.

Issue: dependency of base driver [node] dependency on hugetlbfs module. We want to keep all of the hstate attribute registration and handling in the hugetlb module. However, we need to call into this code to register the per node hstate attributes on node hot plug.

With this patch:

(me):ls /sys/devices/system/node/node0/hugepages/hugepages-2048kB ./ ../ free_hugepages nr_hugepages surplus_hugepages

Starting from:

Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0
Node 1 HugePages_Total:     0
Node 1 HugePages_Free:      0
Node 1 HugePages_Surp:      0
Node 2 HugePages_Total:     0
Node 2 HugePages_Free:      0
Node 2 HugePages_Surp:      0
Node 3 HugePages_Total:     0
Node 3 HugePages_Free:      0
Node 3 HugePages_Surp:      0

vm.nr_hugepages = 0

Allocate 16 persistent huge pages on node 2:
(me):echo 16 >/sys/devices/system/node/node2/hugepages/hugepages-2048kB/nr_hugepages

[Note that this is equivalent to:

	numactl -m 2 hugeadmin --pool-pages-min 2M:+16

]

Yields:

Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0
Node 1 HugePages_Total:     0
Node 1 HugePages_Free:      0
Node 1 HugePages_Surp:      0
Node 2 HugePages_Total:    16
Node 2 HugePages_Free:     16
Node 2 HugePages_Surp:      0
Node 3 HugePages_Total:     0
Node 3 HugePages_Free:      0
Node 3 HugePages_Surp:      0

vm.nr_hugepages = 16

Global controls work as expected--reduce pool to 8 persistent huge pages:
(me):echo 8 >/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages

Node 0 HugePages_Total:     0
Node 0 HugePages_Free:      0
Node 0 HugePages_Surp:      0
Node 1 HugePages_Total:     0
Node 1 HugePages_Free:      0
Node 1 HugePages_Surp:      0
Node 2 HugePages_Total:     8
Node 2 HugePages_Free:      8
Node 2 HugePages_Surp:      0
Node 3 HugePages_Total:     0
Node 3 HugePages_Free:      0
Node 3 HugePages_Surp:      0

Signed-off-by Lee Schermerhorn <[email protected]>

 drivers/base/node.c     |    2 
 include/linux/hugetlb.h |    6 +
 include/linux/node.h    |    3 
 mm/hugetlb.c            |  213 +++++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 197 insertions(+), 27 deletions(-)

Index: linux-2.6.31-rc6-mmotm-090820-1918/drivers/base/node.c
===================================================================

drivers/base/node.c +2 -0

@@ -200,6 +200,7 @@ int register_node(struct node *node, int
 		sysdev_create_file(&node->sysdev, &attr_distance);
 
 		scan_unevictable_register_node(node);
+		hugetlb_register_node(node);
 	}
 	return error;
 }

@@ -220,6 +221,7 @@ void unregister_node(struct node *node)
 	sysdev_remove_file(&node->sysdev, &attr_distance);
 
 	scan_unevictable_unregister_node(node);
+	hugetlb_unregister_node(node);
 
 	sysdev_unregister(&node->sysdev);
 }

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/hugetlb.h
===================================================================

include/linux/hugetlb.h +4 -0

@@ -278,6 +278,10 @@ static inline struct hstate *page_hstate
 	return size_to_hstate(PAGE_SIZE << compound_order(page));
 }
 
+struct node;
+extern void hugetlb_register_node(struct node *);
+extern void hugetlb_unregister_node(struct node *);
+
 #else
 struct hstate {};
 #define alloc_bootmem_huge_page(h) NULL

include/linux/hugetlb.h static inline unsigned int pages_per_hug · line 304

Quoted 7 lines

include/linux/hugetlb.h +2 -0

@@ -294,6 +298,8 @@ static inline unsigned int pages_per_hug
 {
 	return 1;
 }
+#define hugetlb_register_node(NP)
+#define hugetlb_unregister_node(NP)
 #endif
 

This also needs to be done for the !NUMA case. Try building without NUMA set and you get the following with this patch applied

CC mm/hugetlb.o
mm/hugetlb.c: In function AcA?A?hugetlb_exitAcA?A?:
mm/hugetlb.c:1629: error: implicit declaration of function AcA?A?hugetlb_unregister_all_nodesAcA?A? mm/hugetlb.c: In function AcA?A?hugetlb_initAcA?A?:
mm/hugetlb.c:1665: error: implicit declaration of function AcA?A?hugetlb_register_all_nodesAcA?A? make[1]: *** [mm/hugetlb.o] Error 1
make: *** [mm] Error 2

Quoted 16 lines

include/linux/hugetlb.h +0 -0

 #endif /* _LINUX_HUGETLB_H */

Index: linux-2.6.31-rc6-mmotm-090820-1918/mm/hugetlb.c
===================================================================

mm/hugetlb.c +1 -0

@@ -24,6 +24,7 @@
 #include <asm/io.h>
 
 #include <linux/hugetlb.h>
+#include <linux/node.h>
 #include "internal.h"
 
 const unsigned long hugetlb_zero = 0, hugetlb_infinity = ~0UL;

mm/hugetlb.c static int adjust_pool_surplus(struct hs · line 1258

Quoted 5 lines

mm/hugetlb.c +2 -0

@@ -1253,8 +1254,24 @@ static int adjust_pool_surplus(struct hs
 	return ret;
 }
 
+static nodemask_t *nodes_allowed_from_node(int nid)
+{

This name is a bit weird. It's creating a nodemask with just a single node allowed.

Is there something wrong with using the existing function nodemask_of_node()? If stack is the problem, prehaps there is some macro magic that would allow a nodemask to be either declared on the stack or kmalloc'd.

Quoted 22 lines

mm/hugetlb.c +15 -1

+	nodemask_t *nodes_allowed;
+	nodes_allowed = kmalloc(sizeof(*nodes_allowed), GFP_KERNEL);
+	if (!nodes_allowed) {
+		printk(KERN_WARNING "%s unable to allocate nodes allowed mask "
+			"for huge page allocation.\nFalling back to default.\n",
+			current->comm);
+	} else {
+		nodes_clear(*nodes_allowed);
+		node_set(nid, *nodes_allowed);
+	}
+	return nodes_allowed;
+}
+
 #define persistent_huge_pages(h) (h->nr_huge_pages - h->surplus_huge_pages)
-static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count)
+static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
+								int nid)
 {
 	unsigned long min_count, ret;
 	nodemask_t *nodes_allowed;

mm/hugetlb.c static unsigned long set_max_huge_pages( · line 1283

Quoted 6 lines

mm/hugetlb.c +2 -1

@@ -1262,7 +1279,17 @@ static unsigned long set_max_huge_pages(
 	if (h->order >= MAX_ORDER)
 		return h->max_huge_pages;
 
-	nodes_allowed = huge_mpol_nodes_allowed();
+	if (nid < 0)
+		nodes_allowed = huge_mpol_nodes_allowed();

hugetlb is a bit littered with magic numbers been passed into functions. Attempts have been made to clear them up as according as patches change that area. Would it be possible to define something like

#define HUGETLB_OBEY_MEMPOLICY -1

for the nid here as opposed to passing in -1? I know -1 is used in the page allocator functions but there it means "current node" and here it means "obey mempolicies".

Quoted 16 lines

mm/hugetlb.c +9 -0

+	else {
+		/*
+		 * incoming 'count' is for node 'nid' only, so
+		 * adjust count to global, but restrict alloc/free
+		 * to the specified node.
+		 */
+		count += h->nr_huge_pages - h->nr_huge_pages_node[nid];
+		nodes_allowed = nodes_allowed_from_node(nid);
+	}
 
 	/*
 	 * Increase the pool size

@@ -1338,34 +1365,69 @@ out:
 static struct kobject *hugepages_kobj;
 static struct kobject *hstate_kobjs[HUGE_MAX_HSTATE];

mm/hugetlb.c out: · line 1375

Quoted 10 lines

mm/hugetlb.c +8 -1

 
-static struct hstate *kobj_to_hstate(struct kobject *kobj)
+static struct hstate *kobj_to_node_hstate(struct kobject *kobj, int *nidp)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++) {
+		struct node *node = &node_devices[nid];
+		int hi;
+		for (hi = 0; hi < HUGE_MAX_HSTATE; hi++)

Does that hi mean hello, high, nid or hstate_idx?

hstate_idx would appear to be the appropriate name here.

mm/hugetlb.c out: · line 1381

Quoted 7 lines

mm/hugetlb.c +6 -0

+			if (node->hstate_kobjs[hi] == kobj) {
+				if (nidp)
+					*nidp = nid;
+				return &hstates[hi];
+			}
+	}

Ok.... so, there is a struct node array for the sysdev and this patch adds references to the "hugepages" directory kobject and the subdirectories for each page size. We walk all the objects until we find a match. Obviously, this adds a dependency of base node support on hugetlbfs which feels backwards and you call that out in your leader.

Can this be the other way around? i.e. The struct hstate has an array of kobjects arranged by nid that is filled in when the node is registered? There will only be one kobject-per-pagesize-per-node so it seems like it would work. I confess, I haven't prototyped this to be 100% sure.

Quoted 32 lines

mm/hugetlb.c +17 -5

+
+	BUG();
+	return NULL;
+}
+
+static struct hstate *kobj_to_hstate(struct kobject *kobj, int *nidp)
 {
 	int i;
+
 	for (i = 0; i < HUGE_MAX_HSTATE; i++)
-		if (hstate_kobjs[i] == kobj)
+		if (hstate_kobjs[i] == kobj) {
+			if (nidp)
+				*nidp = -1;
 			return &hstates[i];
-	BUG();
-	return NULL;
+		}
+
+	return kobj_to_node_hstate(kobj, nidp);
 }
 
 static ssize_t nr_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->nr_huge_pages);
+	struct hstate *h;
+	unsigned long nr_huge_pages;
+	int nid;
+

mm/hugetlb.c out: · line 1410

Quoted 3 lines

mm/hugetlb.c +3 -0

+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		nr_huge_pages = h->nr_huge_pages;

Here is another magic number except it means something slightly different. It means NR_GLOBAL_HUGEPAGES or something similar. It would be nice if these different special nid values could be named, preferably collapsed to being one "core" thing.

Quoted 21 lines

mm/hugetlb.c +8 -2

+	else
+		nr_huge_pages = h->nr_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", nr_huge_pages);
 }
+
 static ssize_t nr_hugepages_store(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
-	int err;
 	unsigned long input;
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h;
+	int nid;
+	int err;
 
 	err = strict_strtoul(buf, 10, &input);
 	if (err)
 		return 0;
 

mm/hugetlb.c out: · line 1368

Quoted 1 line

mm/hugetlb.c +0 -1

-	h->max_huge_pages = set_max_huge_pages(h, input);

"input" is a bit meaningless. The function you are passing to calls this parameter "count". Can you match the naming please? Otherwise, I might guess that this is a "delta" which occurs elsewhere in the hugetlb code.

Quoted 229 lines

mm/hugetlb.c +115 -16

+	h = kobj_to_hstate(kobj, &nid);
+	h->max_huge_pages = set_max_huge_pages(h, input, nid);
 
 	return count;
 }

@@ -1374,15 +1436,17 @@ HSTATE_ATTR(nr_hugepages);
 static ssize_t nr_overcommit_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
+
 	return sprintf(buf, "%lu\n", h->nr_overcommit_huge_pages);
 }
+
 static ssize_t nr_overcommit_hugepages_store(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
 	int err;
 	unsigned long input;
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
 
 	err = strict_strtoul(buf, 10, &input);
 	if (err)

@@ -1399,15 +1463,24 @@ HSTATE_ATTR(nr_overcommit_hugepages);
 static ssize_t free_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->free_huge_pages);
+	struct hstate *h;
+	unsigned long free_huge_pages;
+	int nid;
+
+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		free_huge_pages = h->free_huge_pages;
+	else
+		free_huge_pages = h->free_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", free_huge_pages);
 }
 HSTATE_ATTR_RO(free_hugepages);
 
 static ssize_t resv_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
 	return sprintf(buf, "%lu\n", h->resv_huge_pages);
 }
 HSTATE_ATTR_RO(resv_hugepages);

@@ -1415,8 +1488,17 @@ HSTATE_ATTR_RO(resv_hugepages);
 static ssize_t surplus_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->surplus_huge_pages);
+	struct hstate *h;
+	unsigned long surplus_huge_pages;
+	int nid;
+
+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		surplus_huge_pages = h->surplus_huge_pages;
+	else
+		surplus_huge_pages = h->surplus_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", surplus_huge_pages);
 }
 HSTATE_ATTR_RO(surplus_hugepages);
 

@@ -1433,19 +1515,21 @@ static struct attribute_group hstate_att
 	.attrs = hstate_attrs,
 };
 
-static int __init hugetlb_sysfs_add_hstate(struct hstate *h)
+static int __init hugetlb_sysfs_add_hstate(struct hstate *h,
+				struct kobject *parent,
+				struct kobject **hstate_kobjs,
+				struct attribute_group *hstate_attr_group)
 {
 	int retval;
+	int hi = h - hstates;
 
-	hstate_kobjs[h - hstates] = kobject_create_and_add(h->name,
-							hugepages_kobj);
-	if (!hstate_kobjs[h - hstates])
+	hstate_kobjs[hi] = kobject_create_and_add(h->name, parent);
+	if (!hstate_kobjs[hi])
 		return -ENOMEM;
 
-	retval = sysfs_create_group(hstate_kobjs[h - hstates],
-							&hstate_attr_group);
+	retval = sysfs_create_group(hstate_kobjs[hi], hstate_attr_group);
 	if (retval)
-		kobject_put(hstate_kobjs[h - hstates]);
+		kobject_put(hstate_kobjs[hi]);
 
 	return retval;
 }

@@ -1460,17 +1544,90 @@ static void __init hugetlb_sysfs_init(vo
 		return;
 
 	for_each_hstate(h) {
-		err = hugetlb_sysfs_add_hstate(h);
+		err = hugetlb_sysfs_add_hstate(h, hugepages_kobj,
+					 hstate_kobjs, &hstate_attr_group);
 		if (err)
 			printk(KERN_ERR "Hugetlb: Unable to add hstate %s",
 								h->name);
 	}
 }
 
+#ifdef CONFIG_NUMA
+static struct attribute *per_node_hstate_attrs[] = {
+	&nr_hugepages_attr.attr,
+	&free_hugepages_attr.attr,
+	&surplus_hugepages_attr.attr,
+	NULL,
+};
+
+static struct attribute_group per_node_hstate_attr_group = {
+	.attrs = per_node_hstate_attrs,
+};
+
+
+void hugetlb_unregister_node(struct node *node)
+{
+	struct hstate *h;
+
+	for_each_hstate(h) {
+		kobject_put(node->hstate_kobjs[h - hstates]);
+		node->hstate_kobjs[h - hstates] = NULL;
+	}
+
+	kobject_put(node->hugepages_kobj);
+	node->hugepages_kobj = NULL;
+}
+
+static void hugetlb_unregister_all_nodes(void)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++)
+		hugetlb_unregister_node(&node_devices[nid]);
+}
+
+void hugetlb_register_node(struct node *node)
+{
+	struct hstate *h;
+	int err;
+
+	if (!hugepages_kobj)
+		return;		/* too early */
+
+	node->hugepages_kobj = kobject_create_and_add("hugepages",
+							&node->sysdev.kobj);
+	if (!node->hugepages_kobj)
+		return;
+
+	for_each_hstate(h) {
+		err = hugetlb_sysfs_add_hstate(h, node->hugepages_kobj,
+						node->hstate_kobjs,
+						&per_node_hstate_attr_group);
+		if (err)
+			printk(KERN_ERR "Hugetlb: Unable to add hstate %s"
+					" for node %d\n",
+						h->name, node->sysdev.id);
+	}
+}
+
+static void hugetlb_register_all_nodes(void)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++) {
+		struct node *node = &node_devices[nid];
+		if (node->sysdev.id == nid && !node->hugepages_kobj)
+			hugetlb_register_node(node);
+	}
+}
+#endif
+
 static void __exit hugetlb_exit(void)
 {
 	struct hstate *h;
 
+	hugetlb_unregister_all_nodes();
+
 	for_each_hstate(h) {
 		kobject_put(hstate_kobjs[h - hstates]);
 	}

@@ -1505,6 +1662,8 @@ static int __init hugetlb_init(void)
 
 	hugetlb_sysfs_init();
 
+	hugetlb_register_all_nodes();
+
 	return 0;
 }
 module_init(hugetlb_init);

@@ -1607,7 +1766,7 @@ int hugetlb_sysctl_handler(struct ctl_ta
 	proc_doulongvec_minmax(table, write, file, buffer, length, ppos);
 
 	if (write)
-		h->max_huge_pages = set_max_huge_pages(h, tmp);
+		h->max_huge_pages = set_max_huge_pages(h, tmp, -1);
 
 	return 0;
 }

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/node.h
===================================================================

include/linux/node.h +3 -0

@@ -21,9 +21,12 @@
 
 #include <linux/sysdev.h>
 #include <linux/cpumask.h>
+#include <linux/hugetlb.h>
 
 struct node {
 	struct sys_device	sysdev;
+	struct kobject		*hugepages_kobj;
+	struct kobject		*hstate_kobjs[HUGE_MAX_HSTATE];
 };
 

include/linux/node.h line 32

Quoted 2 lines

include/linux/node.h +0 -0

 struct memory_block;

I'm not against this idea and think it can work side-by-side with the memory policies. I believe it does need a bit more cleaning up before merging though. I also wasn't able to test this yet due to various build and deploy issues.

Quoted 11 lines

mm/hugetlb.c static int adjust_pool_surplus(struct hs · line 1258

Quoted 3 lines

mm/hugetlb.c +2 -0

+static nodemask_t *nodes_allowed_from_node(int nid)
+{

This name is a bit weird. It's creating a nodemask with just a single node allowed.

Is there something wrong with using the existing function nodemask_of_node()? If stack is the problem, prehaps there is some macro magic that would allow a nodemask to be either declared on the stack or kmalloc'd.

Yeah. nodemask_of_node() creates an on-stack mask, invisibly, in a block nested inside the context where it's invoked. I would be declaring the nodemask in the compound else clause and don't want to access it [via the nodes_allowed pointer] from outside of there.

Quoted 33 lines

Quoted 22 lines

mm/hugetlb.c +15 -1

+	nodemask_t *nodes_allowed;
+	nodes_allowed = kmalloc(sizeof(*nodes_allowed), GFP_KERNEL);
+	if (!nodes_allowed) {
+		printk(KERN_WARNING "%s unable to allocate nodes allowed mask "
+			"for huge page allocation.\nFalling back to default.\n",
+			current->comm);
+	} else {
+		nodes_clear(*nodes_allowed);
+		node_set(nid, *nodes_allowed);
+	}
+	return nodes_allowed;
+}
+
 #define persistent_huge_pages(h) (h->nr_huge_pages - h->surplus_huge_pages)
-static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count)
+static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count,
+								int nid)
 {
 	unsigned long min_count, ret;
 	nodemask_t *nodes_allowed;

mm/hugetlb.c static unsigned long set_max_huge_pages( · line 1283

Quoted 6 lines

mm/hugetlb.c +2 -1

@@ -1262,7 +1279,17 @@ static unsigned long set_max_huge_pages(
 	if (h->order >= MAX_ORDER)
 		return h->max_huge_pages;
 
-	nodes_allowed = huge_mpol_nodes_allowed();
+	if (nid < 0)
+		nodes_allowed = huge_mpol_nodes_allowed();

hugetlb is a bit littered with magic numbers been passed into functions. Attempts have been made to clear them up as according as patches change that area. Would it be possible to define something like

Quoted 5 lines

#define HUGETLB_OBEY_MEMPOLICY -1

for the nid here as opposed to passing in -1? I know -1 is used in the page allocator functions but there it means "current node" and here it means "obey mempolicies".

Well, here it means, NO_NODE_ID_SPECIFIED or, "we didn't get here via a per node attribute". It means "derive nodes allowed from memory policy, if non-default, else use nodes_online_map" [which is not exactly the same as obeying memory policy].

But, I can see defining a symbolic constant such as NO_NODE[_ID_SPECIFIED]. I'll try next spin.

Quoted 12 lines

mm/hugetlb.c out: · line 1375

Quoted 9 lines

mm/hugetlb.c +8 -1

-static struct hstate *kobj_to_hstate(struct kobject *kobj)
+static struct hstate *kobj_to_node_hstate(struct kobject *kobj, int *nidp)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++) {
+		struct node *node = &node_devices[nid];
+		int hi;
+		for (hi = 0; hi < HUGE_MAX_HSTATE; hi++)

Does that hi mean hello, high, nid or hstate_idx?

Quoted 1 line

hstate_idx would appear to be the appropriate name here.

Or just plain 'i', like in the following, pre-existing function?

Quoted 14 lines

mm/hugetlb.c out: · line 1381

Quoted 7 lines

mm/hugetlb.c +6 -0

+			if (node->hstate_kobjs[hi] == kobj) {
+				if (nidp)
+					*nidp = nid;
+				return &hstates[hi];
+			}
+	}

Ok.... so, there is a struct node array for the sysdev and this patch adds references to the "hugepages" directory kobject and the subdirectories for each page size. We walk all the objects until we find a match. Obviously, this adds a dependency of base node support on hugetlbfs which feels backwards and you call that out in your leader.

Quoted 4 lines

Can this be the other way around? i.e. The struct hstate has an array of kobjects arranged by nid that is filled in when the node is registered? There will only be one kobject-per-pagesize-per-node so it seems like it would work. I confess, I haven't prototyped this to be 100% sure.

This will take a bit longer to sort out. I do want to change the registration, tho', so that hugetlb.c registers it's single node register/unregister functions with base/node.c to remove the source level dependency in that direction. node.c will only register nodes on hot plug as it's initialized too early, relative to hugetlb.c to
register them at init time. This should break the call dependency of
base/node.c on the hugetlb module.

As far as moving the per node attributes' kobjects to the hugetlb global hstate arrays... Have to think about that. I agree that it would be nice to remove the source level [header] dependency.

Quoted 32 lines

Quoted 32 lines

mm/hugetlb.c +17 -5

+
+	BUG();
+	return NULL;
+}
+
+static struct hstate *kobj_to_hstate(struct kobject *kobj, int *nidp)
 {
 	int i;
+
 	for (i = 0; i < HUGE_MAX_HSTATE; i++)
-		if (hstate_kobjs[i] == kobj)
+		if (hstate_kobjs[i] == kobj) {
+			if (nidp)
+				*nidp = -1;
 			return &hstates[i];
-	BUG();
-	return NULL;
+		}
+
+	return kobj_to_node_hstate(kobj, nidp);
 }
 
 static ssize_t nr_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->nr_huge_pages);
+	struct hstate *h;
+	unsigned long nr_huge_pages;
+	int nid;
+

Quoted 8 lines

mm/hugetlb.c out: · line 1410

Quoted 3 lines

mm/hugetlb.c +3 -0

+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		nr_huge_pages = h->nr_huge_pages;

Here is another magic number except it means something slightly different. It means NR_GLOBAL_HUGEPAGES or something similar. It would be nice if these different special nid values could be named, preferably collapsed to being one "core" thing.

Again, it means "NO NODE ID specified" [via per node attribute]. Again, I'll address this with a single constant.

Quoted 21 lines

Quoted 21 lines

mm/hugetlb.c +8 -2

+	else
+		nr_huge_pages = h->nr_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", nr_huge_pages);
 }
+
 static ssize_t nr_hugepages_store(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
-	int err;
 	unsigned long input;
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h;
+	int nid;
+	int err;
 
 	err = strict_strtoul(buf, 10, &input);
 	if (err)
 		return 0;

Quoted 5 lines

mm/hugetlb.c out: · line 1368

Quoted 1 line

mm/hugetlb.c +0 -1

-	h->max_huge_pages = set_max_huge_pages(h, input);

"input" is a bit meaningless. The function you are passing to calls this parameter "count". Can you match the naming please? Otherwise, I might guess that this is a "delta" which occurs elsewhere in the hugetlb code.

I guess I can change that. It's the pre-exiting name, and 'count' was already used. Guess I can change 'count' to 'len' and 'input' to 'count'

Quoted 229 lines

Quoted 229 lines

mm/hugetlb.c +115 -16

+	h = kobj_to_hstate(kobj, &nid);
+	h->max_huge_pages = set_max_huge_pages(h, input, nid);
 
 	return count;
 }

@@ -1374,15 +1436,17 @@ HSTATE_ATTR(nr_hugepages);
 static ssize_t nr_overcommit_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
+
 	return sprintf(buf, "%lu\n", h->nr_overcommit_huge_pages);
 }
+
 static ssize_t nr_overcommit_hugepages_store(struct kobject *kobj,
 		struct kobj_attribute *attr, const char *buf, size_t count)
 {
 	int err;
 	unsigned long input;
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
 
 	err = strict_strtoul(buf, 10, &input);
 	if (err)

@@ -1399,15 +1463,24 @@ HSTATE_ATTR(nr_overcommit_hugepages);
 static ssize_t free_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->free_huge_pages);
+	struct hstate *h;
+	unsigned long free_huge_pages;
+	int nid;
+
+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		free_huge_pages = h->free_huge_pages;
+	else
+		free_huge_pages = h->free_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", free_huge_pages);
 }
 HSTATE_ATTR_RO(free_hugepages);
 
 static ssize_t resv_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
+	struct hstate *h = kobj_to_hstate(kobj, NULL);
 	return sprintf(buf, "%lu\n", h->resv_huge_pages);
 }
 HSTATE_ATTR_RO(resv_hugepages);

@@ -1415,8 +1488,17 @@ HSTATE_ATTR_RO(resv_hugepages);
 static ssize_t surplus_hugepages_show(struct kobject *kobj,
 					struct kobj_attribute *attr, char *buf)
 {
-	struct hstate *h = kobj_to_hstate(kobj);
-	return sprintf(buf, "%lu\n", h->surplus_huge_pages);
+	struct hstate *h;
+	unsigned long surplus_huge_pages;
+	int nid;
+
+	h = kobj_to_hstate(kobj, &nid);
+	if (nid < 0)
+		surplus_huge_pages = h->surplus_huge_pages;
+	else
+		surplus_huge_pages = h->surplus_huge_pages_node[nid];
+
+	return sprintf(buf, "%lu\n", surplus_huge_pages);
 }
 HSTATE_ATTR_RO(surplus_hugepages);
 

@@ -1433,19 +1515,21 @@ static struct attribute_group hstate_att
 	.attrs = hstate_attrs,
 };
 
-static int __init hugetlb_sysfs_add_hstate(struct hstate *h)
+static int __init hugetlb_sysfs_add_hstate(struct hstate *h,
+				struct kobject *parent,
+				struct kobject **hstate_kobjs,
+				struct attribute_group *hstate_attr_group)
 {
 	int retval;
+	int hi = h - hstates;
 
-	hstate_kobjs[h - hstates] = kobject_create_and_add(h->name,
-							hugepages_kobj);
-	if (!hstate_kobjs[h - hstates])
+	hstate_kobjs[hi] = kobject_create_and_add(h->name, parent);
+	if (!hstate_kobjs[hi])
 		return -ENOMEM;
 
-	retval = sysfs_create_group(hstate_kobjs[h - hstates],
-							&hstate_attr_group);
+	retval = sysfs_create_group(hstate_kobjs[hi], hstate_attr_group);
 	if (retval)
-		kobject_put(hstate_kobjs[h - hstates]);
+		kobject_put(hstate_kobjs[hi]);
 
 	return retval;
 }

@@ -1460,17 +1544,90 @@ static void __init hugetlb_sysfs_init(vo
 		return;
 
 	for_each_hstate(h) {
-		err = hugetlb_sysfs_add_hstate(h);
+		err = hugetlb_sysfs_add_hstate(h, hugepages_kobj,
+					 hstate_kobjs, &hstate_attr_group);
 		if (err)
 			printk(KERN_ERR "Hugetlb: Unable to add hstate %s",
 								h->name);
 	}
 }
 
+#ifdef CONFIG_NUMA
+static struct attribute *per_node_hstate_attrs[] = {
+	&nr_hugepages_attr.attr,
+	&free_hugepages_attr.attr,
+	&surplus_hugepages_attr.attr,
+	NULL,
+};
+
+static struct attribute_group per_node_hstate_attr_group = {
+	.attrs = per_node_hstate_attrs,
+};
+
+
+void hugetlb_unregister_node(struct node *node)
+{
+	struct hstate *h;
+
+	for_each_hstate(h) {
+		kobject_put(node->hstate_kobjs[h - hstates]);
+		node->hstate_kobjs[h - hstates] = NULL;
+	}
+
+	kobject_put(node->hugepages_kobj);
+	node->hugepages_kobj = NULL;
+}
+
+static void hugetlb_unregister_all_nodes(void)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++)
+		hugetlb_unregister_node(&node_devices[nid]);
+}
+
+void hugetlb_register_node(struct node *node)
+{
+	struct hstate *h;
+	int err;
+
+	if (!hugepages_kobj)
+		return;		/* too early */
+
+	node->hugepages_kobj = kobject_create_and_add("hugepages",
+							&node->sysdev.kobj);
+	if (!node->hugepages_kobj)
+		return;
+
+	for_each_hstate(h) {
+		err = hugetlb_sysfs_add_hstate(h, node->hugepages_kobj,
+						node->hstate_kobjs,
+						&per_node_hstate_attr_group);
+		if (err)
+			printk(KERN_ERR "Hugetlb: Unable to add hstate %s"
+					" for node %d\n",
+						h->name, node->sysdev.id);
+	}
+}
+
+static void hugetlb_register_all_nodes(void)
+{
+	int nid;
+
+	for (nid = 0; nid < nr_node_ids; nid++) {
+		struct node *node = &node_devices[nid];
+		if (node->sysdev.id == nid && !node->hugepages_kobj)
+			hugetlb_register_node(node);
+	}
+}
+#endif
+
 static void __exit hugetlb_exit(void)
 {
 	struct hstate *h;
 
+	hugetlb_unregister_all_nodes();
+
 	for_each_hstate(h) {
 		kobject_put(hstate_kobjs[h - hstates]);
 	}

@@ -1505,6 +1662,8 @@ static int __init hugetlb_init(void)
 
 	hugetlb_sysfs_init();
 
+	hugetlb_register_all_nodes();
+
 	return 0;
 }
 module_init(hugetlb_init);

@@ -1607,7 +1766,7 @@ int hugetlb_sysctl_handler(struct ctl_ta
 	proc_doulongvec_minmax(table, write, file, buffer, length, ppos);
 
 	if (write)
-		h->max_huge_pages = set_max_huge_pages(h, tmp);
+		h->max_huge_pages = set_max_huge_pages(h, tmp, -1);
 
 	return 0;
 }

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/node.h
===================================================================

include/linux/node.h +3 -0

@@ -21,9 +21,12 @@
 
 #include <linux/sysdev.h>
 #include <linux/cpumask.h>
+#include <linux/hugetlb.h>
 
 struct node {
 	struct sys_device	sysdev;
+	struct kobject		*hugepages_kobj;
+	struct kobject		*hstate_kobjs[HUGE_MAX_HSTATE];
 };
 

Quoted 7 lines

include/linux/node.h line 32

Quoted 2 lines

include/linux/node.h +0 -0

 struct memory_block;

I'm not against this idea and think it can work side-by-side with the memory policies. I believe it does need a bit more cleaning up before merging though. I also wasn't able to test this yet due to various build and deploy issues.

OK. I'll do the cleanup. I have tested this atop the mempolicy
version by working around the build issues that I thought were just temporary glitches in the mmotm series. In my [limited] experience, one can interleave numactl+hugeadm with setting values via the per node attributes and it does the right thing. No heavy testing with racing tasks, tho'.

Quoted 12 lines · Lee Schermerhorn

Quoted 11 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

+static nodemask_t *nodes_allowed_from_node(int nid)
+{

This name is a bit weird. It's creating a nodemask with just a single node allowed.

Is there something wrong with using the existing function nodemask_of_node()? If stack is the problem, prehaps there is some macro magic that would allow a nodemask to be either declared on the stack or kmalloc'd.

Quoted 5 lines

Yeah. nodemask_of_node() creates an on-stack mask, invisibly, in a block nested inside the context where it's invoked. I would be declaring the nodemask in the compound else clause and don't want to access it [via the nodes_allowed pointer] from outside of there.

So, the existance of the mask on the stack is the problem. I can understand that, they are potentially quite large.

Would it be possible to add a helper along side it like init_nodemask_of_node() that does the same work as nodemask_of_node() but takes a nodemask parameter? nodemask_of_node() would reuse the init_nodemask_of_node() except it declares the nodemask on the stack.

Against: 2.6.31-rc6-mmotm-090820-1918

Introduce nodemask macro to allocate a nodemask and initialize it to contain a single node, using existing nodemask_of_node() macro. Coded as a macro to avoid header dependency hell.

This will be used to construct the huge pages "nodes_allowed" nodemask for a single node when a persistent huge page pool page count is modified via a per node sysfs attribute.

Signed-off-by Lee Schermerhorn <[email protected]>

 include/linux/nodemask.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/nodemask.h
===================================================================

include/linux/nodemask.h +3 -0

@@ -257,6 +257,16 @@ static inline int __next_node(int n, con
 	m;								\
 })
 
+#define alloc_nodemask_of_node(node)					\
+({									\
+	typeof(_unused_nodemask_arg_) *nmp;				\

Quoted 27 lines · Lee Schermerhorn

Against: 2.6.31-rc6-mmotm-090820-1918

Introduce nodemask macro to allocate a nodemask and initialize it to contain a single node, using existing nodemask_of_node() macro. Coded as a macro to avoid header dependency hell.

This will be used to construct the huge pages "nodes_allowed" nodemask for a single node when a persistent huge page pool page count is modified via a per node sysfs attribute.

Signed-off-by Lee Schermerhorn <[email protected]>

 include/linux/nodemask.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/nodemask.h
===================================================================

include/linux/nodemask.h +3 -0

@@ -257,6 +257,16 @@ static inline int __next_node(int n, con
 	m;								\
 })
 
+#define alloc_nodemask_of_node(node)					\
+({									\
+	typeof(_unused_nodemask_arg_) *nmp;				\

include/linux/nodemask.h static inline int __next_node(int n, con · line 272

Quoted 10 lines

include/linux/nodemask.h +7 -0

+	nmp = kmalloc(sizeof(*nmp), GFP_KERNEL);			\
+	if (nmp)							\
+		*nmp = nodemask_of_node(node);				\
+	nmp;								\
+})
+
+
 #define first_unset_node(mask) __first_unset_node(&(mask))
 static inline int __first_unset_node(const nodemask_t *maskp)
 {

I think it would probably be better to use the generic NODEMASK_ALLOC() interface by requiring it to pass the entire type (including "struct") as part of the first parameter. Then it automatically takes care of dynamically allocating large nodemasks vs. allocating them on the stack.

Would it work by redefining NODEMASK_ALLOC() in the NODES_SHIFT > 8 case to be this:

	#define NODEMASK_ALLOC(x, m) x *m = kmalloc(sizeof(*m), GFP_KERNEL);

and converting NODEMASK_SCRATCH(x) to NODEMASK_ALLOC(struct nodemask_scratch, x), and then doing this in your code:

	NODEMASK_ALLOC(nodemask_t, nodes_allowed);
	if (nodes_allowed)
		*nodes_allowed = nodemask_of_node(node);

The NODEMASK_{ALLOC,SCRATCH}() interface is in its infancy so it can probably be made more general to handle cases like this.

Quoted 53 lines · David Rientjes

Quoted 27 lines · Lee Schermerhorn

Against: 2.6.31-rc6-mmotm-090820-1918

Introduce nodemask macro to allocate a nodemask and initialize it to contain a single node, using existing nodemask_of_node() macro. Coded as a macro to avoid header dependency hell.

This will be used to construct the huge pages "nodes_allowed" nodemask for a single node when a persistent huge page pool page count is modified via a per node sysfs attribute.

Signed-off-by Lee Schermerhorn <[email protected]>

 include/linux/nodemask.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6.31-rc6-mmotm-090820-1918/include/linux/nodemask.h
===================================================================

include/linux/nodemask.h +3 -0

@@ -257,6 +257,16 @@ static inline int __next_node(int n, con
 	m;								\
 })
 
+#define alloc_nodemask_of_node(node)					\
+({									\
+	typeof(_unused_nodemask_arg_) *nmp;				\

include/linux/nodemask.h static inline int __next_node(int n, con · line 272

Quoted 10 lines

include/linux/nodemask.h +7 -0

+	nmp = kmalloc(sizeof(*nmp), GFP_KERNEL);			\
+	if (nmp)							\
+		*nmp = nodemask_of_node(node);				\
+	nmp;								\
+})
+
+
 #define first_unset_node(mask) __first_unset_node(&(mask))
 static inline int __first_unset_node(const nodemask_t *maskp)
 {

I think it would probably be better to use the generic NODEMASK_ALLOC() interface by requiring it to pass the entire type (including "struct") as part of the first parameter. Then it automatically takes care of dynamically allocating large nodemasks vs. allocating them on the stack.

Would it work by redefining NODEMASK_ALLOC() in the NODES_SHIFT > 8 case to be this:

	#define NODEMASK_ALLOC(x, m) x *m = kmalloc(sizeof(*m), GFP_KERNEL);

and converting NODEMASK_SCRATCH(x) to NODEMASK_ALLOC(struct nodemask_scratch, x), and then doing this in your code:

Quoted 6 lines

	NODEMASK_ALLOC(nodemask_t, nodes_allowed);
	if (nodes_allowed)
		*nodes_allowed = nodemask_of_node(node);

The NODEMASK_{ALLOC,SCRATCH}() interface is in its infancy so it can probably be made more general to handle cases like this.

I just don't know what that would accomplish. Heck, I'm not all that happy with the alloc_nodemask_from_node() because it's allocating both a hidden nodemask_t and a pointer thereto on the stack just to return a pointer to a kmalloc()ed nodemask_t--which is what I want/need here.

One issue I have with NODEMASK_ALLOC() [and nodemask_of_node(), et al] is that it declares the pointer variable as well as initializing it,
perhaps with kmalloc(), ... Indeed, it's purpose is to replace on
stack nodemask declarations.

So, to use it at the start of, e.g., set_max_huge_pages() where I can safely use it throughout the function, I'll end up allocating the nodes_allowed mask on every call, whether or not a node is specified or
there is a non-default mempolicy. If it turns out that no node was
specified and we have default policy, we need to free the mask and NULL out nodes_allowed up front so that we get default behavior. That seems uglier to me that only allocating the nodemask when we know we need one.