hugetlb: remove unnecessary nid initialization

PATCH In review Nishanth Aravamudan 3 participants 11 messages May 16, 2007 – May 25, 2007

Subsystems (3)

Memory management Driver core HugeTLB

lore

Share & tools

lore

Nishanth Aravamudan

author

2007-05-16 23:30 UTC #0 lore raw

PATCH 1/3 hugetlb: remove unnecessary nid initialization

nid is initialized to numa_node_id() but will either be overwritten in the loop or not used in the conditional. So remove the initialization.

Signed-off-by Nishanth Aravamudan <[email protected]>

mm/hugetlb.c +1 -1

index eb7180d..abcd9a9 100644
@@ -66,7 +66,7 @@ static void enqueue_huge_page(struct page *page)
 static struct page *dequeue_huge_page(struct vm_area_struct *vma,
 				unsigned long address)
 {
-	int nid = numa_node_id();
+	int nid;
 	struct page *page = NULL;
 	struct zonelist *zonelist = huge_zonelist(vma, address);
 	struct zone **z;

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>

Nishanth Aravamudan

author

2007-05-16 23:31 UTC #1 lore raw

RFC PATCH 2/3 hugetlb: numafy several functions

Add node-parameterized helpers for dequeue_huge_page, alloc_fresh_huge_page and try_to_free_low. Also have update_and_free_page() take a nid parameter. This is necessary to add a per-node sysfs attribute to specify the number of hugepages on that node.

Signed-off-by Nishanth Aravamudan <[email protected]>

mm/hugetlb.c +66 ~~-32~~

index abcd9a9..51f412e 100644
@@ -63,11 +63,22 @@ static void enqueue_huge_page(struct page *page)
 	free_huge_pages_node[nid]++;
 }
 
+static struct page *dequeue_huge_page_node(int nid)
+{
+	struct page *page;
+
+	page = list_entry(hugepage_freelists[nid].next,
+					  struct page, lru);
+	list_del(&page->lru);
+	free_huge_pages--;
+	free_huge_pages_node[nid]--;
+	return page;
+}
+
 static struct page *dequeue_huge_page(struct vm_area_struct *vma,
 				unsigned long address)
 {
 	int nid;
-	struct page *page = NULL;
 	struct zonelist *zonelist = huge_zonelist(vma, address);
 	struct zone **z;

@@ -78,14 +89,9 @@ static struct page *dequeue_huge_page(struct vm_area_struct *vma,
 			break;
 	}
 
-	if (*z) {
-		page = list_entry(hugepage_freelists[nid].next,
-				  struct page, lru);
-		list_del(&page->lru);
-		free_huge_pages--;
-		free_huge_pages_node[nid]--;
-	}
-	return page;
+	if (*z)
+		return dequeue_huge_page_node(nid);
+	return NULL;
 }
 
 static void free_huge_page(struct page *page)

@@ -99,24 +105,40 @@ static void free_huge_page(struct page *page)
 	spin_unlock(&hugetlb_lock);
 }
 
-static int alloc_fresh_huge_page(void)
+static struct page *alloc_fresh_huge_page_node(int nid)
 {
-	static int nid = 0;
 	struct page *page;
-	page = alloc_pages_node(nid, GFP_HIGHUSER|__GFP_COMP|__GFP_NOWARN,
-					HUGETLB_PAGE_ORDER);
-	nid = next_node(nid, node_online_map);
-	if (nid == MAX_NUMNODES)
-		nid = first_node(node_online_map);
+
+	page = alloc_pages_node(nid,
+			GFP_HIGHUSER|__GFP_COMP|GFP_THISNODE,
+			HUGETLB_PAGE_ORDER);
 	if (page) {
 		set_compound_page_dtor(page, free_huge_page);
 		spin_lock(&hugetlb_lock);
 		nr_huge_pages++;
-		nr_huge_pages_node[page_to_nid(page)]++;
+		nr_huge_pages_node[nid]++;
 		spin_unlock(&hugetlb_lock);
 		put_page(page); /* free it into the hugepage allocator */
-		return 1;
 	}
+
+	return page;
+}
+
+static int alloc_fresh_huge_page(void)
+{
+	static int nid = 0;
+	struct page *page;
+	int start_nid = nid;
+
+	do {
+		page = alloc_fresh_huge_page_node(nid);
+
+		nid = next_node(nid, node_online_map);
+		if (nid == MAX_NUMNODES)
+			nid = first_node(node_online_map);
+	} while (!page && nid != start_nid);
+	if (page)
+		return 1;
 	return 0;
 }

