On 04/04/17 19:26, Aneesh Kumar K.V wrote: > Alexey Kardashevskiy <a...@ozlabs.ru> writes: > >> 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 <a...@ozlabs.ru> >> --- >> >> 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)? > If we want to indicate that > we don't handle hugetlb and THP pages, we can write that as a comment ? > > > >> return NULL; >> >> if (PageHighMem(page)) >> @@ -100,7 +100,7 @@ static int mm_iommu_move_page_from_cma(struct page *page) >> LIST_HEAD(cma_migrate_pages); >> >> /* Ignore huge pages for now */ >> - if (PageHuge(page) || PageTransHuge(page) || PageCompound(page)) >> + if (PageCompound(page) || PageHuge(page) || PageTransHuge(page)) >> return -EBUSY; >> >> lru_add_drain(); >> -- >> 2.11.0 > -- Alexey
