* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails
From: Vlastimil Babka @ 2017-05-10 6:51 UTC (permalink / raw)
To: zhong jiang; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm
In-Reply-To: <5911C9F9.3020802@huawei.com>
On 05/09/2017 03:54 PM, zhong jiang wrote:
> Hi, Vlastimil
>
> I review the code again. it works well for NUMA. because
> khugepaged_prealloc_page will put_page when *hpage is true.
>
> the memory leak will still exist in !NUMA. because it ingore
> the put_page. is it right? I miss something.
No, on !NUMA the preallocated and unused new_page is freed by put_page()
at the very end of khugepaged_do_scan().
> Thanks
> zhongjiang
>
> On 2017/5/9 20:41, Vlastimil Babka wrote:
>> On 05/09/2017 02:20 PM, zhong jiang wrote:
>>> On 2017/5/9 19:34, Vlastimil Babka wrote:
>>>> On 05/09/2017 12:55 PM, zhongjiang wrote:
>>>>> From: zhong jiang <zhongjiang@huawei.com>
>>>>>
>>>>> Current, when we prepare a huge page to collapse, due to some
>>>>> reasons, it can fail to collapse. At the moment, we should
>>>>> release the preallocate huge page.
>>>>>
>>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>>> Hmm, scratch that, there's no memory leak. The pointer to new_page is
>>>> stored in *hpage, and put_page() is called all the way up in
>>>> khugepaged_do_scan().
>>> I see. I miss it. but why the new_page need to be release all the way.
>> AFAIK to support preallocation and reusal of preallocated page for
>> collapse attempt in different pmd. It only works for !NUMA so it's
>> likely not worth all the trouble and complicated code, so I wouldn't be
>> opposed to simplifying this.
>>
>>> I do not see the count increment when scan success. it save the memory,
>>> only when page fault happen.
>> I don't understand what you mean here?
>>
>>> Thanks
>>> zhongjiang
>>>>> ---
>>>>> mm/khugepaged.c | 4 ++++
>>>>> 1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>>> index 7cb9c88..586b1f1 100644
>>>>> --- a/mm/khugepaged.c
>>>>> +++ b/mm/khugepaged.c
>>>>> @@ -1082,6 +1082,8 @@ static void collapse_huge_page(struct mm_struct *mm,
>>>>> up_write(&mm->mmap_sem);
>>>>> out_nolock:
>>>>> trace_mm_collapse_huge_page(mm, isolated, result);
>>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>>> + put_page(new_page);
>>>>> return;
>>>>> out:
>>>>> mem_cgroup_cancel_charge(new_page, memcg, true);
>>>>> @@ -1555,6 +1557,8 @@ static void collapse_shmem(struct mm_struct *mm,
>>>>> }
>>>>> out:
>>>>> VM_BUG_ON(!list_empty(&pagelist));
>>>>> + if (page != NULL && result != SCAN_SUCCEED)
>>>>> + put_page(new_page);
>>>>> /* TODO: tracepoints */
>>>>> }
>>>>>
>>>>>
>>>> .
>>>>
>>>
>>
>> .
>>
>
>
--
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>
^ permalink raw reply
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails (fwd)
From: Julia Lawall @ 2017-05-10 6:50 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Julia Lawall, zhong jiang, akpm, kirill.shutemov, hannes, mgorman,
linux-mm, kbuild-all
In-Reply-To: <951ad516-b8da-8277-d4ad-141ba3b47bec@suse.cz>
On Wed, 10 May 2017, Vlastimil Babka wrote:
> On 05/10/2017 08:25 AM, Julia Lawall wrote:
> >
> >
> > On Wed, 10 May 2017, zhong jiang wrote:
> >
> >> 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.
> >
> > I had first been thinking that the if branch was referencing page. In
> > that case, if page were a dummy value, then there could be a problem. But
> > now I see that the branch does not refer to page. So the question is
> > just, if the loop on lines 1481-1491 is executed, is it correct to execute
> > the code put_page(new_page)? Or will result be SCAN_SUCCEEDED in that
> > case?
>
> That loop is under "if (result == SCAN_SUCCEED)", so yeah. It also ends
> with "*hpage = NULL;", so the put_page(hpage) in khugepaged_do_scan()
> won't apply. I see no problem besides the very non-obvious code :/
OK, thanks for the clarification.
julia
--
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>
^ permalink raw reply
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails (fwd)
From: Vlastimil Babka @ 2017-05-10 6:48 UTC (permalink / raw)
To: Julia Lawall, zhong jiang
Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm, kbuild-all
In-Reply-To: <alpine.DEB.2.20.1705101421530.20213@hadrien>
On 05/10/2017 08:25 AM, Julia Lawall wrote:
>
>
> On Wed, 10 May 2017, zhong jiang wrote:
>
>> 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.
>
> I had first been thinking that the if branch was referencing page. In
> that case, if page were a dummy value, then there could be a problem. But
> now I see that the branch does not refer to page. So the question is
> just, if the loop on lines 1481-1491 is executed, is it correct to execute
> the code put_page(new_page)? Or will result be SCAN_SUCCEEDED in that
> case?
That loop is under "if (result == SCAN_SUCCEED)", so yeah. It also ends
with "*hpage = NULL;", so the put_page(hpage) in khugepaged_do_scan()
won't apply. I see no problem besides the very non-obvious code :/
--
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>
^ permalink raw reply
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails (fwd)
From: Julia Lawall @ 2017-05-10 6:25 UTC (permalink / raw)
To: zhong jiang
Cc: akpm, vbabka, kirill.shutemov, hannes, mgorman, linux-mm,
kbuild-all
In-Reply-To: <5912AB58.7020103@huawei.com>
On Wed, 10 May 2017, zhong jiang wrote:
> 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.
I had first been thinking that the if branch was referencing page. In
that case, if page were a dummy value, then there could be a problem. But
now I see that the branch does not refer to page. So the question is
just, if the loop on lines 1481-1491 is executed, is it correct to execute
the code put_page(new_page)? Or will result be SCAN_SUCCEEDED in that
case?
julia
>
> 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>
^ permalink raw reply
* Re: [PATCH] vmscan: scan pages until it founds eligible pages
From: Michal Hocko @ 2017-05-10 6:13 UTC (permalink / raw)
To: Minchan Kim
Cc: Johannes Weiner, Andrew Morton, Mel Gorman, kernel-team,
linux-kernel, linux-mm
In-Reply-To: <20170510014654.GA23584@bbox>
On Wed 10-05-17 10:46:54, Minchan Kim wrote:
> On Wed, May 03, 2017 at 08:00:44AM +0200, Michal Hocko wrote:
[...]
> > @@ -1486,6 +1486,12 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> > continue;
> > }
> >
> > + /*
> > + * Do not count skipped pages because we do want to isolate
> > + * some pages even when the LRU mostly contains ineligible
> > + * pages
> > + */
>
> How about adding comment about "why"?
>
> /*
> * Do not count skipped pages because it makes the function to return with
> * none isolated pages if the LRU mostly contains inelgible pages so that
> * VM cannot reclaim any pages and trigger premature OOM.
> */
I am not sure this is necessarily any better. Mentioning a pre-mature
OOM would require a much better explanation because a first immediate
question would be "why don't we scan those pages at priority 0". Also
decision about the OOM is at a different layer and it might change in
future when this doesn't apply any more. But it is not like I would
insist...
> > + scan++;
> > switch (__isolate_lru_page(page, mode)) {
> > case 0:
> > nr_pages = hpage_nr_pages(page);
>
> Confirmed.
Hmm. I can clearly see how we could skip over too many pages and hit
small reclaim priorities too quickly but I am still scratching my head
about how we could hit the OOM killer as a result. The amount of pages
on the active anonymous list suggests that we are not able to rotate
pages quickly enough. I have to keep thinking about that.
> It works as expected but it changed scan counter's behavior. How
> about this?
OK, it looks good to me. I believe the main motivation of the original
patch from Johannes was to drop the magical total_skipped.
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 2314aca47d12..846922d7942e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1469,7 +1469,7 @@ static __always_inline void update_lru_sizes(struct lruvec *lruvec,
> *
> * Appropriate locks must be held before calling this function.
> *
> - * @nr_to_scan: The number of pages to look through on the list.
> + * @nr_to_scan: The number of eligible pages to look through on the list.
> * @lruvec: The LRU vector to pull pages from.
> * @dst: The temp list to put pages on to.
> * @nr_scanned: The number of pages that were scanned.
> @@ -1489,11 +1489,13 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> unsigned long nr_zone_taken[MAX_NR_ZONES] = { 0 };
> unsigned long nr_skipped[MAX_NR_ZONES] = { 0, };
> unsigned long skipped = 0;
> - unsigned long scan, nr_pages;
> + unsigned long scan, total_scan, nr_pages;
> LIST_HEAD(pages_skipped);
>
> - for (scan = 0; scan < nr_to_scan && nr_taken < nr_to_scan &&
> - !list_empty(src); scan++) {
> + for (total_scan = scan = 0; scan < nr_to_scan &&
> + nr_taken < nr_to_scan &&
> + !list_empty(src);
> + total_scan++) {
> struct page *page;
>
> page = lru_to_page(src);
> @@ -1507,6 +1509,13 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> continue;
> }
>
> + /*
> + * Do not count skipped pages because it makes the function to
> + * return with none isolated pages if the LRU mostly contains
> + * inelgible pages so that VM cannot reclaim any pages and
> + * trigger premature OOM.
> + */
> + scan++;
> switch (__isolate_lru_page(page, mode)) {
> case 0:
> nr_pages = hpage_nr_pages(page);
> @@ -1544,9 +1553,9 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> skipped += nr_skipped[zid];
> }
> }
> - *nr_scanned = scan;
> + *nr_scanned = total_scan;
> trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
> - scan, skipped, nr_taken, mode, lru);
> + total_scan, skipped, nr_taken, mode, lru);
> update_lru_sizes(lruvec, lru, nr_zone_taken);
> return nr_taken;
> }
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails (fwd)
From: zhong jiang @ 2017-05-10 5:55 UTC (permalink / raw)
To: Julia Lawall
Cc: akpm, vbabka, kirill.shutemov, hannes, mgorman, linux-mm,
kbuild-all
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>
^ permalink raw reply
* Re: [RFC 09/10] x86/mm: Rework lazy TLB to track the actual loaded mm
From: Ingo Molnar @ 2017-05-10 5:57 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Andy Lutomirski, X86 ML, linux-kernel@vger.kernel.org,
Borislav Petkov, Linus Torvalds, Andrew Morton, Mel Gorman,
linux-mm@kvack.org, Rik van Riel, Dave Hansen, Nadav Amit,
Michal Hocko, Arjan van de Ven
In-Reply-To: <alpine.DEB.2.20.1705092236290.2295@nanos>
* Thomas Gleixner <tglx@linutronix.de> wrote:
> On Sun, 7 May 2017, Andy Lutomirski wrote:
> > /* context.lock is held for us, so we don't need any locking. */
> > static void flush_ldt(void *current_mm)
> > {
> > + struct mm_struct *mm = current_mm;
> > mm_context_t *pc;
> >
> > - if (current->active_mm != current_mm)
> > + if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
>
> While functional correct, this really should compare against 'mm'.
>
> > return;
> >
> > - pc = ¤t->active_mm->context;
> > + pc = &mm->context;
So this appears to be the function:
static void flush_ldt(void *current_mm)
{
struct mm_struct *mm = current_mm;
mm_context_t *pc;
if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
return;
pc = &mm->context;
set_ldt(pc->ldt->entries, pc->ldt->size);
}
why not rename 'current_mm' to 'mm' and remove the 'mm' local variable?
Thanks,
Ingo
--
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>
^ permalink raw reply
* Re: [PATCH -v3 0/13] mm: make movable onlining suck less
From: Michal Hocko @ 2017-05-10 5:53 UTC (permalink / raw)
To: Dan Williams
Cc: linux-mm, Andrew Morton, Mel Gorman, Vlastimil Babka,
Andrea Arcangeli, Jerome Glisse, Reza Arbab, Yasuaki Ishimatsu,
qiuxishi, Kani Toshimitsu, slaoub, Joonsoo Kim, Andi Kleen,
David Rientjes, Daniel Kiper, Igor Mammedov, Vitaly Kuznetsov,
LKML, Balbir Singh, Heiko Carstens, Martin Schwidefsky,
Tobias Regnery, Yasuaki Ishimatsu
In-Reply-To: <CAA9_cmexLPT4m_TEh69fC_OqBD4n4bND-vz33qoKSgXm_Q72Cw@mail.gmail.com>
On Tue 09-05-17 21:43:16, Dan Williams wrote:
> On Fri, Apr 21, 2017 at 5:05 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > Hi,
> > The last version of this series has been posted here [1]. It has seen
> > some more testing (thanks to Reza Arbab and Igor Mammedov[2]), Jerome's
> > and Vlastimil's review resulted in few fixes mostly folded in their
> > respected patches.
> > There are 4 more patches (patch 6+ in this series). I have checked the
> > most prominent pfn walkers to skip over offline holes and now and I feel
> > more comfortable to have this merged. All the reported issues should be
> > fixed
> >
> > There is still a lot of work on top - namely this implementation doesn't
> > support reonlining to a different zone on the zones boundaries but I
> > will do that in a separate series because this one is getting quite
> > large already and it should work reasonably well now.
> >
> > Joonsoo had some worries about pfn_valid and suggested to change its
> > semantic to return false on offline holes but I would be rally worried
> > to change a established semantic used by a lot of code and so I have
> > introuduced pfn_to_online_page helper instead. If this is seen as a
> > controversial point I would rather drop pfn_to_online_page and related
> > patches as they are not stictly necessary because the code would be
> > similarly broken as now wrt. offline holes.
> >
> > This is a rebase on top of linux-next (next-20170418) and the full
> > series is in git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
> > try attempts/rewrite-mem_hotplug branch.
> >
> [..]
> > Any thoughts, complains, suggestions?
> >
> > As a bonus we will get a nice cleanup in the memory hotplug codebase.
> > arch/ia64/mm/init.c | 11 +-
> > arch/powerpc/mm/mem.c | 12 +-
> > arch/s390/mm/init.c | 32 +--
> > arch/sh/mm/init.c | 10 +-
> > arch/x86/mm/init_32.c | 7 +-
> > arch/x86/mm/init_64.c | 11 +-
> > drivers/base/memory.c | 79 +++----
> > drivers/base/node.c | 58 ++----
> > include/linux/memory_hotplug.h | 40 +++-
> > include/linux/mmzone.h | 44 +++-
> > include/linux/node.h | 35 +++-
> > kernel/memremap.c | 6 +-
> > mm/compaction.c | 5 +-
> > mm/memory_hotplug.c | 455 ++++++++++++++---------------------------
> > mm/page_alloc.c | 13 +-
> > mm/page_isolation.c | 26 ++-
> > mm/sparse.c | 48 ++++-
> > 17 files changed, 407 insertions(+), 485 deletions(-)
> >
> > Shortlog says:
> > Michal Hocko (13):
> > mm: remove return value from init_currently_empty_zone
> > mm, memory_hotplug: use node instead of zone in can_online_high_movable
> > mm: drop page_initialized check from get_nid_for_pfn
> > mm, memory_hotplug: get rid of is_zone_device_section
> > mm, memory_hotplug: split up register_one_node
> > mm, memory_hotplug: consider offline memblocks removable
> > mm: consider zone which is not fully populated to have holes
> > mm, compaction: skip over holes in __reset_isolation_suitable
> > mm: __first_valid_page skip over offline pages
> > mm, memory_hotplug: do not associate hotadded memory to zones until online
> > mm, memory_hotplug: replace for_device by want_memblock in arch_add_memory
> > mm, memory_hotplug: fix the section mismatch warning
> > mm, memory_hotplug: remove unused cruft after memory hotplug rework
> >
> > [1] http://lkml.kernel.org/r/20170410110351.12215-1-mhocko@kernel.org
> > [2] http://lkml.kernel.org/r/20170410162749.7d7f31c1@nial.brq.redhat.com
> >
> >
>
> The latest "attempts/rewrite-mem_hotplug" branch passes my regression
> testing if I cherry-pick the following x86/mm fixes from mainline:
>
> e6ab9c4d4377 x86/mm/64: Fix crash in remove_pagetable()
> 71389703839e mm, zone_device: Replace {get, put}_zone_device_page()
> with a single reference to fix pmem crash
I will make sure those will appear in the mmotm git tree (I will
probably pull the whole tip/x86-mm-for-linus.
> You can add:
>
> Tested-by: Dan Williams <dan.j.williams@intel.com>
Thanks a lot for your testing! I will put your tested-by to patches
where you were on the CC explicitly (and which might affect zone device)
- mm, memory_hotplug: get rid of is_zone_device_section
- mm, memory_hotplug: do not associate hotadded memory to zones until
online
- mm, memory_hotplug: replace for_device by want_memblock in
arch_add_memory
Let me know if you want other patches as well.
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply
* Re: [PATCH -v3 0/13] mm: make movable onlining suck less
From: Dan Williams @ 2017-05-10 4:43 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, Mel Gorman, Vlastimil Babka,
Andrea Arcangeli, Jerome Glisse, Reza Arbab, Yasuaki Ishimatsu,
qiuxishi, Kani Toshimitsu, slaoub, Joonsoo Kim, Andi Kleen,
David Rientjes, Daniel Kiper, Igor Mammedov, Vitaly Kuznetsov,
LKML, Balbir Singh, Heiko Carstens, Martin Schwidefsky,
Michal Hocko, Tobias Regnery, Yasuaki Ishimatsu
In-Reply-To: <20170421120512.23960-1-mhocko@kernel.org>
On Fri, Apr 21, 2017 at 5:05 AM, Michal Hocko <mhocko@kernel.org> wrote:
> Hi,
> The last version of this series has been posted here [1]. It has seen
> some more testing (thanks to Reza Arbab and Igor Mammedov[2]), Jerome's
> and Vlastimil's review resulted in few fixes mostly folded in their
> respected patches.
> There are 4 more patches (patch 6+ in this series). I have checked the
> most prominent pfn walkers to skip over offline holes and now and I feel
> more comfortable to have this merged. All the reported issues should be
> fixed
>
> There is still a lot of work on top - namely this implementation doesn't
> support reonlining to a different zone on the zones boundaries but I
> will do that in a separate series because this one is getting quite
> large already and it should work reasonably well now.
>
> Joonsoo had some worries about pfn_valid and suggested to change its
> semantic to return false on offline holes but I would be rally worried
> to change a established semantic used by a lot of code and so I have
> introuduced pfn_to_online_page helper instead. If this is seen as a
> controversial point I would rather drop pfn_to_online_page and related
> patches as they are not stictly necessary because the code would be
> similarly broken as now wrt. offline holes.
>
> This is a rebase on top of linux-next (next-20170418) and the full
> series is in git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
> try attempts/rewrite-mem_hotplug branch.
>
[..]
> Any thoughts, complains, suggestions?
>
> As a bonus we will get a nice cleanup in the memory hotplug codebase.
> arch/ia64/mm/init.c | 11 +-
> arch/powerpc/mm/mem.c | 12 +-
> arch/s390/mm/init.c | 32 +--
> arch/sh/mm/init.c | 10 +-
> arch/x86/mm/init_32.c | 7 +-
> arch/x86/mm/init_64.c | 11 +-
> drivers/base/memory.c | 79 +++----
> drivers/base/node.c | 58 ++----
> include/linux/memory_hotplug.h | 40 +++-
> include/linux/mmzone.h | 44 +++-
> include/linux/node.h | 35 +++-
> kernel/memremap.c | 6 +-
> mm/compaction.c | 5 +-
> mm/memory_hotplug.c | 455 ++++++++++++++---------------------------
> mm/page_alloc.c | 13 +-
> mm/page_isolation.c | 26 ++-
> mm/sparse.c | 48 ++++-
> 17 files changed, 407 insertions(+), 485 deletions(-)
>
> Shortlog says:
> Michal Hocko (13):
> mm: remove return value from init_currently_empty_zone
> mm, memory_hotplug: use node instead of zone in can_online_high_movable
> mm: drop page_initialized check from get_nid_for_pfn
> mm, memory_hotplug: get rid of is_zone_device_section
> mm, memory_hotplug: split up register_one_node
> mm, memory_hotplug: consider offline memblocks removable
> mm: consider zone which is not fully populated to have holes
> mm, compaction: skip over holes in __reset_isolation_suitable
> mm: __first_valid_page skip over offline pages
> mm, memory_hotplug: do not associate hotadded memory to zones until online
> mm, memory_hotplug: replace for_device by want_memblock in arch_add_memory
> mm, memory_hotplug: fix the section mismatch warning
> mm, memory_hotplug: remove unused cruft after memory hotplug rework
>
> [1] http://lkml.kernel.org/r/20170410110351.12215-1-mhocko@kernel.org
> [2] http://lkml.kernel.org/r/20170410162749.7d7f31c1@nial.brq.redhat.com
>
>
The latest "attempts/rewrite-mem_hotplug" branch passes my regression
testing if I cherry-pick the following x86/mm fixes from mainline:
e6ab9c4d4377 x86/mm/64: Fix crash in remove_pagetable()
71389703839e mm, zone_device: Replace {get, put}_zone_device_page()
with a single reference to fix pmem crash
You can add:
Tested-by: Dan Williams <dan.j.williams@intel.com>
--
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>
^ permalink raw reply
* Re: [PATCH -mm] mm, swap: Remove unused function prototype
From: Wei Yang @ 2017-05-10 2:57 UTC (permalink / raw)
To: Huang, Ying; +Cc: Andrew Morton, linux-mm, linux-kernel, Tim Chen
In-Reply-To: <20170405071017.23677-1-ying.huang@intel.com>
[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]
On Wed, Apr 05, 2017 at 03:10:17PM +0800, Huang, Ying wrote:
>From: Huang Ying <ying.huang@intel.com>
>
>This is a code cleanup patch, no functionality changes. There are 2
>unused function prototype in swap.h, they are removed.
>
>Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>Cc: Tim Chen <tim.c.chen@linux.intel.com>
>---
> include/linux/swap.h | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/include/linux/swap.h b/include/linux/swap.h
>index 486494e6b2fc..ba5882419a7d 100644
>--- a/include/linux/swap.h
>+++ b/include/linux/swap.h
>@@ -411,9 +411,6 @@ struct backing_dev_info;
> extern int init_swap_address_space(unsigned int type, unsigned long nr_pages);
> extern void exit_swap_address_space(unsigned int type);
>
>-extern int get_swap_slots(int n, swp_entry_t *slots);
>-extern void swapcache_free_batch(swp_entry_t *entries, int n);
>-
> #else /* CONFIG_SWAP */
>
> #define swap_address_space(entry) (NULL)
>--
>2.11.0
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] oom: improve oom disable handling
From: Wei Yang @ 2017-05-10 2:47 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Vladimir Davydov, Tetsuo Handa, linux-mm, LKML,
Michal Hocko
In-Reply-To: <20170404134705.6361-1-mhocko@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3272 bytes --]
Hi, Michal
If the subject is "improve the log message on oom disable handling" would be
more accurate to me.
The original subject sounds the patch will adjust the code path, while it
doesn't.
On Tue, Apr 04, 2017 at 03:47:05PM +0200, Michal Hocko wrote:
>From: Michal Hocko <mhocko@suse.com>
>
>Tetsuo has reported that sysrq triggered OOM killer will print a
>misleading information when no tasks are selected:
>
>[ 713.805315] sysrq: SysRq : Manual OOM execution
>[ 713.808920] Out of memory: Kill process 4468 ((agetty)) score 0 or sacrifice child
>[ 713.814913] Killed process 4468 ((agetty)) total-vm:43704kB, anon-rss:1760kB, file-rss:0kB, shmem-rss:0kB
>[ 714.004805] sysrq: SysRq : Manual OOM execution
>[ 714.005936] Out of memory: Kill process 4469 (systemd-cgroups) score 0 or sacrifice child
>[ 714.008117] Killed process 4469 (systemd-cgroups) total-vm:10704kB, anon-rss:120kB, file-rss:0kB, shmem-rss:0kB
>[ 714.189310] sysrq: SysRq : Manual OOM execution
>[ 714.193425] sysrq: OOM request ignored because killer is disabled
>[ 714.381313] sysrq: SysRq : Manual OOM execution
>[ 714.385158] sysrq: OOM request ignored because killer is disabled
>[ 714.573320] sysrq: SysRq : Manual OOM execution
>[ 714.576988] sysrq: OOM request ignored because killer is disabled
>
>The real reason is that there are no eligible tasks for the OOM killer
>to select but since 7c5f64f84483bd13 ("mm: oom: deduplicate victim
>selection code for memcg and global oom") the semantic of out_of_memory
>has changed without updating moom_callback.
>
>This patch updates moom_callback to tell that no task was eligible
>which is the case for both oom killer disabled and no eligible tasks.
>In order to help distinguish first case from the second add printk to
>both oom_killer_{enable,disable}. This information is useful on its own
>because it might help debugging potential memory allocation failures.
>
>Fixes: 7c5f64f84483bd13 ("mm: oom: deduplicate victim selection code for memcg and global oom")
>Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>Signed-off-by: Michal Hocko <mhocko@suse.com>
>---
> drivers/tty/sysrq.c | 2 +-
> mm/oom_kill.c | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
>index 71136742e606..a91f58dc2cb6 100644
>--- a/drivers/tty/sysrq.c
>+++ b/drivers/tty/sysrq.c
>@@ -370,7 +370,7 @@ static void moom_callback(struct work_struct *ignored)
>
> mutex_lock(&oom_lock);
> if (!out_of_memory(&oc))
>- pr_info("OOM request ignored because killer is disabled\n");
>+ pr_info("OOM request ignored. No task eligible\n");
> mutex_unlock(&oom_lock);
> }
>
>diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>index 51c091849dcb..ad2b112cdf3e 100644
>--- a/mm/oom_kill.c
>+++ b/mm/oom_kill.c
>@@ -682,6 +682,7 @@ void exit_oom_victim(void)
> void oom_killer_enable(void)
> {
> oom_killer_disabled = false;
>+ pr_info("OOM killer enabled.\n");
> }
>
> /**
>@@ -718,6 +719,7 @@ bool oom_killer_disable(signed long timeout)
> oom_killer_enable();
> return false;
> }
>+ pr_info("OOM killer disabled.\n");
>
> return true;
> }
>--
>2.11.0
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] mm: fix spelling error
From: Wei Yang @ 2017-05-10 2:38 UTC (permalink / raw)
To: Hao Lee
Cc: akpm, alexander.h.duyck, mhocko, vbabka, mgorman, l.stach,
vdavydov.dev, linux-mm, linux-kernel
In-Reply-To: <20170403161655.5081-1-haolee.swjtu@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]
On Tue, Apr 04, 2017 at 12:16:55AM +0800, Hao Lee wrote:
>Fix variable name error in comments. No code changes.
>
>Signed-off-by: Hao Lee <haolee.swjtu@gmail.com>
>---
> include/linux/gfp.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>index db373b9..ff3d651 100644
>--- a/include/linux/gfp.h
>+++ b/include/linux/gfp.h
>@@ -297,8 +297,8 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
>
> /*
> * GFP_ZONE_TABLE is a word size bitstring that is used for looking up the
>- * zone to use given the lowest 4 bits of gfp_t. Entries are ZONE_SHIFT long
>- * and there are 16 of them to cover all possible combinations of
>+ * zone to use given the lowest 4 bits of gfp_t. Entries are GFP_ZONES_SHIFT
>+ * bits long and there are 16 of them to cover all possible combinations of
> * __GFP_DMA, __GFP_DMA32, __GFP_MOVABLE and __GFP_HIGHMEM.
> *
> * The zone fallback order is MOVABLE=>HIGHMEM=>NORMAL=>DMA32=>DMA.
Looks good to me.
Acked-by: Wei Yang <richard.weiyang@gmail.com>
>--
>2.9.3
--
Wei Yang
Help you, Help me
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RESENT PATCH] x86/mem: fix the offset overflow when read/write mem
From: Xishi Qiu @ 2017-05-10 2:15 UTC (permalink / raw)
To: Rik van Riel
Cc: zhong jiang, David Rientjes, Bjorn Helgaas, Yoshinori Sato,
Rich Felker, Andrew Morton, arnd, hannes, kirill, mgorman, hughd,
linux-mm, linux-kernel
In-Reply-To: <1494344803.20270.27.camel@redhat.com>
On 2017/5/9 23:46, Rik van Riel wrote:
> On Thu, 2017-05-04 at 10:28 +0800, zhong jiang wrote:
>> On 2017/5/4 2:46, Rik van Riel wrote:
>
>>> However, it is not as easy as simply checking the
>>> end against __pa(high_memory). Some systems have
>>> non-contiguous physical memory ranges, with gaps
>>> of invalid addresses in-between.
>>
>> The invalid physical address means that it is used as
>> io mapped. not in system ram region. /dev/mem is not
>> access to them , is it right?
>
> Not necessarily. Some systems simply have large
> gaps in physical memory access. Their memory map
> may look like this:
>
> |MMMMMM|IO|MMMM|..................|MMMMMMMM|
>
> Where M is memory, IO is IO space, and the
> dots are simply a gap in physical address
> space with no valid accesses at all.
>
Hi Rik,
Do you mean IO space is allowed to access from mmap /dev/mem?
Thanks,
Xishi Qiu
>>> At that point, is the complexity so much that it no
>>> longer makes sense to try to protect against root
>>> crashing the system?
>>>
>>
>> your suggestion is to let the issue along without any protection.
>> just root user know what they are doing.
>
> Well, root already has other ways to crash the system.
>
> Implementing validation on /dev/mem may make sense if
> it can be done in a simple way, but may not be worth
> it if it becomes too complex.
>
--
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>
^ permalink raw reply
* Re: [RESENT PATCH] x86/mem: fix the offset overflow when read/write mem
From: zhong jiang @ 2017-05-10 2:14 UTC (permalink / raw)
To: Rik van Riel
Cc: David Rientjes, Bjorn Helgaas, Yoshinori Sato, Rich Felker,
Andrew Morton, arnd, hannes, kirill, mgorman, hughd, linux-mm,
linux-kernel, Xishi Qiu
In-Reply-To: <1494344803.20270.27.camel@redhat.com>
On 2017/5/9 23:46, Rik van Riel wrote:
> On Thu, 2017-05-04 at 10:28 +0800, zhong jiang wrote:
>> On 2017/5/4 2:46, Rik van Riel wrote:
>>> However, it is not as easy as simply checking the
>>> end against __pa(high_memory). Some systems have
>>> non-contiguous physical memory ranges, with gaps
>>> of invalid addresses in-between.
>> The invalid physical address means that it is used as
>> io mapped. not in system ram region. /dev/mem is not
>> access to them , is it right?
> Not necessarily. Some systems simply have large
> gaps in physical memory access. Their memory map
> may look like this:
>
> |MMMMMM|IO|MMMM|..................|MMMMMMMM|
>
> Where M is memory, IO is IO space, and the
> dots are simply a gap in physical address
> space with no valid accesses at all.
>
>>> At that point, is the complexity so much that it no
>>> longer makes sense to try to protect against root
>>> crashing the system?
>>>
>> your suggestion is to let the issue along without any protection.
>> just root user know what they are doing.
> Well, root already has other ways to crash the system.
>
> Implementing validation on /dev/mem may make sense if
> it can be done in a simple way, but may not be worth
> it if it becomes too complex.
>
I have no a simple way to fix. Do you any suggestion. or you can send
a patch for me ?
Thanks
zhongjiang
--
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>
^ permalink raw reply
* Re: [PATCH] vmscan: scan pages until it founds eligible pages
From: Minchan Kim @ 2017-05-10 1:46 UTC (permalink / raw)
To: Michal Hocko
Cc: Johannes Weiner, Andrew Morton, Mel Gorman, kernel-team,
linux-kernel, linux-mm
In-Reply-To: <20170503060044.GA1236@dhcp22.suse.cz>
On Wed, May 03, 2017 at 08:00:44AM +0200, Michal Hocko wrote:
> On Wed 03-05-17 13:48:09, Minchan Kim wrote:
> > On Tue, May 02, 2017 at 05:14:36PM +0200, Michal Hocko wrote:
> > > On Tue 02-05-17 23:51:50, Minchan Kim wrote:
> > > > Hi Michal,
> > > >
> > > > On Tue, May 02, 2017 at 09:54:32AM +0200, Michal Hocko wrote:
> > > > > On Tue 02-05-17 14:14:52, Minchan Kim wrote:
> > > > > > Oops, forgot to add lkml and linux-mm.
> > > > > > Sorry for that.
> > > > > > Send it again.
> > > > > >
> > > > > > >From 8ddf1c8aa15baf085bc6e8c62ce705459d57ea4c Mon Sep 17 00:00:00 2001
> > > > > > From: Minchan Kim <minchan@kernel.org>
> > > > > > Date: Tue, 2 May 2017 12:34:05 +0900
> > > > > > Subject: [PATCH] vmscan: scan pages until it founds eligible pages
> > > > > >
> > > > > > On Tue, May 02, 2017 at 01:40:38PM +0900, Minchan Kim wrote:
> > > > > > There are premature OOM happening. Although there are a ton of free
> > > > > > swap and anonymous LRU list of elgible zones, OOM happened.
> > > > > >
> > > > > > With investigation, skipping page of isolate_lru_pages makes reclaim
> > > > > > void because it returns zero nr_taken easily so LRU shrinking is
> > > > > > effectively nothing and just increases priority aggressively.
> > > > > > Finally, OOM happens.
> > > > >
> > > > > I am not really sure I understand the problem you are facing. Could you
> > > > > be more specific please? What is your configuration etc...
> > > >
> > > > Sure, KVM guest on x86_64, It has 2G memory and 1G swap and configured
> > > > movablecore=1G to simulate highmem zone.
> > > > Workload is a process consumes 2.2G memory and then random touch the
> > > > address space so it makes lots of swap in/out.
> > > >
> > > > >
> > > > > > balloon invoked oom-killer: gfp_mask=0x17080c0(GFP_KERNEL_ACCOUNT|__GFP_ZERO|__GFP_NOTRACK), nodemask=(null), order=0, oom_score_adj=0
> > > > > [...]
> > > > > > Node 0 active_anon:1698864kB inactive_anon:261256kB active_file:208kB inactive_file:184kB unevictable:0kB isolated(anon):0kB isolated(file):0kB mapped:532kB dirty:108kB writeback:0kB shmem:172kB writeback_tmp:0kB unstable:0kB all_unreclaimable? no
> > > > > > DMA free:7316kB min:32kB low:44kB high:56kB active_anon:8064kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB present:15992kB managed:15908kB mlocked:0kB slab_reclaimable:464kB slab_unreclaimable:40kB kernel_stack:0kB pagetables:24kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> > > > > > lowmem_reserve[]: 0 992 992 1952
> > > > > > DMA32 free:9088kB min:2048kB low:3064kB high:4080kB active_anon:952176kB inactive_anon:0kB active_file:36kB inactive_file:0kB unevictable:0kB writepending:88kB present:1032192kB managed:1019388kB mlocked:0kB slab_reclaimable:13532kB slab_unreclaimable:16460kB kernel_stack:3552kB pagetables:6672kB bounce:0kB free_pcp:56kB local_pcp:24kB free_cma:0kB
> > > > > > lowmem_reserve[]: 0 0 0 959
> > > > >
> > > > > Hmm DMA32 has sufficient free memory to allow this order-0 request.
> > > > > Inactive anon lru is basically empty. Why do not we rotate a really
> > > > > large active anon list? Isn't this the primary problem?
> > > >
> > > > It's a side effect by skipping page logic in isolate_lru_pages
> > > > I mentioned above in changelog.
> > > >
> > > > The problem is a lot of anonymous memory in movable zone(ie, highmem)
> > > > and non-small memory in DMA32 zone.
> > >
> > > Such a configuration is questionable on its own. But let't keep this
> > > part alone.
> >
> > It seems you are misunderstood. It's really common on 32bit.
>
> Yes, I am not arguing about 32b systems. It is quite common to see
> issues which are inherent to the highmem zone.
>
> > Think of 2G DRAM system on 32bit. Normally, it's 1G normal:1G highmem.
> > It's almost same with one I configured.
> >
> > >
> > > > In heavy memory pressure,
> > > > requesting a page in GFP_KERNEL triggers reclaim. VM knows inactive list
> > > > is low so it tries to deactivate pages. For it, first of all, it tries
> > > > to isolate pages from active list but there are lots of anonymous pages
> > > > from movable zone so skipping logic in isolate_lru_pages works. With
> > > > the result, isolate_lru_pages cannot isolate any eligible pages so
> > > > reclaim trial is effectively void. It continues to meet OOM.
> > >
> > > But skipped pages should be rotated and we should eventually hit pages
> > > from the right zone(s). Moreover we should scan the full LRU at priority
> > > 0 so why exactly we hit the OOM killer?
> >
> > Yes, full scan in priority 0 but keep it in mind that the number of full
> > LRU pages to scan is one of eligible pages, not all pages of the node.
>
> I have hard time understanding what you are trying to say here.
>
> > And isolate_lru_pages have accounted skipped pages as scan count so that
> > VM cannot isolate any pages of eligible pages in LRU if non-eligible pages
> > are a lot in the LRU.
> >
> > >
> > > Anyway [1] has changed this behavior. Are you seeing the issue with this
> > > patch dropped?
> >
> > Good point. Before the patch, it didn't increase scan count with skipped
> > pages so with reverting [1], I guess it might work but worry about
> > isolating lots of skipped pages into temporal pages_skipped list which
> > might causes premate OOM. Anyway, I will test it when I returns at
> > office after vacation.
>
> I do not think we want to drop this patch. I think we might be good
> enough to simply fold this into the patch
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 24efcc20af91..ac146f10f222 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1472,7 +1472,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> LIST_HEAD(pages_skipped);
>
> for (scan = 0; scan < nr_to_scan && nr_taken < nr_to_scan &&
> - !list_empty(src); scan++) {
> + !list_empty(src);) {
> struct page *page;
>
> page = lru_to_page(src);
> @@ -1486,6 +1486,12 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> continue;
> }
>
> + /*
> + * Do not count skipped pages because we do want to isolate
> + * some pages even when the LRU mostly contains ineligible
> + * pages
> + */
How about adding comment about "why"?
/*
* Do not count skipped pages because it makes the function to return with
* none isolated pages if the LRU mostly contains inelgible pages so that
* VM cannot reclaim any pages and trigger premature OOM.
*/
> + scan++;
> switch (__isolate_lru_page(page, mode)) {
> case 0:
> nr_pages = hpage_nr_pages(page);
Confirmed. It works as expected but it changed scan counter's behavior.
How about this?
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2314aca47d12..846922d7942e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1469,7 +1469,7 @@ static __always_inline void update_lru_sizes(struct lruvec *lruvec,
*
* Appropriate locks must be held before calling this function.
*
- * @nr_to_scan: The number of pages to look through on the list.
+ * @nr_to_scan: The number of eligible pages to look through on the list.
* @lruvec: The LRU vector to pull pages from.
* @dst: The temp list to put pages on to.
* @nr_scanned: The number of pages that were scanned.
@@ -1489,11 +1489,13 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
unsigned long nr_zone_taken[MAX_NR_ZONES] = { 0 };
unsigned long nr_skipped[MAX_NR_ZONES] = { 0, };
unsigned long skipped = 0;
- unsigned long scan, nr_pages;
+ unsigned long scan, total_scan, nr_pages;
LIST_HEAD(pages_skipped);
- for (scan = 0; scan < nr_to_scan && nr_taken < nr_to_scan &&
- !list_empty(src); scan++) {
+ for (total_scan = scan = 0; scan < nr_to_scan &&
+ nr_taken < nr_to_scan &&
+ !list_empty(src);
+ total_scan++) {
struct page *page;
page = lru_to_page(src);
@@ -1507,6 +1509,13 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
continue;
}
+ /*
+ * Do not count skipped pages because it makes the function to
+ * return with none isolated pages if the LRU mostly contains
+ * inelgible pages so that VM cannot reclaim any pages and
+ * trigger premature OOM.
+ */
+ scan++;
switch (__isolate_lru_page(page, mode)) {
case 0:
nr_pages = hpage_nr_pages(page);
@@ -1544,9 +1553,9 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
skipped += nr_skipped[zid];
}
}
- *nr_scanned = scan;
+ *nr_scanned = total_scan;
trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
- scan, skipped, nr_taken, mode, lru);
+ total_scan, skipped, nr_taken, mode, lru);
update_lru_sizes(lruvec, lru, nr_zone_taken);
return nr_taken;
}
--
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>
^ permalink raw reply related
* Re: [PATCH -mm -v10 1/3] mm, THP, swap: Delay splitting THP during swap out
From: Minchan Kim @ 2017-05-10 1:03 UTC (permalink / raw)
To: Huang, Ying
Cc: Johannes Weiner, Andrew Morton, linux-mm, linux-kernel,
Andrea Arcangeli, Ebru Akagunduz, Michal Hocko, Tejun Heo,
Hugh Dickins, Shaohua Li, Rik van Riel, cgroups
In-Reply-To: <87d1bwvi26.fsf@yhuang-dev.intel.com>
Hi Huang,
On Fri, Apr 28, 2017 at 08:21:37PM +0800, Huang, Ying wrote:
> Minchan Kim <minchan@kernel.org> writes:
>
> > On Thu, Apr 27, 2017 at 03:12:34PM +0800, Huang, Ying wrote:
> >> Minchan Kim <minchan@kernel.org> writes:
> >>
> >> > On Tue, Apr 25, 2017 at 08:56:56PM +0800, Huang, Ying wrote:
> >> >> From: Huang Ying <ying.huang@intel.com>
> >> >>
> >> >> In this patch, splitting huge page is delayed from almost the first
> >> >> step of swapping out to after allocating the swap space for the
> >> >> THP (Transparent Huge Page) and adding the THP into the swap cache.
> >> >> This will batch the corresponding operation, thus improve THP swap out
> >> >> throughput.
> >> >>
> >> >> This is the first step for the THP swap optimization. The plan is to
> >> >> delay splitting the THP step by step and avoid splitting the THP
> >> >> finally.
> >> >>
> >> >> The advantages of the THP swap support include:
> >> >>
> >> >> - Batch the swap operations for the THP and reduce lock
> >> >> acquiring/releasing, including allocating/freeing the swap space,
> >> >> adding/deleting to/from the swap cache, and writing/reading the swap
> >> >> space, etc. This will help to improve the THP swap performance.
> >> >>
> >> >> - The THP swap space read/write will be 2M sequential IO. It is
> >> >> particularly helpful for the swap read, which usually are 4k random
> >> >> IO. This will help to improve the THP swap performance.
> >> >>
> >> >> - It will help the memory fragmentation, especially when the THP is
> >> >> heavily used by the applications. The 2M continuous pages will be
> >> >> free up after the THP swapping out.
> >> >>
> >> >> - It will improve the THP utilization on the system with the swap
> >> >> turned on. Because the speed for khugepaged to collapse the normal
> >> >> pages into the THP is quite slow. After the THP is split during the
> >> >> swapping out, it will take quite long time for the normal pages to
> >> >> collapse back into the THP after being swapped in. The high THP
> >> >> utilization helps the efficiency of the page based memory management
> >> >> too.
> >> >>
> >> >> There are some concerns regarding THP swap in, mainly because possible
> >> >> enlarged read/write IO size (for swap in/out) may put more overhead on
> >> >> the storage device. To deal with that, the THP swap in should be
> >> >> turned on only when necessary. For example, it can be selected via
> >> >> "always/never/madvise" logic, to be turned on globally, turned off
> >> >> globally, or turned on only for VMA with MADV_HUGEPAGE, etc.
> >> >>
> >> >> In this patch, one swap cluster is used to hold the contents of each
> >> >> THP swapped out. So, the size of the swap cluster is changed to that
> >> >> of the THP (Transparent Huge Page) on x86_64 architecture (512). For
> >> >> other architectures which want such THP swap optimization,
> >> >> ARCH_USES_THP_SWAP_CLUSTER needs to be selected in the Kconfig file
> >> >> for the architecture. In effect, this will enlarge swap cluster size
> >> >> by 2 times on x86_64. Which may make it harder to find a free cluster
> >> >> when the swap space becomes fragmented. So that, this may reduce the
> >> >> continuous swap space allocation and sequential write in theory. The
> >> >> performance test in 0day shows no regressions caused by this.
> >> >
> >> > What about other architecures?
> >> >
> >> > I mean THP page size on every architectures would be various.
> >> > If THP page size is much bigger than 2M, the architecture should
> >> > have big swap cluster size for supporting THP swap-out feature.
> >> > It means fast empty-swap cluster consumption so that it can suffer
> >> > from fragmentation easily which causes THP swap void and swap slot
> >> > allocations slow due to not being able to use per-cpu.
> >> >
> >> > What I suggested was contiguous multiple swap cluster allocations
> >> > to meet THP page size. If some of architecure's THP size is 64M
> >> > and SWAP_CLUSTER_SIZE is 2M, it should allocate 32 contiguos
> >> > swap clusters. For that, swap layer need to manage clusters sort
> >> > in order which would be more overhead in CONFIG_THP_SWAP case
> >> > but I think it's tradeoff. With that, every architectures can
> >> > support THP swap easily without arch-specific something.
> >>
> >> That may be a good solution for other architectures. But I am afraid I
> >> am not the right person to work on that. Because I don't know the
> >> requirement of other architectures, and I have no other architectures
> >> machines to work on and measure the performance.
> >
> > IMO, THP swapout is good thing for every architecture so I dobut
> > you need to know other architecture's requirement.
> >
> >>
> >> And the swap clusters aren't sorted in order now intentionally to avoid
> >> cache line false sharing between the spinlock of struct
> >> swap_cluster_info. If we want to sort clusters in order, we need a
> >> solution for that.
> >
> > Does it really matter for this work? IOW, if we couldn't solve it,
> > cannot we support THP swapout? I don't think so. That's the least
> > of your worries.
> > Also, if we have sorted cluster data structure, we need to change
> > current single linked list of swap cluster to other one so we would
> > need to revisit to see whether it's really problem.
> >
> >>
> >> > If (PAGE_SIZE * 512) swap cluster size were okay for most of
> >> > architecture, just increase it. It's orthogonal work regardless of
> >> > THP swapout. Then, we don't need to manage swap clusters sort
> >> > in order in x86_64 which SWAP_CLUSTER_SIZE is equal to
> >> > THP_PAGE_SIZE. It's just a bonus by side-effect.
> >>
> >> Andrew suggested to make swap cluster size = huge page size (or turn on
> >> THP swap optimization) only if we enabled CONFIG_THP_SWAP. So that, THP
> >> swap optimization will not be turned on unintentionally.
> >>
> >> We may adjust default swap cluster size, but I don't think it need to be
> >> in this patchset.
> >
> > That's it. This feature shouldn't be aware of swap cluster size. IOW,
> > it would be better to work with every swap cluster size if the align
> > between THP and swap cluster size is matched at least.
>
> Using one swap cluster for each THP is simpler, so why not start from
> the simple design? Complex design may be necessary in the future, but
> we can work on that at that time.
If it were really architecture specific issue, I'm okay with such simple
start with major architecture first. However, I don't think it's the case.
THP swap: I don't think it's a architecure issue. It's generally good thing
once system supports THP. It is also good thing for HDD swap as well as SSD.
(In fact, I don't understand why we should have CONFIG_THP_SWAP. I think
it should work with CONFIG_TRANSPARENT_HUGEPAGE automatically).
Current design is selected for just *simple implementation" and put the
heavy drift to make it generalize to others in the future.
I don't think it's good thing. But others might have different opinions
so I'm not insisting.
--
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>
^ permalink raw reply
* Re: [PATCH v3 2/3] ARM: Silence first allocation with CONFIG_ARM_MODULE_PLTS=y
From: Russell King - ARM Linux @ 2017-05-09 23:24 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Ard Biesheuvel,
Andrew Morton, Michal Hocko, zijun_hu, Kirill A. Shutemov,
Andrey Ryabinin, Chris Wilson, open list,
open list:MEMORY MANAGEMENT, angus
In-Reply-To: <fccefcb2-b711-0589-168a-714e55064279@gmail.com>
On Tue, May 09, 2017 at 04:16:09PM -0700, Florian Fainelli wrote:
> On 04/27/2017 11:19 AM, Florian Fainelli wrote:
> > When CONFIG_ARM_MODULE_PLTS is enabled, the first allocation using the
> > module space fails, because the module is too big, and then the module
> > allocation is attempted from vmalloc space. Silence the first allocation
> > failure in that case by setting __GFP_NOWARN.
>
> Russell, are you okay with this change? Do you have a preference as
> which tree should carry this patch series?
It looks sensible.
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
No preference which tree it goes through...
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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>
^ permalink raw reply
* Re: [PATCH v3 2/3] ARM: Silence first allocation with CONFIG_ARM_MODULE_PLTS=y
From: Florian Fainelli @ 2017-05-09 23:16 UTC (permalink / raw)
To: Russell King
Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Ard Biesheuvel,
Andrew Morton, Michal Hocko, zijun_hu, Kirill A. Shutemov,
Andrey Ryabinin, Chris Wilson, open list,
open list:MEMORY MANAGEMENT, angus
In-Reply-To: <20170427181902.28829-3-f.fainelli@gmail.com>
On 04/27/2017 11:19 AM, Florian Fainelli wrote:
> When CONFIG_ARM_MODULE_PLTS is enabled, the first allocation using the
> module space fails, because the module is too big, and then the module
> allocation is attempted from vmalloc space. Silence the first allocation
> failure in that case by setting __GFP_NOWARN.
Russell, are you okay with this change? Do you have a preference as
which tree should carry this patch series?
Thanks
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> arch/arm/kernel/module.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index 80254b47dc34..3ff571c2c71c 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -40,8 +40,15 @@
> #ifdef CONFIG_MMU
> void *module_alloc(unsigned long size)
> {
> - void *p = __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
> - GFP_KERNEL, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
> + gfp_t gfp_mask = GFP_KERNEL;
> + void *p;
> +
> + /* Silence the initial allocation */
> + if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS))
> + gfp_mask |= __GFP_NOWARN;
> +
> + p = __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
> + gfp_mask, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
> __builtin_return_address(0));
> if (!IS_ENABLED(CONFIG_ARM_MODULE_PLTS) || p)
> return p;
>
--
Florian
--
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>
^ permalink raw reply
* Re: [PATCH v2 1/2] mm: Uncharge poisoned pages
From: Naoya Horiguchi @ 2017-05-09 22:59 UTC (permalink / raw)
To: Michal Hocko
Cc: akpm@linux-foundation.org, Andi Kleen, Johannes Weiner,
Laurent Dufour, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Vladimir Davydov
In-Reply-To: <20170509091823.GF6481@dhcp22.suse.cz>
On Tue, May 09, 2017 at 11:18:23AM +0200, Michal Hocko wrote:
> On Mon 08-05-17 02:58:36, Naoya Horiguchi wrote:
> > On Tue, May 02, 2017 at 08:55:07PM +0200, Michal Hocko wrote:
> > > On Tue 02-05-17 16:59:30, Laurent Dufour wrote:
> > > > On 28/04/2017 15:48, Michal Hocko wrote:
> > > [...]
> > > > > This is getting quite hairy. What is the expected page count of the
> > > > > hwpoison page?
> > >
> > > OK, so from the quick check of the hwpoison code it seems that the ref
> > > count will be > 1 (from get_hwpoison_page).
> > >
> > > > > I guess we would need to update the VM_BUG_ON in the
> > > > > memcg uncharge code to ignore the page count of hwpoison pages if it can
> > > > > be arbitrary.
> > > >
> > > > Based on the experiment I did, page count == 2 when isolate_lru_page()
> > > > succeeds, even in the case of a poisoned page.
> > >
> > > that would make some sense to me. The page should have been already
> > > unmapped therefore but memory_failure increases the ref count and 1 is
> > > for isolate_lru_page().
> >
> > # sorry for late reply, I was on holidays last week...
> >
> > Right, and the refcount taken for memory_failure is not freed after
> > memory_failure() returns. unpoison_memory() does free the refcount.
>
> OK, from the charge POV this would be safe because we clear page->memcg
> so it wouldn't get uncharged more times.
>
> > > > In my case I think this
> > > > is because the page is still used by the process which is calling madvise().
> > > >
> > > > I'm wondering if I'm looking at the right place. May be the poisoned
> > > > page should remain attach to the memory_cgroup until no one is using it.
> > > > In that case this means that something should be done when the page is
> > > > off-lined... I've to dig further here.
> > >
> > > No, AFAIU the page will not drop the reference count down to 0 in most
> > > cases. Maybe there are some scenarios where this can happen but I would
> > > expect that the poisoned page will be mapped and in use most of the time
> > > and won't drop down 0. And then we should really uncharge it because it
> > > will pin the memcg and make it unfreeable which doesn't seem to be what
> > > we want. So does the following work reasonable? Andi, Johannes, what do
> > > you think? I cannot say I would be really comfortable touching hwpoison
> > > code as I really do not understand the workflow. Maybe we want to move
> > > this uncharge down to memory_failure() right before we report success?
> >
> > memory_failure() can be called for any types of page (including slab or
> > any kernel/driver pages), and the reported problem seems happen only on
> > in-use user pages, so uncharging in delete_from_lru_cache() as done below
> > looks better to me.
>
> Yeah, we do see problems only for LRU/page cache pages but my
> understanding is that error_states (e.g. me_kernel for the kernel
> memory) might change in the future and then we wouldn't catch the same
> bug, no?
Right about future change, and we will see the same bug. I guess that the
first target of kernel page is slab page, and memcg_kmem_uncharge() will
be used there. Implementors/Reviewers should care about uncharging when the
time comes.
Thanks,
Naoya Horiguchi
>
> > > ---
> > > From 8bf0791bcf35996a859b6d33fb5494e5b53de49d Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko <mhocko@suse.com>
> > > Date: Tue, 2 May 2017 20:32:24 +0200
> > > Subject: [PATCH] hwpoison, memcg: forcibly uncharge LRU pages
> > >
> > > Laurent Dufour has noticed that hwpoinsoned pages are kept charged. In
> > > his particular case he has hit a bad_page("page still charged to cgroup")
> > > when onlining a hwpoison page.
> >
> > > While this looks like something that shouldn't
> > > happen in the first place because onlining hwpages and returning them to
> > > the page allocator makes only little sense it shows a real problem.
> > >
> > > hwpoison pages do not get freed usually so we do not uncharge them (at
> > > least not since 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API")).
> > > Each charge pins memcg (since e8ea14cc6ead ("mm: memcontrol: take a css
> > > reference for each charged page")) as well and so the mem_cgroup and the
> > > associated state will never go away. Fix this leak by forcibly
> > > uncharging a LRU hwpoisoned page in delete_from_lru_cache(). We also
> > > have to tweak uncharge_list because it cannot rely on zero ref count
> > > for these pages.
> > >
> > > Fixes: 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API")
> > > Reported-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> > > Signed-off-by: Michal Hocko <mhocko@suse.com>
> >
> > Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>
> Thanks! I will wait a day or two for Johannes and repost the patch.
> Andrew could you drop
> http://www.ozlabs.org/~akpm/mmotm/broken-out/mm-uncharge-poisoned-pages.patch
> in the mean time, please?
>
> --
> Michal Hocko
> SUSE Labs
>
--
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>
^ permalink raw reply
* Re: [RFC 03/10] x86/mm: Make the batched unmap TLB flush API more generic
From: Andy Lutomirski @ 2017-05-09 22:54 UTC (permalink / raw)
To: Dave Hansen
Cc: Andy Lutomirski, X86 ML, linux-kernel@vger.kernel.org,
Borislav Petkov, Linus Torvalds, Andrew Morton, Mel Gorman,
linux-mm@kvack.org, Rik van Riel, Nadav Amit, Michal Hocko,
Sasha Levin
In-Reply-To: <cf830e99-10a0-1013-4ea2-e184b2017854@intel.com>
On Tue, May 9, 2017 at 10:13 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> On 05/09/2017 06:02 AM, Andy Lutomirski wrote:
>> On Mon, May 8, 2017 at 8:34 AM, Dave Hansen <dave.hansen@intel.com> wrote:
>>> On 05/07/2017 05:38 AM, Andy Lutomirski wrote:
>>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>>> index f6838015810f..2e568c82f477 100644
>>>> --- a/mm/rmap.c
>>>> +++ b/mm/rmap.c
>>>> @@ -579,25 +579,12 @@ void page_unlock_anon_vma_read(struct anon_vma *anon_vma)
>>>> void try_to_unmap_flush(void)
>>>> {
>>>> struct tlbflush_unmap_batch *tlb_ubc = ¤t->tlb_ubc;
>>>> - int cpu;
>>>>
>>>> if (!tlb_ubc->flush_required)
>>>> return;
>>>>
>>>> - cpu = get_cpu();
>>>> -
>>>> - if (cpumask_test_cpu(cpu, &tlb_ubc->cpumask)) {
>>>> - count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
>>>> - local_flush_tlb();
>>>> - trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
>>>> - }
>>>> -
>>>> - if (cpumask_any_but(&tlb_ubc->cpumask, cpu) < nr_cpu_ids)
>>>> - flush_tlb_others(&tlb_ubc->cpumask, NULL, 0, TLB_FLUSH_ALL);
>>>> - cpumask_clear(&tlb_ubc->cpumask);
>>>> tlb_ubc->flush_required = false;
>>>> tlb_ubc->writable = false;
>>>> - put_cpu();
>>>> }
>>>>
>>>> /* Flush iff there are potentially writable TLB entries that can race with IO */
>>>> @@ -613,7 +600,7 @@ static void set_tlb_ubc_flush_pending(struct mm_struct *mm, bool writable)
>>>> {
>>>> struct tlbflush_unmap_batch *tlb_ubc = ¤t->tlb_ubc;
>>>>
>>>> - cpumask_or(&tlb_ubc->cpumask, &tlb_ubc->cpumask, mm_cpumask(mm));
>>>> + arch_tlbbatch_add_mm(&tlb_ubc->arch, mm);
>>>> tlb_ubc->flush_required = true;
>>>>
>>>> /*
>>>
>>> Looking at this patch in isolation, how can this be safe? It removes
>>> TLB flushes from the generic code. Do other patches in the series fix
>>> this up?
>>
>> Hmm? Unless I totally screwed this up, this patch just moves the
>> flushes around -- it shouldn't remove any flushes.
>
> This takes a flush out of try_to_unmap_flush(). It adds code for
> arch_tlbbatch_flush(), but not *calls* to arch_tlbbatch_flush() that I
> can see.
>
> I actually don't see _any_ in the whole series in a quick grepping. Am
> I just missing them?
Oops! I must have stared at that function for so long that I started
seeing the invisible call. I'll fix that.
--
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>
^ permalink raw reply
* Re: [RFC 09/10] x86/mm: Rework lazy TLB to track the actual loaded mm
From: Andy Lutomirski @ 2017-05-09 22:54 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Andy Lutomirski, X86 ML, linux-kernel@vger.kernel.org,
Borislav Petkov, Linus Torvalds, Andrew Morton, Mel Gorman,
linux-mm@kvack.org, Rik van Riel, Dave Hansen, Nadav Amit,
Michal Hocko, Arjan van de Ven
In-Reply-To: <alpine.DEB.2.20.1705092236290.2295@nanos>
On Tue, May 9, 2017 at 1:41 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Sun, 7 May 2017, Andy Lutomirski wrote:
>> /* context.lock is held for us, so we don't need any locking. */
>> static void flush_ldt(void *current_mm)
>> {
>> + struct mm_struct *mm = current_mm;
>> mm_context_t *pc;
>>
>> - if (current->active_mm != current_mm)
>> + if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
>
> While functional correct, this really should compare against 'mm'.
>
Fixed.
--
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>
^ permalink raw reply
* Re: [PATCH v4 13/27] lib: add errseq_t type and infrastructure for handling it
From: NeilBrown @ 2017-05-09 22:03 UTC (permalink / raw)
To: Jeff Layton, linux-fsdevel, linux-kernel, linux-btrfs, linux-ext4,
linux-cifs, linux-nfs, linux-mm, jfs-discussion, linux-xfs,
cluster-devel, linux-f2fs-devel, v9fs-developer, linux-nilfs,
linux-block
Cc: dhowells, akpm, hch, ross.zwisler, mawilcox, jack, viro, corbet,
neilb, clm, tytso, axboe, josef, hubcap, rpeterso, bo.li.liu
In-Reply-To: <20170509154930.29524-14-jlayton@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 10479 bytes --]
On Tue, May 09 2017, Jeff Layton wrote:
> An errseq_t is a way of recording errors in one place, and allowing any
> number of "subscribers" to tell whether an error has been set again
> since a previous time.
>
> It's implemented as an unsigned 32-bit value that is managed with atomic
> operations. The low order bits are designated to hold an error code
> (max size of MAX_ERRNO). The upper bits are used as a counter.
>
> The API works with consumers sampling an errseq_t value at a particular
> point in time. Later, that value can be used to tell whether new errors
> have been set since that time.
>
> Note that there is a 1 in 512k risk of collisions here if new errors
> are being recorded frequently, since we have so few bits to use as a
> counter. To mitigate this, one bit is used as a flag to tell whether the
> value has been sampled since a new value was recorded. That allows
> us to avoid bumping the counter if no one has sampled it since it
> was last bumped.
>
> Later patches will build on this infrastructure to change how writeback
> errors are tracked in the kernel.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
I like that this is a separate lib/*.c - nicely structured too.
Reviewed-by: NeilBrown <neilb@suse.com>
Thanks,
NeilBrown
> ---
> include/linux/errseq.h | 19 +++++
> lib/Makefile | 2 +-
> lib/errseq.c | 199 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 219 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/errseq.h
> create mode 100644 lib/errseq.c
>
> diff --git a/include/linux/errseq.h b/include/linux/errseq.h
> new file mode 100644
> index 000000000000..0d2555f310cd
> --- /dev/null
> +++ b/include/linux/errseq.h
> @@ -0,0 +1,19 @@
> +#ifndef _LINUX_ERRSEQ_H
> +#define _LINUX_ERRSEQ_H
> +
> +/* See lib/errseq.c for more info */
> +
> +typedef u32 errseq_t;
> +
> +void __errseq_set(errseq_t *eseq, int err);
> +static inline void errseq_set(errseq_t *eseq, int err)
> +{
> + /* Optimize for the common case of no error */
> + if (unlikely(err))
> + __errseq_set(eseq, err);
> +}
> +
> +errseq_t errseq_sample(errseq_t *eseq);
> +int errseq_check(errseq_t *eseq, errseq_t since);
> +int errseq_check_and_advance(errseq_t *eseq, errseq_t *since);
> +#endif
> diff --git a/lib/Makefile b/lib/Makefile
> index 320ac46a8725..2423afef40f7 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -41,7 +41,7 @@ obj-y += bcd.o div64.o sort.o parser.o debug_locks.o random32.o \
> gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \
> bsearch.o find_bit.o llist.o memweight.o kfifo.o \
> percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o \
> - once.o refcount.o
> + once.o refcount.o errseq.o
> obj-y += string_helpers.o
> obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
> obj-y += hexdump.o
> diff --git a/lib/errseq.c b/lib/errseq.c
> new file mode 100644
> index 000000000000..0f8b4ed460f0
> --- /dev/null
> +++ b/lib/errseq.c
> @@ -0,0 +1,199 @@
> +#include <linux/err.h>
> +#include <linux/bug.h>
> +#include <linux/atomic.h>
> +#include <linux/errseq.h>
> +
> +/*
> + * An errseq_t is a way of recording errors in one place, and allowing any
> + * number of "subscribers" to tell whether it has changed since an arbitrary
> + * time of their choosing.
> + *
> + * It's implemented as an unsigned 32-bit value. The low order bits are
> + * designated to hold an error code (between 0 and -MAX_ERRNO). The upper bits
> + * are used as a counter. This is done with atomics instead of locking so that
> + * these functions can be called from any context.
> + *
> + * The general idea is for consumers to sample an errseq_t value at a
> + * particular point in time. Later, that value can be used to tell whether any
> + * new errors have occurred since that time.
> + *
> + * Note that there is a risk of collisions, if new errors are being recorded
> + * frequently, since we have so few bits to use as a counter.
> + *
> + * To mitigate this, one bit is used as a flag to tell whether the value has
> + * been sampled since a new value was recorded. That allows us to avoid bumping
> + * the counter if no one has sampled it since the last time an error was
> + * recorded.
> + *
> + * A new errseq_t should always be zeroed out. A errseq_t value of all zeroes
> + * is the special (but common) case where there has never been an error. An all
> + * zero value thus serves as the "epoch" if one wishes to know whether there
> + * has ever been an error set since it was first initialized.
> + */
> +
> +/* The low bits are designated for error code (max of MAX_ERRNO) */
> +#define ERRSEQ_SHIFT ilog2(MAX_ERRNO + 1)
> +
> +/* This bit is used as a flag to indicate whether the value has been seen */
> +#define ERRSEQ_SEEN (1 << ERRSEQ_SHIFT)
> +
> +/* The "ones" bit for the counter */
> +#define ERRSEQ_CTR_INC (1 << (ERRSEQ_SHIFT + 1))
> +
> +/**
> + * __errseq_set - set a errseq_t for later reporting
> + * @eseq: errseq_t field that should be set
> + * @err: error to set
> + *
> + * This function sets the error in *eseq, and increments the sequence counter
> + * if the last sequence was sampled at some point in the past.
> + *
> + * Any error set will always overwrite an existing error.
> + *
> + * Most callers will want to use the errseq_set inline wrapper to efficiently
> + * handle the common case where err is 0.
> + */
> +void __errseq_set(errseq_t *eseq, int err)
> +{
> + errseq_t old;
> +
> + /* MAX_ERRNO must be able to serve as a mask */
> + BUILD_BUG_ON_NOT_POWER_OF_2(MAX_ERRNO + 1);
> +
> + /*
> + * Ensure the error code actually fits where we want it to go. If it
> + * doesn't then just throw a warning and don't record anything. We
> + * also don't accept zero here as that would effectively clear a
> + * previous error.
> + */
> + if (WARN(unlikely(err == 0 || (unsigned int)-err > MAX_ERRNO),
> + "err = %d\n", err))
> + return;
> +
> + old = READ_ONCE(*eseq);
> + for (;;) {
> + errseq_t new, cur;
> +
> + /* Clear out error bits and set new error */
> + new = (old & ~(MAX_ERRNO|ERRSEQ_SEEN)) | -err;
> +
> + /* Only increment if someone has looked at it */
> + if (old & ERRSEQ_SEEN)
> + new += ERRSEQ_CTR_INC;
> +
> + /* If there would be no change, then call it done */
> + if (new == old)
> + break;
> +
> + /* Try to swap the new value into place */
> + cur = cmpxchg(eseq, old, new);
> +
> + /*
> + * Call it success if we did the swap or someone else beat us
> + * to it for the same value.
> + */
> + if (likely(cur == old || cur == new))
> + break;
> +
> + /* Raced with an update, try again */
> + old = cur;
> + }
> +}
> +EXPORT_SYMBOL(__errseq_set);
> +
> +/**
> + * errseq_sample - grab current errseq_t value
> + * @eseq: pointer to errseq_t to be sampled
> + *
> + * This function allows callers to sample an errseq_t value, marking it as
> + * "seen" if required.
> + */
> +errseq_t errseq_sample(errseq_t *eseq)
> +{
> + errseq_t old = READ_ONCE(*eseq);
> + errseq_t new = old;
> +
> + /*
> + * For the common case of no errors ever having been set, we can skip
> + * marking the SEEN bit. Once an error has been set, the value will
> + * never go back to zero.
> + */
> + if (old != 0) {
> + new |= ERRSEQ_SEEN;
> + if (old != new)
> + cmpxchg(eseq, old, new);
> + }
> + return new;
> +}
> +EXPORT_SYMBOL(errseq_sample);
> +
> +/**
> + * errseq_check - has an error occurred since a particular point in time?
> + * @eseq: pointer to errseq_t value to be checked
> + * @since: previously-sampled errseq_t from which to check
> + *
> + * Grab the value that eseq points to, and see if it has changed "since"
> + * the given value was sampled. The "since" value is not advanced, so there
> + * is no need to mark the value as seen.
> + *
> + * Returns the latest error set in the errseq_t or 0 if it hasn't changed.
> + */
> +int errseq_check(errseq_t *eseq, errseq_t since)
> +{
> + errseq_t cur = READ_ONCE(*eseq);
> +
> + if (likely(cur == since))
> + return 0;
> + return -(cur & MAX_ERRNO);
> +}
> +EXPORT_SYMBOL(errseq_check);
> +
> +/**
> + * errseq_check_and_advance - check an errseq_t and advance it to the current value
> + * @eseq: pointer to value being checked reported
> + * @since: pointer to previously-sampled errseq_t to check against and advance
> + *
> + * Grab the eseq value, and see whether it matches the value that "since"
> + * points to. If it does, then just return 0.
> + *
> + * If it doesn't, then the value has changed. Set the "seen" flag, and try to
> + * swap it into place as the new eseq value. Then, set that value as the new
> + * "since" value, and return whatever the error portion is set to.
> + *
> + * Note that no locking is provided here for concurrent updates to the "since"
> + * value. The caller must provide that if necessary. Because of this, callers
> + * may want to do a lockless errseq_check before taking the lock and calling
> + * this.
> + */
> +int errseq_check_and_advance(errseq_t *eseq, errseq_t *since)
> +{
> + int err = 0;
> + errseq_t old, new;
> +
> + /*
> + * Most callers will want to use the inline wrapper to check this,
> + * so that the common case of no error is handled without needing
> + * to lock.
> + */
> + old = READ_ONCE(*eseq);
> + if (old != *since) {
> + /*
> + * Set the flag and try to swap it into place if it has
> + * changed.
> + *
> + * We don't care about the outcome of the swap here. If the
> + * swap doesn't occur, then it has either been updated by a
> + * writer who is bumping the seq count anyway, or another
> + * reader who is just setting the "seen" flag. Either outcome
> + * is OK, and we can advance "since" and return an error based
> + * on what we have.
> + */
> + new = old | ERRSEQ_SEEN;
> + if (new != old)
> + cmpxchg(eseq, old, new);
> + *since = new;
> + err = -(new & MAX_ERRNO);
> + }
> + return err;
> +}
> +EXPORT_SYMBOL(errseq_check_and_advance);
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH RFC] hugetlbfs 'noautofill' mount option
From: Prakash Sangappa @ 2017-05-09 20:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Dave Hansen, linux-kernel, linux-mm
In-Reply-To: <20170509085825.GB32555@infradead.org>
On 5/9/17 1:58 AM, Christoph Hellwig wrote:
> On Mon, May 08, 2017 at 03:12:42PM -0700, prakash.sangappa wrote:
>> Regarding #3 as a general feature, do we want to
>> consider this and the complexity associated with the
>> implementation?
> We have to. Given that no one has exclusive access to hugetlbfs
> a mount option is fundamentally the wrong interface.
A hugetlbfs filesystem may need to be mounted for exclusive use by
an application. Note, recently the 'min_size' mount option was added
to hugetlbfs, which would reserve minimum number of huge pages
for that filesystem for use by an application. If the filesystem with
min size specified, is not setup for exclusive use by an application,
then the purpose of reserving huge pages is defeated. The
min_size option was for use by applications like the database.
Also, I am investigating enabling hugetlbfs mounts within user
namespace's mount namespace. That would allow an application
to mount a hugetlbfs filesystem inside a namespace exclusively for
its use, running as a non root user. For this it seems like the 'min_size'
should be subject to some user limits. Anyways, mounting inside
user namespaces is a different discussion.
So, if a filesystem has to be setup for exclusive use by an application,
then different mount options can be used for that filesystem.
--
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>
^ permalink raw reply
* Re: [RFC 09/10] x86/mm: Rework lazy TLB to track the actual loaded mm
From: Thomas Gleixner @ 2017-05-09 20:41 UTC (permalink / raw)
To: Andy Lutomirski
Cc: X86 ML, linux-kernel@vger.kernel.org, Borislav Petkov,
Linus Torvalds, Andrew Morton, Mel Gorman, linux-mm@kvack.org,
Rik van Riel, Dave Hansen, Nadav Amit, Michal Hocko,
Arjan van de Ven
In-Reply-To: <1a124281c99741606f1789140f9805beebb119da.1494160201.git.luto@kernel.org>
On Sun, 7 May 2017, Andy Lutomirski wrote:
> /* context.lock is held for us, so we don't need any locking. */
> static void flush_ldt(void *current_mm)
> {
> + struct mm_struct *mm = current_mm;
> mm_context_t *pc;
>
> - if (current->active_mm != current_mm)
> + if (this_cpu_read(cpu_tlbstate.loaded_mm) != current_mm)
While functional correct, this really should compare against 'mm'.
> return;
>
> - pc = ¤t->active_mm->context;
> + pc = &mm->context;
Thanks,
tglx
--
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>
^ permalink raw reply
* Re: [PATCH] mm, vmalloc: fix vmalloc users tracking properly
From: Michal Hocko @ 2017-05-09 19:36 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Torvalds, Andrew Morton, Tobias Klauser, Linux MM, LKML
In-Reply-To: <CAMuHMdV2=PUs64K8tnGw1oPDRjKbx0SRkN-59ToTpj57=CXYdA@mail.gmail.com>
On Tue 09-05-17 21:01:25, Geert Uytterhoeven wrote:
> Hi Michal,
>
> On Tue, May 9, 2017 at 4:41 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > From: Michal Hocko <mhocko@suse.com>
> >
> > 1f5307b1e094 ("mm, vmalloc: properly track vmalloc users") has pulled
> > asm/pgtable.h include dependency to linux/vmalloc.h and that turned out
> > to be a bad idea for some architectures. E.g. m68k fails with
> > In file included from arch/m68k/include/asm/pgtable_mm.h:145:0,
> > from arch/m68k/include/asm/pgtable.h:4,
> > from include/linux/vmalloc.h:9,
> > from arch/m68k/kernel/module.c:9:
> > arch/m68k/include/asm/mcf_pgtable.h: In function 'nocache_page':
> >>> arch/m68k/include/asm/mcf_pgtable.h:339:43: error: 'init_mm' undeclared (first use in this function)
> > #define pgd_offset_k(address) pgd_offset(&init_mm, address)
> >
> > as spotted by kernel build bot. nios2 fails for other reason
> > In file included from ./include/asm-generic/io.h:767:0,
> > from ./arch/nios2/include/asm/io.h:61,
> > from ./include/linux/io.h:25,
> > from ./arch/nios2/include/asm/pgtable.h:18,
> > from ./include/linux/mm.h:70,
> > from ./include/linux/pid_namespace.h:6,
> > from ./include/linux/ptrace.h:9,
> > from ./arch/nios2/include/uapi/asm/elf.h:23,
> > from ./arch/nios2/include/asm/elf.h:22,
> > from ./include/linux/elf.h:4,
> > from ./include/linux/module.h:15,
> > from init/main.c:16:
> > ./include/linux/vmalloc.h: In function '__vmalloc_node_flags':
> > ./include/linux/vmalloc.h:99:40: error: 'PAGE_KERNEL' undeclared (first use in this function); did you mean 'GFP_KERNEL'?
> >
> > which is due to the newly added #include <asm/pgtable.h>, which on nios2
> > includes <linux/io.h> and thus <asm/io.h> and <asm-generic/io.h> which
> > again includes <linux/vmalloc.h>.
> >
> > Tweaking that around just turns out a bigger headache than
> > necessary. This patch reverts 1f5307b1e094 and reimplements the original
> > fix in a different way. __vmalloc_node_flags can stay static inline
> > which will cover vmalloc* functions. We only have one external user
> > (kvmalloc_node) and we can export __vmalloc_node_flags_caller and
> > provide the caller directly. This is much simpler and it doesn't really
> > need any games with header files.
> >
> > Fixes: 1f5307b1e094 ("mm, vmalloc: properly track vmalloc users")
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
>
> FWIW, this did fix the following build failure on m68k in linus/master
> (commit 2868b2513aa732a9 ("Merge tag 'linux-kselftest-4.12-rc1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest"):
>
> In file included from arch/m68k/include/asm/pgtable_mm.h:148,
> from arch/m68k/include/asm/pgtable.h:5,
> from include/linux/vmalloc.h:10,
> from arch/m68k/kernel/module.c:10:
> arch/m68k/include/asm/motorola_pgtable.h: In function a??pgd_offseta??:
> arch/m68k/include/asm/motorola_pgtable.h:198: error: dereferencing
> pointer to incomplete type
> scripts/Makefile.build:294: recipe for target
> 'arch/m68k/kernel/module.o' failed
>
> but given the complaints from 0day on this and future versions, I think it's
> better not to provide a Tested-by yet.
FWIW I have already sent a follow up fix
http://lkml.kernel.org/r/20170509153702.GR6481@dhcp22.suse.cz
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox