From: zhong jiang <zhongjiang@huawei.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: akpm@linux-foundation.org, vbabka@suse.cz,
kirill.shutemov@linux.intel.com, hannes@cmpxchg.org,
mgorman@techsingularity.net, linux-mm@kvack.org,
kbuild-all@01.org
Subject: Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails (fwd)
Date: Wed, 10 May 2017 13:55:36 +0800 [thread overview]
Message-ID: <5912AB58.7020103@huawei.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1705092341330.3502@hadrien>
On 2017/5/9 23:43, Julia Lawall wrote:
> Hello,
>
> I don't know if there is a bug here, but it could e worth checking on. If
> the loop on line 1481 is executed, page will not be NULL at the out label
> on line 1560. Instead it will have a dummy value. Perhaps the value of
> result keeps the if at the out label from being taken.
>
> julia
Hi, Julia
it has no memory leak. so my initial thought is not correct. but I do not know you mean.
The page is local variable. it aybe a dummy value. but it should not cause any issue.
is it right? or I miss something.
Thanks
zhongjiang
> ---------- Forwarded message ----------
> Date: Tue, 9 May 2017 23:27:43 +0800
> From: kbuild test robot <fengguang.wu@intel.com>
> To: kbuild@01.org
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Subject: Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page
> fails
>
> Hi zhong,
>
> [auto build test WARNING on mmotm/master]
> [also build test WARNING on v4.11 next-20170509]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/zhongjiang/mm-fix-the-memory-leak-after-collapsing-the-huge-page-fails/20170509-193011
> base: git://git.cmpxchg.org/linux-mmotm.git master
> :::::: branch date: 4 hours ago
> :::::: commit date: 4 hours ago
>
>>> mm/khugepaged.c:1560:5-9: ERROR: invalid reference to the index variable of the iterator on line 1481
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout a5318ea654d5b764d6e06c6cfbfc21e44ce56e2b
> vim +1560 mm/khugepaged.c
>
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1475 struct zone *zone = page_zone(new_page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1476
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1477 /*
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1478 * Replacing old pages with new one has succeed, now we need to
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1479 * copy the content and free old pages.
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1480 */
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 @1481 list_for_each_entry_safe(page, tmp, &pagelist, lru) {
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1482 copy_highpage(new_page + (page->index % HPAGE_PMD_NR),
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1483 page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1484 list_del(&page->lru);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1485 unlock_page(page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1486 page_ref_unfreeze(page, 1);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1487 page->mapping = NULL;
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1488 ClearPageActive(page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1489 ClearPageUnevictable(page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1490 put_page(page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1491 }
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1492
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1493 local_irq_save(flags);
> 11fb9989 Mel Gorman 2016-07-28 1494 __inc_node_page_state(new_page, NR_SHMEM_THPS);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1495 if (nr_none) {
> 11fb9989 Mel Gorman 2016-07-28 1496 __mod_node_page_state(zone->zone_pgdat, NR_FILE_PAGES, nr_none);
> 11fb9989 Mel Gorman 2016-07-28 1497 __mod_node_page_state(zone->zone_pgdat, NR_SHMEM, nr_none);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1498 }
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1499 local_irq_restore(flags);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1500
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1501 /*
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1502 * Remove pte page tables, so we can re-faulti
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1503 * the page as huge.
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1504 */
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1505 retract_page_tables(mapping, start);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1506
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1507 /* Everything is ready, let's unfreeze the new_page */
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1508 set_page_dirty(new_page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1509 SetPageUptodate(new_page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1510 page_ref_unfreeze(new_page, HPAGE_PMD_NR);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1511 mem_cgroup_commit_charge(new_page, memcg, false, true);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1512 lru_cache_add_anon(new_page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1513 unlock_page(new_page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1514
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1515 *hpage = NULL;
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1516 } else {
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1517 /* Something went wrong: rollback changes to the radix-tree */
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1518 shmem_uncharge(mapping->host, nr_none);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1519 spin_lock_irq(&mapping->tree_lock);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1520 radix_tree_for_each_slot(slot, &mapping->page_tree, &iter,
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1521 start) {
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1522 if (iter.index >= end)
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1523 break;
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1524 page = list_first_entry_or_null(&pagelist,
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1525 struct page, lru);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1526 if (!page || iter.index < page->index) {
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1527 if (!nr_none)
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1528 break;
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1529 nr_none--;
> 59749e6c Johannes Weiner 2016-12-12 1530 /* Put holes back where they were */
> 59749e6c Johannes Weiner 2016-12-12 1531 radix_tree_delete(&mapping->page_tree,
> 59749e6c Johannes Weiner 2016-12-12 1532 iter.index);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1533 continue;
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1534 }
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1535
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1536 VM_BUG_ON_PAGE(page->index != iter.index, page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1537
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1538 /* Unfreeze the page. */
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1539 list_del(&page->lru);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1540 page_ref_unfreeze(page, 2);
> 6d75f366 Johannes Weiner 2016-12-12 1541 radix_tree_replace_slot(&mapping->page_tree,
> 6d75f366 Johannes Weiner 2016-12-12 1542 slot, page);
> 148deab2 Matthew Wilcox 2016-12-14 1543 slot = radix_tree_iter_resume(slot, &iter);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1544 spin_unlock_irq(&mapping->tree_lock);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1545 putback_lru_page(page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1546 unlock_page(page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1547 spin_lock_irq(&mapping->tree_lock);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1548 }
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1549 VM_BUG_ON(nr_none);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1550 spin_unlock_irq(&mapping->tree_lock);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1551
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1552 /* Unfreeze new_page, caller would take care about freeing it */
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1553 page_ref_unfreeze(new_page, 1);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1554 mem_cgroup_cancel_charge(new_page, memcg, true);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1555 unlock_page(new_page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1556 new_page->mapping = NULL;
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1557 }
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1558 out:
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1559 VM_BUG_ON(!list_empty(&pagelist));
> a5318ea6 zhong jiang 2017-05-09 @1560 if (page != NULL && result != SCAN_SUCCEED)
> a5318ea6 zhong jiang 2017-05-09 1561 put_page(new_page);
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1562 /* TODO: tracepoints */
> f3f0e1d2 Kirill A. Shutemov 2016-07-26 1563 }
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
> .
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-05-10 6:06 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-09 15:43 [PATCH v2] mm: fix the memory leak after collapsing the huge page fails (fwd) Julia Lawall
2017-05-10 5:55 ` zhong jiang [this message]
2017-05-10 6:25 ` Julia Lawall
2017-05-10 6:48 ` Vlastimil Babka
2017-05-10 6:50 ` Julia Lawall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5912AB58.7020103@huawei.com \
--to=zhongjiang@huawei.com \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=julia.lawall@lip6.fr \
--cc=kbuild-all@01.org \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=vbabka@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).