Thread (7 messages) 7 messages, 4 authors, 2017-04-05

Re: [PATCH kernel] powerpc/iommu: Do not call PageTransHuge() on tail pages

From: Aneesh Kumar K.V <hidden>
Date: 2017-04-05 03:34:59


On Wednesday 05 April 2017 08:29 AM, Alexey Kardashevskiy wrote:
On 04/04/17 19:26, Aneesh Kumar K.V wrote:
quoted
Alexey Kardashevskiy [off-list ref] writes:
quoted
The CMA pages migration code does not support compound pages at
the moment so it performs few tests before proceeding to actual page
migration.

One of the tests - PageTransHuge() - has VM_BUG_ON_PAGE(PageTail()) as
it should be called on head pages. Since we also test for PageCompound(),
and it contains PageTail(), we can simply move PageCompound() in front
of PageTransHuge() and therefore avoid possible VM_BUG_ON_PAGE.

Signed-off-by: Alexey Kardashevskiy <redacted>
---

Some of actual POWER8 systems do crash on that BUG_ON.
---
 arch/powerpc/mm/mmu_context_iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index 497130c5c742..ba7fccf993b3 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -81,7 +81,7 @@ struct page *new_iommu_non_cma_page(struct page *page, unsigned long private,
 	gfp_t gfp_mask = GFP_USER;
 	struct page *new_page;

-	if (PageHuge(page) || PageTransHuge(page) || PageCompound(page))
+	if (PageCompound(page) || PageHuge(page) || PageTransHuge(page))

A checked for compound page should be sufficient here, because a
Huge/TransHuge page is also marked compound.

But PageCompound() calls PageTail() so PageTail() will be called on a trans
page which is BUG_ON in PageTransHuge but it is not in PageCompound() -
this inconsistency is bothering me. Does not this BUG_ON tell us that we
should not be calling PageTail() on _any_ page?

In other words, should I get a head page (via compound_head()) first and
only then test the head page if it is thp/huge (as you suggested in a chat)?

I was suggesting to replace that if () condition with just

/* We don't handle hugetlb/THP pages yet */
if (PageCompund(page)) {

}

-aneesh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help