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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.