On Wed, Sep 12, 2012 at 02:20:19PM -0700, Andrew Morton wrote: > OK, I'll slip this in there: > > --- a/mm/compaction.c~mm-compaction-abort-compaction-loop-if-lock-is-contended-or-run-too-long-fix > +++ a/mm/compaction.c > @@ -909,8 +909,7 @@ static unsigned long compact_zone_order( > INIT_LIST_HEAD(&cc.migratepages); > > ret = compact_zone(zone, &cc); > - if (contended) > - *contended = cc.contended; > + *contended = cc.contended; > return ret; > } Ack the above, thanks. One more thing, today a bug tripped while building cyanogenmod10 (it swaps despite so much ram) after I added the cc->contended loop break patch. The original version of the fix from Shaohua didn't have this problem because it would only abort compaction if the low_pfn didn't advance and in turn the list would be guaranteed empty. Verifying the list is empty before aborting compaction (which takes a path that ignores the cc->migratelist) should be enough to fix it and it makes it really equivalent to the previous fix. Both cachelines should be cache hot so it should be practically zero cost to check it. Only lightly tested so far. === >From b2a50e49d65596d3920773316ad9b7dd54e4acaf Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli <aarcange@xxxxxxxxxx> Date: Thu, 13 Sep 2012 01:22:03 +0200 Subject: [PATCH] mm: compaction: fix leak in cc->contended loop breaking logic We cannot return ISOLATE_ABORT when cc->contended is true, if we have some pages already successfully isolated in the cc->migratepages list, or they will be leaked. The bug was highlighted by a nice VM_BUG_ON in the async compaction in kswapd. So I also added the symmetric VM_BUG_ON to the other caller of the function considering it looks a worthwhile VM_BUG_ON. ------------[ cut here ]------------ kernel BUG at mm/compaction.c:934! invalid opcode: 0000 [#1] SMP Modules linked in: tun usbhid kvm_intel xhci_hcd kvm snd_hda_codec_realtek ehci_hcd usbcore snd_hda_intel sn er crc32c_intel psmouse ghash_clmulni_intel sr_mod snd sg cdrom snd_page_alloc usb_common pcspkr [last unloa CPU 0 Pid: 513, comm: kswapd0 Not tainted 3.6.0-rc4+ #17 /DH61BE RIP: 0010:[<ffffffff8111302c>] [<ffffffff8111302c>] __compact_pgdat+0x1ac/0x1b0 RSP: 0018:ffff880216fa5cb0 EFLAGS: 00010283 RAX: 0000000000000003 RBX: ffff880216fa5d00 RCX: 0000000000000002 RDX: 00000000000008d7 RSI: 0000000000000002 RDI: ffffffff8195b058 RBP: ffffffff8195b000 R08: 0000000000000be4 R09: ffffffff8195a9c0 R10: ffffffff8195b400 R11: ffffffff8195b570 R12: 0000000000000001 R13: 0000000000000001 R14: ffff880216fa5d10 R15: 0000000000000003 FS: 0000000000000000(0000) GS:ffff88021fa00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: 00007f14d4167000 CR3: 00000000018f1000 CR4: 00000000000407f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process kswapd0 (pid: 513, threadinfo ffff880216fa4000, task ffff880216cfef20) Stack: ffffffff8195a9c0 ffffffff8195b000 0000000000000320 0000000000000003 ffffffff8195a9c0 ffffffff8195b640 0000000000000002 0000000000000c80 0000000000000001 ffffffff811132f3 ffff880216fa5d00 ffff880216fa5d00 Call Trace: [<ffffffff811132f3>] ? compact_pgdat+0x23/0x30 [<ffffffff8110503f>] ? kswapd+0x89f/0xac0 [<ffffffff8106f450>] ? wake_up_bit+0x40/0x40 [<ffffffff811047a0>] ? shrink_lruvec+0x510/0x510 [<ffffffff811047a0>] ? shrink_lruvec+0x510/0x510 [<ffffffff8106ef1e>] ? kthread+0x9e/0xb0 Signed-off-by: Andrea Arcangeli <aarcange@xxxxxxxxxx> --- mm/compaction.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/mm/compaction.c b/mm/compaction.c index 6066a35..0292984 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -633,7 +633,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone, /* Perform the isolation */ low_pfn = isolate_migratepages_range(zone, cc, low_pfn, end_pfn); - if (!low_pfn || cc->contended) + if (!low_pfn || (cc->contended && !cc->nr_migratepages)) return ISOLATE_ABORT; cc->migrate_pfn = low_pfn; @@ -843,6 +843,10 @@ static unsigned long compact_zone_order(struct zone *zone, INIT_LIST_HEAD(&cc.migratepages); ret = compact_zone(zone, &cc); + + VM_BUG_ON(!list_empty(&cc.freepages)); + VM_BUG_ON(!list_empty(&cc.migratepages)); + *contended = cc.contended; return ret; } -- 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>