@@ -186,11 +208,11 @@ static unsigned int cpuset_mems_nr(unsigned int *array)
 }
 
 #ifdef CONFIG_SYSCTL
-static void update_and_free_page(struct page *page)
+static void update_and_free_page(int nid, struct page *page)
 {
 	int i;
 	nr_huge_pages--;
-	nr_huge_pages_node[page_to_nid(page)]--;
+	nr_huge_pages_node[nid]--;
 	for (i = 0; i < (HPAGE_SIZE / PAGE_SIZE); i++) {
 		page[i].flags &= ~(1 << PG_locked | 1 << PG_error | 1 << PG_referenced |
 				1 << PG_dirty | 1 << PG_active | 1 << PG_reserved |

@@ -202,25 +224,37 @@ static void update_and_free_page(struct page *page)
 }
 
 #ifdef CONFIG_HIGHMEM
+static void try_to_free_low_node(int nid, unsigned long count)
+{
+	struct page *page, *next;
+
+	list_for_each_entry_safe(page, next,
+				&hugepage_freelists[nid], lru) {
+		if (PageHighMem(page))
+			continue;
+		list_del(&page->lru);
+		update_and_free_page(nid, page);
+		free_huge_pages--;
+		free_huge_pages_node[nid]--;
+		if (count >= nr_huge_pages_node[nid])
+			return;
+	}
+}
+
 static void try_to_free_low(unsigned long count)
 {
 	int i;
 
 	for (i = 0; i < MAX_NUMNODES; ++i) {
-		struct page *page, *next;
-		list_for_each_entry_safe(page, next, &hugepage_freelists[i], lru) {
-			if (PageHighMem(page))
-				continue;
-			list_del(&page->lru);
-			update_and_free_page(page);
-			free_huge_pages--;
-			free_huge_pages_node[page_to_nid(page)]--;
-			if (count >= nr_huge_pages)
-				return;
-		}
+		try_to_free_low_node(i, count);
+		if (count >= nr_huge_pages)
+			break;
 	}
 }
 #else
+static inline void try_to_free_low_node(int nid, unsigned long count)
+{
+}
 static inline void try_to_free_low(unsigned long count)
 {
 }

@@ -242,7 +276,7 @@ static unsigned long set_max_huge_pages(unsigned long count)
 		struct page *page = dequeue_huge_page(NULL, 0);
 		if (!page)
 			break;
-		update_and_free_page(page);
+		update_and_free_page(page_to_nid(page), page);
 	}
 	spin_unlock(&hugetlb_lock);
 	return nr_huge_pages;

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>

Nishanth Aravamudan

author

2007-05-16 22:32 UTC #2 lore raw

RFC PATCH 3/3 hugetlb: add per-node nr_hugepages sysfs attribute

Allow specifying the number of hugepages to allocate on a particular node. Our current global sysctl will try its best to put hugepages equally on each node, but htat may not always be desired. This allows the admin to control the layout of hugepage allocation at a finer level (while not breaking the existing interface).

Signed-off-by Nishanth Aravamudan <[email protected]>

---
I would have liked to have avoided the #ifdef's in node.c, but I couldn't figure out a simple way to conditionalize the create_file/remove_file calls.

drivers/base/node.c +11 -0

index cae346e..42c17fc 100644
@@ -131,6 +131,11 @@ static ssize_t node_read_distance(struct sys_device * dev, char * buf)
 }
 static SYSDEV_ATTR(distance, S_IRUGO, node_read_distance, NULL);
 
