RE: Hugepages demand paging V1 [4/4]: Numa patch

RE: Hugepages demand paging V1 [4/4]: Numa patch

[Christoph Lameter]

On Mon, 25 Oct 2004, Chen, Kenneth W wrote:

> > @@ -32,14 +32,17 @@
> > +	struct zonelist *zonelist = NODE_DATA(nid)->node_zonelists;
> > +	struct zone **zones = zonelist->zones;
> > +	struct zone *z;
> > +	int i;
> > +
> > +	for(i=0; (z = zones[i])!= NULL; i++) {
> > +		nid = z->zone_pgdat->node_id;
> > +		if (list_empty(&hugepage_freelists[node_id]))
> > +			break;
> >  	}
>
> Also this is generic code, we should consider scanning ZONE_HIGHMEM
> zonelist. Otherwise, this will likely screw up x86 numa machine.

The highmem zones are included in the zones[] array AFAIK.

RE: Hugepages demand paging V1 [4/4]: Numa patch

[Chen, Kenneth W]

On Mon, 25 Oct 2004, Chen, Kenneth W wrote:
> > @@ -32,14 +32,17 @@
> > +	struct zonelist *zonelist = NODE_DATA(nid)->node_zonelists;
> > +	struct zone **zones = zonelist->zones;
> > +	struct zone *z;
> > +	int i;
> > +
> > +	for(i=0; (z = zones[i])!= NULL; i++) {
> > +		nid = z->zone_pgdat->node_id;
> > +		if (list_empty(&hugepage_freelists[node_id]))
> > +			break;
> >  	}
>
> Also this is generic code, we should consider scanning ZONE_HIGHMEM
> zonelist. Otherwise, this will likely screw up x86 numa machine.

Christoph Lameter wrote on Wednesday, October 27, 2004 10:57 AM
> The highmem zones are included in the zones[] array AFAIK.


node_zonelists is an array in the struct pglist_data.  In your patch,
you are referencing the first element in that array, which has a zone
list for all node memory in normal zone.

What will happen for a x86 numa box with highmem only on some nodes?

- Ken

Hugepages demand paging V1 [0/4]: Discussion and overview

[Christoph Lameter]

This is a revised edition of the hugetlb demand page patches by
Kenneth Chen which were discussed in the following thread in August 2004

http://marc.theaimsgroup.com/?t=109171285000004&r=1&w=2

The initial post by Ken was in April in

http://marc.theaimsgroup.com/?l=linux-ia64&m=108189860401704&w=2

Hugetlb demand paging has been part of the production release of SuSE SLES
9 for awhile now (used by such products as Oracle) and this patchset is
intended to help hugetlb demand paging also get into the official Linux kernel.

This first version of the patchset is a collection of the patches from the
above mentioned thread in August. The key unresolved issue in that thread was
the necessity of using update_cache_mmu after setting up a huge pte.

update_cache_mmu is intended to update the mmu cache for a PAGESIZE page and
not for a huge page. The solution adopted here (as already suggested as
a possible solution in that thread) is to require an extension of
the semantics of set_huge_pte: set_huge_pte() must also do for huge pages
what update_cache_mmu does for PAGESIZE pages. For that purpose an additional
address parameter was added to set_huge_pte() which will conviently break
any old code. The patch included hopefully already fixes all occurrences
of set_huge_pte.

A linux-2.6.9-bk5 kernel with this patchset was build on IA64 and successfully
tested using a performance test program for hugetlb pages.

Note that this is the first patchset and is to be seen as discussion basis.
not as a final patchset. Please review these patches.

The patchset consists of 4 patches.

1/4 Demand Paging patch. This is the base and is mostly Ken's original work
	plus a fix that was posted later.

2/4 set_huge_pte update. This updates the set_huge_pte function for all
	architectures and insures that the arch specific action for
	update_mmu_cache is taken (which may be do nothing for some arches).
	Please verify that this really addresses the issues for each arch
	and that it is complete.

3/4 Overcommit patch: Mostly the original work by Ken plus a fix that he
	posted later.

4/4 Numa patch: Work by Raymund Bryant and myself at SGI to make
	the huge page allocator try to allocate local memory instead always
	starting at node zero. This definitely needs to be more sophisticated.

