On 04/29/2015 12:44 AM, Kirill A. Shutemov wrote: >>> >>> I wrote this to fix bug I originally attributed to refcounting patchset, >>> but Sasha triggered the same bug on -next without the patchset applied: >>> >>> http://lkml.kernel.org/g/553EB993.7030401@xxxxxxxxxx >> >> Well why the heck didn't the changelog tell us this?!?!? > > Sasha reported bug in -next after I sent the patch. > >> >>> Now I think it's related to changing of PageLRU() behaviour on tail page >>> by my page flags patchset. >> >> So this patch is a bugfix against one of >> >> page-flags-trivial-cleanup-for-pagetrans-helpers.patch >> page-flags-introduce-page-flags-policies-wrt-compound-pages.patch >> page-flags-define-pg_locked-behavior-on-compound-pages.patch >> page-flags-define-behavior-of-fs-io-related-flags-on-compound-pages.patch >> page-flags-define-behavior-of-lru-related-flags-on-compound-pages.patch > > ^^^ this one is fault, I think. So this patch is now: http://www.ozlabs.org/~akpm/mmotm/broken-out/page-flags-define-behavior-of-lru-related-flags-on-compound-pages-fix.patch I've found a non-fatal but misleading issues. First, the "will fail to detect hugetlb pages in this case" part of the changelog, and mention of hugetlbfs in the comment is AFAIK moot. There's a PageLRU() check preceding the compound check, so hugetlbfs pages (which are not PageLRU() AFAIK) are already skipped at that point. I want to improve that in another series, but that's out of scope here. Second, compound_order(page) returns 0 for a tail page, so the ALIGN() trick that's supposed to properly advance pfn from a tail page is useless. We could grab a head page, but stumbling on a THP tail page should be very rare so it's not worth the trouble - just remove the ALIGN. From: Vlastimil Babka <vbabka@xxxxxxx> Date: Wed, 3 Jun 2015 11:03:37 +0200 Subject: [PATCH] page-flags-define-behavior-of-lru-related-flags-on-compound-pages-fix-fix Mentioning hugetlbfs is misleading, because PageLRU() checks skip over hugetlb pages. The ALIGN() parts are useless, because compound_order(page) returns 0 for tail pages. --- mm/compaction.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/mm/compaction.c b/mm/compaction.c index 6ef2fdf..16e1b57 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -733,9 +733,12 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, * if PageLRU is set) but the lock is not necessarily taken * here and it is wasteful to take it just to check transhuge. * Check PageCompound without lock and skip the whole pageblock - * if it's either a transhuge or hugetlbfs page, as calling - * compound_order() without preventing THP from splitting the - * page underneath us may return surprising results. + * if it's a transhuge page, as calling compound_order() + * without preventing THP from splitting the page underneath us + * may return surprising results. + * If we happen to check a THP tail page, compound_order() + * returns 0. It should be rare enough to not bother with + * using compound_head() in that case. */ if (PageCompound(page)) { int nr; @@ -743,7 +746,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, nr = 1 << compound_order(page); else nr = pageblock_nr_pages; - low_pfn = ALIGN(low_pfn + 1, nr) - 1; + low_pfn += nr - 1; continue; } @@ -768,7 +771,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, continue; if (PageCompound(page)) { int nr = 1 << compound_order(page); - low_pfn = ALIGN(low_pfn + 1, nr) - 1; + low_pfn += nr - 1; continue; } } -- 2.1.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>