+#ifdef CONFIG_HUGETLB_PAGE
+static SYSDEV_ATTR(nr_hugepages, S_IRUGO | S_IWUSR,
+				hugetlb_read_nr_hugepages_node,
+				hugetlb_write_nr_hugepages_node);
+#endif
 
 /*
  * register_node - Setup a sysfs device for a node.

@@ -151,6 +156,9 @@ int register_node(struct node *node, int num, struct node *parent)
 		sysdev_create_file(&node->sysdev, &attr_meminfo);
 		sysdev_create_file(&node->sysdev, &attr_numastat);
 		sysdev_create_file(&node->sysdev, &attr_distance);
+#ifdef CONFIG_HUGETLB_PAGE
+		sysdev_create_file(&node->sysdev, &attr_nr_hugepages);
+#endif
 	}
 	return error;
 }

@@ -168,7 +176,9 @@ void unregister_node(struct node *node)
 	sysdev_remove_file(&node->sysdev, &attr_meminfo);
 	sysdev_remove_file(&node->sysdev, &attr_numastat);
 	sysdev_remove_file(&node->sysdev, &attr_distance);
+#ifdef CONFIG_HUGETLB_PAGE
+	sysdev_remove_file(&node->sysdev, &attr_nr_hugepages);
+#endif
 
 	sysdev_unregister(&node->sysdev);
 }

include/linux/hugetlb.h +3 -0

index b4570b6..cd21086 100644
@@ -5,6 +5,7 @@
 
 #include <linux/mempolicy.h>
 #include <linux/shm.h>
+#include <linux/sysdev.h>
 #include <asm/tlbflush.h>
 
 struct ctl_table;

@@ -22,6 +23,8 @@ void __unmap_hugepage_range(struct vm_area_struct *, unsigned long, unsigned lon
 int hugetlb_prefault(struct address_space *, struct vm_area_struct *);
 int hugetlb_report_meminfo(char *);
 int hugetlb_report_node_meminfo(int, char *);
+ssize_t hugetlb_read_nr_hugepages_node(struct sys_device *, char *);
+ssize_t hugetlb_write_nr_hugepages_node(struct sys_device *, const char *, size_t);
 unsigned long hugetlb_total_pages(void);
 int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 			unsigned long address, int write_access);

mm/hugetlb.c +45 -1

64 lines not on this page · raw message

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>

Nishanth Aravamudan

author

2007-05-23 17:51 UTC #3 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

In reply to Nishanth Aravamudan

Quoted 5 lines · Nishanth Aravamudan

Add node-parameterized helpers for dequeue_huge_page, alloc_fresh_huge_page and try_to_free_low. Also have update_and_free_page() take a nid parameter. This is necessary to add a per-node sysfs attribute to specify the number of hugepages on that node.

I saw that 1/3 was picked up by Andrew, but have not got any responses to the other two (I know Adam is out of town...).

Thoughts, comments? Bad idea, good idea?

I found it pretty handy to specify the exact layout of hugepages on each node.

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>

Lee Schermerhorn

2007-05-23 19:16 UTC #4 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

Quoted 9 lines · Nishanth Aravamudan

Quoted 5 lines · Nishanth Aravamudan
Add node-parameterized helpers for dequeue_huge_page, alloc_fresh_huge_page and try_to_free_low. Also have update_and_free_page() take a nid parameter. This is necessary to add a per-node sysfs attribute to specify the number of hugepages on that node.
I saw that 1/3 was picked up by Andrew, but have not got any responses to the other two (I know Adam is out of town...).

Nish: I haven't had a chance to test these patches. Other alligators in the swamp right now.

Quoted 5 lines

Thoughts, comments? Bad idea, good idea?
I found it pretty handy to specify the exact layout of hugepages on each node.

Could be useful for system with unequal memory per node, or where you know you want more huge pages on a given node. I recall that Tru64 Unix used to support something similar: most vm tunables that involved sizes or percentages of memory, such as page cache limits, locked memory limits, reserved huge pages, ..., could be specified as a single value that was distributed across nodes [backwards compatibility] or as list of per node values. However, I don't recall if marketing/customers asked for this or if it was a case of gratuitous design excess ;-).

I see that we'll need to reconcile the modified alloc_fresh_huge_page with the patch to skip unpopulated nodes when/if they collide in -mm.

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>

Nishanth Aravamudan

author

2007-05-23 19:29 UTC #5 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

Quoted 8 lines · Lee Schermerhorn

Quoted 7 lines · Nishanth Aravamudan
Quoted 5 lines · Nishanth Aravamudan
Add node-parameterized helpers for dequeue_huge_page, alloc_fresh_huge_page and try_to_free_low. Also have update_and_free_page() take a nid parameter. This is necessary to add a per-node sysfs attribute to specify the number of hugepages on that node.

Quoted 5 lines

Quoted 2 lines
I saw that 1/3 was picked up by Andrew, but have not got any responses to the other two (I know Adam is out of town...).
Nish: I haven't had a chance to test these patches. Other alligators in the swamp right now.

No problem.

Quoted 5 lines

Quoted 4 lines
Thoughts, comments? Bad idea, good idea?
I found it pretty handy to specify the exact layout of hugepages on each node.

Quoted 8 lines

Could be useful for system with unequal memory per node, or where you know you want more huge pages on a given node. I recall that Tru64 Unix used to support something similar: most vm tunables that involved sizes or percentages of memory, such as page cache limits, locked memory limits, reserved huge pages, ..., could be specified as a single value that was distributed across nodes [backwards compatibility] or as list of per node values. However, I don't recall if marketing/customers asked for this or if it was a case of gratuitous design excess ;-).

Yep, exactly the kind of use cases I was thinking of.

Quoted 2 lines

I see that we'll need to reconcile the modified alloc_fresh_huge_page with the patch to skip unpopulated nodes when/if they collide in -mm.

Yeah, if folks like the interface and are satisfied with it working, I'll rebase onto -mm for Andrew's sanity.

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>

Nishanth Aravamudan

author

2007-05-25 19:43 UTC #6 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

Andrew,

Quoted 32 lines · Nishanth Aravamudan

Quoted 8 lines · Lee Schermerhorn
Quoted 7 lines · Nishanth Aravamudan
Quoted 5 lines · Nishanth Aravamudan
Add node-parameterized helpers for dequeue_huge_page, alloc_fresh_huge_page and try_to_free_low. Also have update_and_free_page() take a nid parameter. This is necessary to add a per-node sysfs attribute to specify the number of hugepages on that node.
Quoted 5 lines
Quoted 2 lines
I saw that 1/3 was picked up by Andrew, but have not got any responses to the other two (I know Adam is out of town...).
Nish: I haven't had a chance to test these patches. Other alligators in the swamp right now.
No problem.
Quoted 5 lines
Quoted 4 lines
Thoughts, comments? Bad idea, good idea?
I found it pretty handy to specify the exact layout of hugepages on each node.
Quoted 9 lines
Could be useful for system with unequal memory per node, or where you know you want more huge pages on a given node. I recall that Tru64 Unix used to support something similar: most vm tunables that involved sizes or percentages of memory, such as page cache limits, locked memory limits, reserved huge pages, ..., could be specified as a single value that was distributed across nodes [backwards compatibility] or as list of per node values. However, I don't recall if marketing/customers asked for this or if it was a case of gratuitous design excess ;-).

Quoted 8 lines

Yep, exactly the kind of use cases I was thinking of.
Quoted 3 lines
I see that we'll need to reconcile the modified alloc_fresh_huge_page with the patch to skip unpopulated nodes when/if they collide in -mm.
Yeah, if folks like the interface and are satisfied with it working, I'll rebase onto -mm for Andrew's sanity.

Would you like me to rebase onto 2.6.22-rc2-mm1? I think this is a very useful feature for NUMA systems that may have an unequal distribution of memory and don't like the hugepage allocations provided by the global sysctl.

If I recall right, the collisions with Lee's hugetlb.c changes were pretty small, so it shouldn't be any trouble at all.

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>

Lee Schermerhorn

2007-05-25 20:55 UTC #7 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

Quoted 2 lines · Nishanth Aravamudan

Andrew,

<snip>

Quoted 11 lines

Quoted 3 lines
Yeah, if folks like the interface and are satisfied with it working, I'll rebase onto -mm for Andrew's sanity.
Would you like me to rebase onto 2.6.22-rc2-mm1? I think this is a very useful feature for NUMA systems that may have an unequal distribution of memory and don't like the hugepage allocations provided by the global sysctl.
If I recall right, the collisions with Lee's hugetlb.c changes were pretty small, so it shouldn't be any trouble at all.

Nish:

Unless I missed your post, I think Andrew is waiting to hear from you on the results of your testing of the 22-rc2 based v4 patch before merging the huge page allocation fix.

Lee

Nishanth Aravamudan

author

2007-05-25 21:10 UTC #8 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

Quoted 13 lines · Lee Schermerhorn

Quoted 2 lines · Nishanth Aravamudan
Andrew,
<snip>
Quoted 9 lines
Quoted 3 lines
Yeah, if folks like the interface and are satisfied with it working, I'll rebase onto -mm for Andrew's sanity.
Would you like me to rebase onto 2.6.22-rc2-mm1? I think this is a very useful feature for NUMA systems that may have an unequal distribution of memory and don't like the hugepage allocations provided by the global sysctl.

Quoted 8 lines

Quoted 2 lines
If I recall right, the collisions with Lee's hugetlb.c changes were pretty small, so it shouldn't be any trouble at all.
Nish:
Unless I missed your post, I think Andrew is waiting to hear from you on the results of your testing of the 22-rc2 based v4 patch before merging the huge page allocation fix.

Err, right.

Preliminary results:

x86 2-node success
x86_64 4-node success

ppc64 4-node fails with mainline, still investigating x86 1-node fails with mainline, believe it is machine setup error,
trying with another one

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>

Andrew Morton

maintainer

2007-05-25 21:39 UTC #9 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

In reply to Lee Schermerhorn

Quoted 13 lines · On Fri, 25 May 2007 16:55:59 -0400 Lee Schermerhorn <[email protected]>…

Quoted 2 lines · Nishanth Aravamudan
Andrew,
<snip>
Quoted 9 lines
Quoted 3 lines
Yeah, if folks like the interface and are satisfied with it working, I'll rebase onto -mm for Andrew's sanity.
Would you like me to rebase onto 2.6.22-rc2-mm1? I think this is a very useful feature for NUMA systems that may have an unequal distribution of memory and don't like the hugepage allocations provided by the global sysctl.

Quoted 9 lines

Quoted 2 lines
If I recall right, the collisions with Lee's hugetlb.c changes were pretty small, so it shouldn't be any trouble at all.
Nish:
Unless I missed your post, I think Andrew is waiting to hear from you on the results of your testing of the 22-rc2 based v4 patch before merging the huge page allocation fix.

I am in my common state of lost-the-plot on this patchset.

Once all the issues are believed to be settled, please send fresh shiny new patches against latest -linus or, if there is colliding stuff in -mm, against latest -mm.

Lee Schermerhorn

2007-05-25 21:47 UTC #10 lore raw

Re: [RFC][PATCH 2/3] hugetlb: numafy several functions

Quoted 18 lines · Andrew Morton

Quoted 16 lines · On Fri, 25 May 2007 16:55:59 -0400 Lee Schermerhorn <[email protected]>…
Quoted 2 lines · Nishanth Aravamudan
Andrew,
<snip>
Quoted 11 lines
Quoted 3 lines
Yeah, if folks like the interface and are satisfied with it working, I'll rebase onto -mm for Andrew's sanity.
Would you like me to rebase onto 2.6.22-rc2-mm1? I think this is a very useful feature for NUMA systems that may have an unequal distribution of memory and don't like the hugepage allocations provided by the global sysctl.
If I recall right, the collisions with Lee's hugetlb.c changes were pretty small, so it shouldn't be any trouble at all.

Quoted 8 lines

Quoted 6 lines
Nish:
Unless I missed your post, I think Andrew is waiting to hear from you on the results of your testing of the 22-rc2 based v4 patch before merging the huge page allocation fix.
I am in my common state of lost-the-plot on this patchset.

I was referring to the patch that Anton Blanchard started, and I reworked that distributes huge pages evenly over populated nodes in the presence of nodes with no memory in the zone from which HUGEPAGES are allocated. You had asked [off list, last week, 17may] whether we're still waiting for test results, to which I replied, "yes".

Later, Nish asked me for a version of the patch rebased against 22-rc2 because he was having difficulty building 22-rc*-mm* on his platform.

In this exchange, Nish is referring to his patch to allow explicit, per node, specification of nr_hugepages via a sysfs attribute.

Quoted 4 lines

Once all the issues are believed to be settled, please send fresh shiny new patches against latest -linus or, if there is colliding stuff in -mm, against latest -mm.

Will do.

Lee