Patches 1 to 3 must be applied together. The Numa patch is optional.

Hugepages demand paging V1 [4/4]: Numa patch

[Christoph Lameter]

Changelog
	* NUMA enhancements (rough first implementation)
	* Do not begin search for huge page memory at the first node
	  but start at the current node and then search previous and
	  the following nodes for memory.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

Index: linux-2.6.9/mm/hugetlb.c
===================================================================
--- linux-2.6.9.orig/mm/hugetlb.c	2004-10-21 20:39:50.000000000 -0700
+++ linux-2.6.9/mm/hugetlb.c	2004-10-21 20:44:12.000000000 -0700
@@ -28,15 +28,30 @@
 	free_huge_pages_node[nid]++;
 }

-static struct page *dequeue_huge_page(void)
+static struct page *dequeue_huge_page(struct vm_area_struct *vma, unsigned long addr)
 {
 	int nid = numa_node_id();
+	int tid, nid2;
 	struct page *page = NULL;

 	if (list_empty(&hugepage_freelists[nid])) {
-		for (nid = 0; nid < MAX_NUMNODES; ++nid)
-			if (!list_empty(&hugepage_freelists[nid]))
-				break;
+		/* Prefer the neighboring nodes */
+		for (tid =1 ; tid < MAX_NUMNODES; tid++) {
+
+			/* Is there space in a following node ? */
+			nid2 = (nid + tid) % MAX_NUMNODES;
+			if (mpol_node_valid(nid2, vma, addr) &&
+				!list_empty(&hugepage_freelists[nid2]))
+					break;
+
+			/* or in an previous node ? */
+			if (tid > nid) continue;
+			nid2 = nid - tid;
+			if (mpol_node_valid(nid2, vma, addr) &&
+				!list_empty(&hugepage_freelists[nid2]))
+					break;
+		}
+		nid = nid2;
 	}
 	if (nid >= 0 && nid < MAX_NUMNODES &&
 	    !list_empty(&hugepage_freelists[nid])) {
@@ -75,13 +90,13 @@
 	spin_unlock(&hugetlb_lock);
 }

-struct page *alloc_huge_page(void)
+struct page *alloc_huge_page(struct vm_area_struct *vma, unsigned long addr)
 {
 	struct page *page;
 	int i;

 	spin_lock(&hugetlb_lock);
-	page = dequeue_huge_page();
+	page = dequeue_huge_page(vma, addr);
 	if (!page) {
 		spin_unlock(&hugetlb_lock);
 		return NULL;
@@ -181,7 +196,7 @@
 	spin_lock(&hugetlb_lock);
 	try_to_free_low(count);
 	while (count < nr_huge_pages) {
-		struct page *page = dequeue_huge_page();
+		struct page *page = dequeue_huge_page(NULL, 0);
 		if (!page)
 			break;
 		update_and_free_page(page);
@@ -255,7 +270,7 @@
 retry:
 	page = find_get_page(mapping, idx);
 	if (!page) {
-		page = alloc_huge_page();
+		page = alloc_huge_page(vma, addr);
 		if (!page)
 			/*
 			 * with strict overcommit accounting, we should never
Index: linux-2.6.9/include/linux/hugetlb.h
===================================================================
--- linux-2.6.9.orig/include/linux/hugetlb.h	2004-10-21 20:44:10.000000000 -0700
+++ linux-2.6.9/include/linux/hugetlb.h	2004-10-21 20:44:56.000000000 -0700
@@ -31,7 +31,7 @@
 				pmd_t *pmd, int write);
 int is_aligned_hugepage_range(unsigned long addr, unsigned long len);
 int pmd_huge(pmd_t pmd);
-struct page *alloc_huge_page(void);
+struct page *alloc_huge_page(struct vm_area_struct *vma, unsigned long addr);
 void free_huge_page(struct page *);

 extern unsigned long max_huge_pages;

RE: Hugepages demand paging V1 [4/4]: Numa patch

[Chen, Kenneth W]

Christoph Lameter wrote on Thursday, October 21, 2004 9:59 PM
> Changelog
> 	* NUMA enhancements (rough first implementation)
> 	* Do not begin search for huge page memory at the first node
> 	  but start at the current node and then search previous and
> 	  the following nodes for memory.
>
> -static struct page *dequeue_huge_page(void)
> +static struct page *dequeue_huge_page(struct vm_area_struct *vma, unsigned long addr)
>  {
>  	int nid = numa_node_id();
> +	int tid, nid2;
>  	struct page *page = NULL;
>
>  	if (list_empty(&hugepage_freelists[nid])) {
> -		for (nid = 0; nid < MAX_NUMNODES; ++nid)
> -			if (!list_empty(&hugepage_freelists[nid]))
> -				break;
> +		/* Prefer the neighboring nodes */
> +		for (tid =1 ; tid < MAX_NUMNODES; tid++) {
> +
> +			/* Is there space in a following node ? */
> +			nid2 = (nid + tid) % MAX_NUMNODES;
> +			if (mpol_node_valid(nid2, vma, addr) &&
> +				!list_empty(&hugepage_freelists[nid2]))
> +					break;
> +
> +			/* or in an previous node ? */
> +			if (tid > nid) continue;
> +			nid2 = nid - tid;
> +			if (mpol_node_valid(nid2, vma, addr) &&
> +				!list_empty(&hugepage_freelists[nid2]))
> +					break;

Are you sure about this?  Looked flawed to me.  Logical node number
does not directly correlate to numa memory hierarchy.

- Ken

Re: Hugepages demand paging V1 [4/4]: Numa patch

[William Lee Irwin III]

On Thu, Oct 21, 2004 at 09:58:54PM -0700, Christoph Lameter wrote:
> Changelog
> 	* NUMA enhancements (rough first implementation)
> 	* Do not begin search for huge page memory at the first node
> 	  but start at the current node and then search previous and
> 	  the following nodes for memory.
> Signed-off-by: Christoph Lameter <clameter@sgi.com>

dequeue_huge_page() seems to want a nodemask, not a vma, though I
suppose it's not particularly pressing.


> Index: linux-2.6.9/mm/hugetlb.c
> ===================================================================
> --- linux-2.6.9.orig/mm/hugetlb.c	2004-10-21 20:39:50.000000000 -0700
> +++ linux-2.6.9/mm/hugetlb.c	2004-10-21 20:44:12.000000000 -0700
> @@ -28,15 +28,30 @@
>  	free_huge_pages_node[nid]++;
>  }
> 
> -static struct page *dequeue_huge_page(void)
> +static struct page *dequeue_huge_page(struct vm_area_struct *vma, unsigned long addr)
>  {
>  	int nid = numa_node_id();
> +	int tid, nid2;
>  	struct page *page = NULL;
> 
>  	if (list_empty(&hugepage_freelists[nid])) {
> -		for (nid = 0; nid < MAX_NUMNODES; ++nid)
> -			if (!list_empty(&hugepage_freelists[nid]))
> -				break;
> +		/* Prefer the neighboring nodes */
> +		for (tid =1 ; tid < MAX_NUMNODES; tid++) {
> +
> +			/* Is there space in a following node ? */
> +			nid2 = (nid + tid) % MAX_NUMNODES;
> +			if (mpol_node_valid(nid2, vma, addr) &&
> +				!list_empty(&hugepage_freelists[nid2]))
> +					break;
> +
> +			/* or in an previous node ? */
> +			if (tid > nid) continue;
> +			nid2 = nid - tid;
> +			if (mpol_node_valid(nid2, vma, addr) &&
> +				!list_empty(&hugepage_freelists[nid2]))
> +					break;
> +		}
> +		nid = nid2;
>  	}
>  	if (nid >= 0 && nid < MAX_NUMNODES &&
>  	    !list_empty(&hugepage_freelists[nid])) {
> @@ -75,13 +90,13 @@
>  	spin_unlock(&hugetlb_lock);
>  }
> 
> -struct page *alloc_huge_page(void)
> +struct page *alloc_huge_page(struct vm_area_struct *vma, unsigned long addr)
>  {
>  	struct page *page;
>  	int i;
> 
>  	spin_lock(&hugetlb_lock);
> -	page = dequeue_huge_page();
> +	page = dequeue_huge_page(vma, addr);
>  	if (!page) {
>  		spin_unlock(&hugetlb_lock);
>  		return NULL;
> @@ -181,7 +196,7 @@
>  	spin_lock(&hugetlb_lock);
>  	try_to_free_low(count);
>  	while (count < nr_huge_pages) {
> -		struct page *page = dequeue_huge_page();
> +		struct page *page = dequeue_huge_page(NULL, 0);
>  		if (!page)
>  			break;
>  		update_and_free_page(page);
> @@ -255,7 +270,7 @@
>  retry:
>  	page = find_get_page(mapping, idx);
>  	if (!page) {
> -		page = alloc_huge_page();
> +		page = alloc_huge_page(vma, addr);
>  		if (!page)
>  			/*
>  			 * with strict overcommit accounting, we should never
> Index: linux-2.6.9/include/linux/hugetlb.h
> ===================================================================
> --- linux-2.6.9.orig/include/linux/hugetlb.h	2004-10-21 20:44:10.000000000 -0700
> +++ linux-2.6.9/include/linux/hugetlb.h	2004-10-21 20:44:56.000000000 -0700
> @@ -31,7 +31,7 @@
>  				pmd_t *pmd, int write);
>  int is_aligned_hugepage_range(unsigned long addr, unsigned long len);
>  int pmd_huge(pmd_t pmd);
> -struct page *alloc_huge_page(void);
> +struct page *alloc_huge_page(struct vm_area_struct *vma, unsigned long addr);
>  void free_huge_page(struct page *);
> 
>  extern unsigned long max_huge_pages;
> 

Re: Hugepages demand paging V1 [4/4]: Numa patch

[Christoph Lameter]

On Fri, 22 Oct 2004, William Lee Irwin III wrote:

> On Thu, Oct 21, 2004 at 09:58:54PM -0700, Christoph Lameter wrote:
> > Changelog
> > 	* NUMA enhancements (rough first implementation)
> > 	* Do not begin search for huge page memory at the first node
> > 	  but start at the current node and then search previous and
> > 	  the following nodes for memory.
> > Signed-off-by: Christoph Lameter <clameter@sgi.com>
>
> dequeue_huge_page() seems to want a nodemask, not a vma, though I
> suppose it's not particularly pressing.

How about this variation following __alloc_page:

Index: linux-2.6.9/mm/hugetlb.c
===================================================================
--- linux-2.6.9.orig/mm/hugetlb.c	2004-10-21 20:39:50.000000000 -0700
+++ linux-2.6.9/mm/hugetlb.c	2004-10-22 10:53:18.000000000 -0700
@@ -32,14 +32,17 @@
 {
 	int nid = numa_node_id();
 	struct page *page = NULL;
-
-	if (list_empty(&hugepage_freelists[nid])) {
-		for (nid = 0; nid < MAX_NUMNODES; ++nid)
-			if (!list_empty(&hugepage_freelists[nid]))
-				break;
+	struct zonelist *zonelist = NODE_DATA(nid)->node_zonelists;
+	struct zone **zones = zonelist->zones;
+	struct zone *z;
+	int i;
+
+	for(i=0; (z = zones[i])!= NULL; i++) {
+		nid = z->zone_pgdat->node_id;
+		if (list_empty(&hugepage_freelists[node_id]))
+			break;
 	}
-	if (nid >= 0 && nid < MAX_NUMNODES &&
-	    !list_empty(&hugepage_freelists[nid])) {
+	if (z) {
 		page = list_entry(hugepage_freelists[nid].next,
 				  struct page, lru);
 		list_del(&page->lru);

Re: Hugepages demand paging V1 [4/4]: Numa patch

[William Lee Irwin III]

On Fri, 22 Oct 2004, William Lee Irwin III wrote:
>> dequeue_huge_page() seems to want a nodemask, not a vma, though I
>> suppose it's not particularly pressing.

On Fri, Oct 22, 2004 at 12:37:13PM -0700, Christoph Lameter wrote:
> How about this variation following __alloc_page:

Looks reasonable. The bit that struck me as quirky was the mpol_* on
the NULL vma. This pretty much eliminates the hidden dispatch, so I'm
happy.


-- wli

RE: Hugepages demand paging V1 [4/4]: Numa patch

[Chen, Kenneth W]

On Fri, 22 Oct 2004, William Lee Irwin III wrote:
>> dequeue_huge_page() seems to want a nodemask, not a vma, though I
>> suppose it's not particularly pressing.

On Fri, Oct 22, 2004 at 12:37:13PM -0700, Christoph Lameter wrote:
> How about this variation following __alloc_page:

William Lee Irwin III wrote on Friday, October 22, 2004 12:41 PM
> Looks reasonable. The bit that struck me as quirky was the mpol_* on
> the NULL vma. This pretty much eliminates the hidden dispatch, so I'm
> happy.

The allocate from next best node is orthogonal to hugetlb demand paging.
This should be merged once all the bugs are fixed and later when demand
paging goes in, we can add the mpol_* stuff.

- Ken

Re: Hugepages demand paging V1 [4/4]: Numa patch

[William Lee Irwin III]

On Fri, Oct 22, 2004 at 12:37:13PM -0700, Christoph Lameter wrote:
>> How about this variation following __alloc_page:

William Lee Irwin III wrote on Friday, October 22, 2004 12:41 PM
>> Looks reasonable. The bit that struck me as quirky was the mpol_* on
>> the NULL vma. This pretty much eliminates the hidden dispatch, so I'm
>> happy.

On Mon, Oct 25, 2004 at 02:25:09PM -0700, Chen, Kenneth W wrote:
> The allocate from next best node is orthogonal to hugetlb demand paging.
> This should be merged once all the bugs are fixed and later when demand
> paging goes in, we can add the mpol_* stuff.

I'm not too picky about this. It appears to be the 4th of the series,
so assuming they go in in order that should meet your expectations. I
am significantly more concerned about the flush_dcache_page() issue in
general, though. I guess this should light a fire under my backside to
dredge up the docs describing the proper TLB flushing methods to use
in conjunction with large page extensions for the affected arches.


-- wli

Re: Hugepages demand paging V1 [4/4]: Numa patch

[William Lee Irwin III]

On Mon, Oct 25, 2004 at 02:25:09PM -0700, Chen, Kenneth W wrote:
>> The allocate from next best node is orthogonal to hugetlb demand paging.
>> This should be merged once all the bugs are fixed and later when demand
>> paging goes in, we can add the mpol_* stuff.

On Mon, Oct 25, 2004 at 02:52:19PM -0700, William Lee Irwin III wrote:
> I'm not too picky about this. It appears to be the 4th of the series,
> so assuming they go in in order that should meet your expectations. I
> am significantly more concerned about the flush_dcache_page() issue in
> general, though. I guess this should light a fire under my backside to
> dredge up the docs describing the proper TLB flushing methods to use
> in conjunction with large page extensions for the affected arches.

Cache flushing methods.


-- wli

RE: Hugepages demand paging V1 [4/4]: Numa patch

[Chen, Kenneth W]

Christoph Lameter wrote on Friday, October 22, 2004 12:37 PM
> > On Thu, Oct 21, 2004 at 09:58:54PM -0700, Christoph Lameter wrote:
> > > Changelog
> > > 	* NUMA enhancements (rough first implementation)
> > > 	* Do not begin search for huge page memory at the first node
> > > 	  but start at the current node and then search previous and
> > > 	  the following nodes for memory.
> > > Signed-off-by: Christoph Lameter <clameter@sgi.com>
> >
> > dequeue_huge_page() seems to want a nodemask, not a vma, though I
> > suppose it's not particularly pressing.
>
> How about this variation following __alloc_page:
>
> @@ -32,14 +32,17 @@
> +	struct zonelist *zonelist = NODE_DATA(nid)->node_zonelists;
> +	struct zone **zones = zonelist->zones;
> +	struct zone *z;
> +	int i;
> +
> +	for(i=0; (z = zones[i])!= NULL; i++) {
> +		nid = z->zone_pgdat->node_id;
> +		if (list_empty(&hugepage_freelists[node_id]))
> +			break;
>  	}

Must be typos in the if statement.  Two fatal errors here:  You don't
really mean to break out of the for loop if there are no hugetlb page
on that node, do you?  The variable name to index into the freelist is
wrong, should be nid, otherwise this code won't compile.  That line
should be this:

+		if (!list_empty(&hugepage_freelists[nid]))


Also this is generic code, we should consider scanning ZONE_HIGHMEM
zonelist. Otherwise, this will likely screw up x86 numa machine.

- Ken