[PATCH] thp: avoid VM_BUG_ON page_count(page) false positives in __collapse_huge_page_copy

[PATCH] thp: avoid VM_BUG_ON page_count(page) false positives in __collapse_huge_page_copy

[Andrea Arcangeli]

Some time ago Petr once reproduced a false positive VM_BUG_ON in
khugepaged while running the autonuma-benchmark on a large 8 node
system. All production kernels out there have DEBUG_VM=n so it was
only noticeable on self built kernels. It's not easily reproducible
even on the 8 nodes system.

This patch removes the false positive and it has been tested for a
while and it's good idea to queue it for upstream too. It's not urgent
and probably not worth it for -stable, though it wouldn't hurt. On
smaller systems it's not reproducible AFIK.

Andrea Arcangeli (1):
  thp: avoid VM_BUG_ON page_count(page) false positives in
    __collapse_huge_page_copy

 mm/huge_memory.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

--
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] thp: avoid VM_BUG_ON page_count(page) false positives in __collapse_huge_page_copy

[Andrea Arcangeli]

Use page_freeze_refs to prevent speculative pagecache lookups to
trigger the false positives, so we're still able to check the
page_count to be exact.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 mm/huge_memory.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 1598708..7eca652 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1704,6 +1704,9 @@ void __khugepaged_exit(struct mm_struct *mm)
 
 static void release_pte_page(struct page *page)
 {
+#ifdef CONFIG_DEBUG_VM
+	page_unfreeze_refs(page, 2);
+#endif
 	/* 0 stands for page_is_file_cache(page) == false */
 	dec_zone_page_state(page, NR_ISOLATED_ANON + 0);
 	unlock_page(page);
@@ -1784,6 +1787,20 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		VM_BUG_ON(!PageLocked(page));
 		VM_BUG_ON(PageLRU(page));
 
+#ifdef CONFIG_DEBUG_VM
+		/*
+		 * For the VM_BUG_ON check on page_count(page) in
+		 * __collapse_huge_page_copy not to trigger false
+		 * positives we've to prevent the speculative
+		 * pagecache lookups too with page_freeze_refs. We
+		 * could check for >= 2 instead but this provides for
+		 * a more strict debugging behavior.
+		 */
+		if (!page_freeze_refs(page, 2)) {
+			release_pte_pages(pte, _pte+1);
+			goto out;
+		}
+#endif
 		/* If there is no mapped pte young don't collapse the page */
 		if (pte_young(pteval) || PageReferenced(page) ||
 		    mmu_notifier_test_young(vma->vm_mm, address))
@@ -1814,7 +1831,7 @@ static void __collapse_huge_page_copy(pte_t *pte, struct page *page,
 			src_page = pte_page(pteval);
 			copy_user_highpage(page, src_page, address, vma);
 			VM_BUG_ON(page_mapcount(src_page) != 1);
-			VM_BUG_ON(page_count(src_page) != 2);
+			VM_BUG_ON(page_count(src_page) != 0);
 			release_pte_page(src_page);
 			/*
 			 * ptl mostly unnecessary, but preempt has to

--
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] thp: avoid VM_BUG_ON page_count(page) false positives in __collapse_huge_page_copy

[Andrea Arcangeli]

Speculative cache pagecache lookups can elevate the refcount from
under us, so avoid the false positive. If the refcount is < 2 we'll be
notified by a VM_BUG_ON in put_page_testzero as there are two
put_page(src_page) in a row before returning from this function.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 mm/huge_memory.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 1598708..ad56497 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1814,7 +1814,6 @@ static void __collapse_huge_page_copy(pte_t *pte, struct page *page,
 			src_page = pte_page(pteval);
 			copy_user_highpage(page, src_page, address, vma);
 			VM_BUG_ON(page_mapcount(src_page) != 1);
-			VM_BUG_ON(page_count(src_page) != 2);
 			release_pte_page(src_page);
 			/*
 			 * ptl mostly unnecessary, but preempt has to

--
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] thp: avoid VM_BUG_ON page_count(page) false positives in __collapse_huge_page_copy

[Rik van Riel]

On 09/28/2012 08:35 AM, Andrea Arcangeli wrote:
> Speculative cache pagecache lookups can elevate the refcount from
> under us, so avoid the false positive. If the refcount is < 2 we'll be
> notified by a VM_BUG_ON in put_page_testzero as there are two
> put_page(src_page) in a row before returning from this function.
>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

--
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] thp: avoid VM_BUG_ON page_count(page) false positives in __collapse_huge_page_copy

[Johannes Weiner]

On Fri, Sep 28, 2012 at 02:35:31PM +0200, Andrea Arcangeli wrote:
> Speculative cache pagecache lookups can elevate the refcount from
> under us, so avoid the false positive. If the refcount is < 2 we'll be
> notified by a VM_BUG_ON in put_page_testzero as there are two
> put_page(src_page) in a row before returning from this function.
> 
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>

Much better, thank you.

Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

--
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>