[PATCH 0/2] Fix two bugs in hugetlbfs MAP_PRIVATE page reservation

[PATCH 0/2] Fix two bugs in hugetlbfs MAP_PRIVATE page reservation

[Mel Gorman]

The following two patches fix minor issues with the MAP_PRIVATE-reservation
support for hugetlbfs that showed up during testing. The first patch fixes a
problem whereby a check is made for MAP_SHARED mappings that is intended for
MAP_PRIVATE mappings only. The second fixes a BUG_ON that is triggered due to
an unaligned address.

Both patches are fixes for
hugetlb-reserve-huge-pages-for-reliable-map_private-hugetlbfs-mappings-until-fork.patch.
Credit goes to Adam Litke for spotting the problems during regression testing.

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

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

[PATCH 1/2] [PATCH] Fix a hugepage reservation check for MAP_SHARED

[Mel Gorman]

When removing a huge page from the hugepage pool for a fault the system
checks to see if the mapping requires additional pages to be reserved, and
if it does whether there are any unreserved pages remaining.  If not, the
allocation fails without even attempting to get a page. In order to determine
whether to apply this check we call vma_has_private_reserves() which tells us
if this vma is MAP_PRIVATE and is the owner.  This incorrectly triggers the
remaining reservation test for MAP_SHARED mappings which prevents allocation
of the final page in the pool even though it is reserved for this mapping.

In reality we only want to check this for MAP_PRIVATE mappings where the
process is not the original mapper.  Replace vma_has_private_reserves() with
vma_has_reserves() which indicates whether further reserves are required,
and update the caller.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 mm/hugetlb.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c
--- linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c	2008-07-08 11:54:34.000000000 -0700
+++ linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c	2008-07-08 12:41:36.000000000 -0700
@@ -343,13 +343,13 @@ void reset_vma_resv_huge_pages(struct vm
 }
 
 /* Returns true if the VMA has associated reserve pages */
