Hi Andrea, On Mon, Jan 17, 2011 at 06:11:35AM +0100, Andrea Arcangeli wrote: > On Sun, Jan 16, 2011 at 09:41:41PM -0500, Chris Mason wrote: > > Excerpts from Andrew Morton's message of 2011-01-16 21:30:00 -0500: > > > (lots of cc's added) > > > > > > On Sun, 16 Jan 2011 21:07:40 -0500 Chris Mason <chris.mason@xxxxxxxxxx> wrote: > > > > > > > Excerpts from Linus Torvalds's message of 2011-01-16 20:53:04 -0500: > > > > > .. except I actually didn't add Andrew to the cc after all. > > > > > > > > > > NOW I did. > > > > > > > > > > Oh, and if you can repeat this and bisect it, it would obviously be > > > > > great. But that sounds rather painful. > > > > > > > > Ok, so I've got 3 different problems in 3 totally different areas. > > > > I'm running w/kvm, but this VM is very stable with 2.6.37. Running > > > > Linus' current git it goes boom in exotic ways, this time it was only on > > > > ext3, btrfs code never loaded. > > > > > > > > Linus, if you're planning on rc1 tonight I'll send my pull request out > > > > the door. Otherwise I'd prefer to fix this and send my pull after > > > > actually getting a long btrfs run on the current code. > > > > > > > > Next up, CONFIG_DEBUG*, always an adventure on rc1 kernels ;) > > > > > > > > WARNING: at lib/list_debug.c:57 list_del+0xc0/0xed() > > > > Hardware name: Bochs > > > > list_del corruption. next->prev should be ffffea000010cde0, but was ffff88007cff6bc8 > > > > Modules linked in: > > > > Pid: 524, comm: kswapd0 Not tainted 2.6.37-josef+ #180 > > > > Call Trace: > > > > [<ffffffff8106ec94>] ? warn_slowpath_common+0x85/0x9d > > > > [<ffffffff8106ed4f>] ? warn_slowpath_fmt+0x46/0x48 > > > > [<ffffffff81263d6c>] ? list_del+0xc0/0xed > > > > [<ffffffff81106d9d>] ? migrate_pages+0x26f/0x357 > > > > [<ffffffff81100e18>] ? compaction_alloc+0x0/0x2dc > > > > [<ffffffff8110150d>] ? compact_zone+0x391/0x5c4 > > > > [<ffffffff81101905>] ? compact_zone_order+0xc2/0xd1 > > > > [<ffffffff815c321e>] ? _raw_spin_unlock+0xe/0x10 > > > > [<ffffffff810dc446>] ? kswapd+0x5c8/0x88f > > > > [<ffffffff810dbe7e>] ? kswapd+0x0/0x88f > > > > [<ffffffff81089ce8>] ? kthread+0x82/0x8a > > > > [<ffffffff810347d4>] ? kernel_thread_helper+0x4/0x10 > > > > [<ffffffff81089c66>] ? kthread+0x0/0x8a > > > > [<ffffffff810347d0>] ? kernel_thread_helper+0x0/0x10 > > > > ---[ end trace 5c6b7933d16b301f ]--- > > > > > > uh-oh. Does disabling CONFIG_COMPACTION make this go away (requires > > > disabling CONFIG_TRANSPARENT_HUGEPAGE first). > > > > We'll see. I gave THP this same run of tests back in November, it > > passed without any problems (after fixing the related btrfs migration > > bug). All of the crashes I've seen this weekend had this in the > > .config: > > > > # CONFIG_TRANSPARENT_HUGEPAGE is not set > > CONFIG_COMPACTION=y > > CONFIG_MIGRATION=y > > I think it's unrelated but reading commit > cf608ac19c95804dc2df43b1f4f9e068aa9034ab if page_count(page) == 1 we > leave the page in the lru but we return 0 (so the caller of > migrate_pages won't call putback_lru_pages to actually free the page, Good catch. Totally It's my fault. I made linux memory hogger. :( Sorry for that. > however compaction would free it because it checks if the list is > empty and it ignores the migrate_pages retval). And in I want to change it with checking retval for consistency. > mm/memory-failure.c:1419, nobody is calling putback_lru_pages (it > seems a missing bit from that older patch). They seem just two memleak > unrelated to the above though. Nice point. Thanks fot notice me. It's strange in memory-failure.c:1419. I modified it at that time and didn't modified migrate_huge_pages(At that time, I didn't noticed hugepage migration so I missed it but I want to change it like migrate_pages) You can see my final patch in https://lkml.org/lkml/2010/8/24/248. It was changed. Hmm. The putback_lru_pages in soft_offline_huge_page makes critical BUG(early free) so we should fix it, too. We need 3 patch. 1. fix migrate_huge_pages(take out of put_page in migrate_huge_pages) for page corruption. 2. Your patch for memory leak 3. compaction retval check for putback lru pages for consistency Hmm. Could I make a patch for any kernel tree? Unfortunately I can make it out of office and we may spend 1 day at least. If it is urgent, could you make the patch for me? Thanks, Andrea. > > NOTE: with the last changes compaction is used for all order > 0 and > even from kswapd, so you will now be able to trigger bugs in > compaction or migration even with THP off. However I'm surprised that > you have issues with compaction... > > I'm posting this for Minchan to review (not meant for merging, untested). > > ====== > Subject: when migrate_pages returns 0, all pages must have been released > > From: Andrea Arcangeli <aarcange@xxxxxxxxxx> > > In some cases migrate_pages could return zero while still leaving a > few pages in the pagelist (and some caller wouldn't notice it has to > call putback_lru_pages). > > Add one missing putback_lru_pages not added by commit > cf608ac19c95804dc2df43b1f4f9e068aa9034ab. > > Signed-off-by: Andrea Arcangeli <aarcange@xxxxxxxxxx> > --- > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 548fbd7..75398b0 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1419,6 +1419,7 @@ int soft_offline_page(struct page *page, int flags) > ret = migrate_pages(&pagelist, new_page, MPOL_MF_MOVE_ALL, > 0, true); > if (ret) { > + putback_lru_pages(&pagelist); > pr_info("soft offline: %#lx: migration failed %d, type %lx\n", > pfn, ret, page->flags); > if (ret > 0) > diff --git a/mm/migrate.c b/mm/migrate.c > index 46fe8cc..bea2a34 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -611,6 +611,14 @@ static int move_to_new_page(struct page *newpage, struct page *page, > return rc; > } > > +static void unmap_and_move_release_page(struct page *page) > +{ > + list_del(&page->lru); > + dec_zone_page_state(page, NR_ISOLATED_ANON + > + page_is_file_cache(page)); > + putback_lru_page(page); > +} > + > /* > * Obtain the lock on page, remove all ptes and migrate the page > * to the newly allocated page in newpage. > @@ -631,11 +639,14 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private, > > if (page_count(page) == 1) { > /* page was freed from under us. So we are done. */ > + unmap_and_move_release_page(page); > goto move_newpage; > } > if (unlikely(PageTransHuge(page))) > - if (unlikely(split_huge_page(page))) > + if (unlikely(split_huge_page(page))) { > + unmap_and_move_release_page(page); > goto move_newpage; > + } > > /* prepare cgroup just returns 0 or -ENOMEM */ > rc = -EAGAIN; > @@ -779,10 +790,7 @@ unlock: > * migrated will have kepts its references and be > * restored. > */ > - list_del(&page->lru); > - dec_zone_page_state(page, NR_ISOLATED_ANON + > - page_is_file_cache(page)); > - putback_lru_page(page); > + unmap_and_move_release_page(page); > } > > move_newpage: -- Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>