-static int vma_has_private_reserves(struct vm_area_struct *vma)
+static int vma_has_reserves(struct vm_area_struct *vma)
 {
 	if (vma->vm_flags & VM_SHARED)
-		return 0;
-	if (!is_vma_resv_set(vma, HPAGE_RESV_OWNER))
-		return 0;
-	return 1;
+		return 1;
+	if (is_vma_resv_set(vma, HPAGE_RESV_OWNER))
+		return 1;
+	return 0;
 }
 
 static void clear_huge_page(struct page *page,
@@ -421,7 +421,7 @@ static struct page *dequeue_huge_page_vm
 	 * have no page reserves. This check ensures that reservations are
 	 * not "stolen". The child may still get SIGKILLed
 	 */
-	if (!vma_has_private_reserves(vma) &&
+	if (!vma_has_reserves(vma) &&
 			h->free_huge_pages - h->resv_huge_pages == 0)
 		return NULL;
 

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

[PATCH 2/2] [PATCH] Align faulting address to a hugepage boundary before unmapping

[Mel Gorman]

When taking a fault for COW on a private mapping it is possible that the
parent will have to steal the original page from its children due to an
insufficient hugepage pool.  In this case, unmap_ref_private() is called
for the faulting address to unmap via unmap_hugepage_range(). This patch
ensures that the address used for unmapping is hugepage-aligned.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 mm/hugetlb.c |    1 +
 1 file changed, 1 insertion(+)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c
--- linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c	2008-07-08 11:54:34.000000000 -0700
+++ linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c	2008-07-08 15:50:00.000000000 -0700
@@ -1767,6 +1767,7 @@ int unmap_ref_private(struct mm_struct *
 	 * vm_pgoff is in PAGE_SIZE units, hence the different calculation
 	 * from page cache lookup which is in HPAGE_SIZE units.
 	 */
+	address = address & huge_page_mask(hstate_vma(vma));
 	pgoff = ((address - vma->vm_start) >> PAGE_SHIFT)
 		+ (vma->vm_pgoff >> PAGE_SHIFT);
 	mapping = (struct address_space *)page_private(page);

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

Re: [PATCH 2/2] [PATCH] Align faulting address to a hugepage boundary before unmapping

[Adam Litke]

On Thu, 2008-07-10 at 18:30 +0100, Mel Gorman wrote:
> When taking a fault for COW on a private mapping it is possible that the
> parent will have to steal the original page from its children due to an
> insufficient hugepage pool.  In this case, unmap_ref_private() is called
> for the faulting address to unmap via unmap_hugepage_range(). This patch
> ensures that the address used for unmapping is hugepage-aligned.
> 
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>

Acked-by: Adam Litke <agl@us.ibm.com>

Tested and confirmed.

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

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

Re: [PATCH 1/2] [PATCH] Fix a hugepage reservation check for MAP_SHARED

[Adam Litke]

On Thu, 2008-07-10 at 18:30 +0100, Mel Gorman wrote:
> When removing a huge page from the hugepage pool for a fault the system
> checks to see if the mapping requires additional pages to be reserved, and
> if it does whether there are any unreserved pages remaining.  If not, the
> allocation fails without even attempting to get a page. In order to determine
> whether to apply this check we call vma_has_private_reserves() which tells us
> if this vma is MAP_PRIVATE and is the owner.  This incorrectly triggers the
> remaining reservation test for MAP_SHARED mappings which prevents allocation
> of the final page in the pool even though it is reserved for this mapping.
> 
> In reality we only want to check this for MAP_PRIVATE mappings where the
> process is not the original mapper.  Replace vma_has_private_reserves() with
> vma_has_reserves() which indicates whether further reserves are required,
> and update the caller.
> 
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>

Acked-by: Adam Litke <agl@us.ibm.com>

Tested and confirmed.

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

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

Re: [PATCH 1/2] [PATCH] Fix a hugepage reservation check for MAP_SHARED

[Andy Whitcroft]

On Thu, Jul 10, 2008 at 06:30:21PM +0100, Mel Gorman wrote:
> 
> When removing a huge page from the hugepage pool for a fault the system
> checks to see if the mapping requires additional pages to be reserved, and
> if it does whether there are any unreserved pages remaining.  If not, the
> allocation fails without even attempting to get a page. In order to determine
> whether to apply this check we call vma_has_private_reserves() which tells us
> if this vma is MAP_PRIVATE and is the owner.  This incorrectly triggers the
> remaining reservation test for MAP_SHARED mappings which prevents allocation
> of the final page in the pool even though it is reserved for this mapping.
> 
> In reality we only want to check this for MAP_PRIVATE mappings where the
> process is not the original mapper.  Replace vma_has_private_reserves() with
> vma_has_reserves() which indicates whether further reserves are required,
> and update the caller.

Acked-by: Andy Whitcroft <apw@shadowen.org>

> Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> ---
> 
>  mm/hugetlb.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c
> --- linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c	2008-07-08 11:54:34.000000000 -0700
> +++ linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c	2008-07-08 12:41:36.000000000 -0700
> @@ -343,13 +343,13 @@ void reset_vma_resv_huge_pages(struct vm
>  }
>  
>  /* Returns true if the VMA has associated reserve pages */
> -static int vma_has_private_reserves(struct vm_area_struct *vma)
> +static int vma_has_reserves(struct vm_area_struct *vma)
>  {
>  	if (vma->vm_flags & VM_SHARED)
> -		return 0;
> -	if (!is_vma_resv_set(vma, HPAGE_RESV_OWNER))
> -		return 0;
> -	return 1;
> +		return 1;
> +	if (is_vma_resv_set(vma, HPAGE_RESV_OWNER))
> +		return 1;
> +	return 0;
>  }
>  
>  static void clear_huge_page(struct page *page,
> @@ -421,7 +421,7 @@ static struct page *dequeue_huge_page_vm
>  	 * have no page reserves. This check ensures that reservations are
>  	 * not "stolen". The child may still get SIGKILLed
>  	 */
> -	if (!vma_has_private_reserves(vma) &&
> +	if (!vma_has_reserves(vma) &&
>  			h->free_huge_pages - h->resv_huge_pages == 0)
>  		return NULL;
>  

-apw

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

Re: [PATCH 2/2] [PATCH] Align faulting address to a hugepage boundary before unmapping

[Andy Whitcroft]

On Thu, Jul 10, 2008 at 06:30:41PM +0100, Mel Gorman wrote:
> 
> When taking a fault for COW on a private mapping it is possible that the
> parent will have to steal the original page from its children due to an
> insufficient hugepage pool.  In this case, unmap_ref_private() is called
> for the faulting address to unmap via unmap_hugepage_range(). This patch
> ensures that the address used for unmapping is hugepage-aligned.
> 
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>

Acked-by: Andy Whitcroft <apw@shadowen.org>

Clearly far too many of our tests use page aligned accesss to trigger
behaviour.

>  mm/hugetlb.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c
> --- linux-2.6.26-rc8-mm1-clean/mm/hugetlb.c	2008-07-08 11:54:34.000000000 -0700
> +++ linux-2.6.26-rc8-mm1-fix-needsreserve-check/mm/hugetlb.c	2008-07-08 15:50:00.000000000 -0700
> @@ -1767,6 +1767,7 @@ int unmap_ref_private(struct mm_struct *
>  	 * vm_pgoff is in PAGE_SIZE units, hence the different calculation
>  	 * from page cache lookup which is in HPAGE_SIZE units.
>  	 */
> +	address = address & huge_page_mask(hstate_vma(vma));
>  	pgoff = ((address - vma->vm_start) >> PAGE_SHIFT)
>  		+ (vma->vm_pgoff >> PAGE_SHIFT);
>  	mapping = (struct address_space *)page_private(page);

-apw

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