* Re: [PATCH] mm: fix the memory leak after collapsing the huge page fails
2017-05-09 8:42 ` Vlastimil Babka
@ 2017-05-09 9:25 ` zhong jiang
2017-05-09 9:57 ` zhong jiang
1 sibling, 0 replies; 4+ messages in thread
From: zhong jiang @ 2017-05-09 9:25 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: akpm, mgorman, hannes, kirill.shutemov, linux-mm
On 2017/5/9 16:42, Vlastimil Babka wrote:
> On 05/09/2017 10:12 AM, 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.
> Yeah, looks like the leak is there...
>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> mm/khugepaged.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 7cb9c88..3f5749e 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1080,6 +1080,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>> result = SCAN_SUCCEED;
>> out_up_write:
>> up_write(&mm->mmap_sem);
>> + put_page(new_page);
> This doesn't seem correct.
> - the put_page() will be called also on success, so a premature free?
my God, I forget this.
> - the out_nolock: case should be also handled
khugepage_alloc_page fails . it also will reach out_nolock.
The out_nolock is very mess, need to reconsider.
> - collapse_shmem() seems to have the same problem
yes. I will add the path to fix.
>> out_nolock:
>> trace_mm_collapse_huge_page(mm, isolated, result);
>> return;
>>
>
> .
>
--
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 [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: fix the memory leak after collapsing the huge page fails
2017-05-09 8:42 ` Vlastimil Babka
2017-05-09 9:25 ` zhong jiang
@ 2017-05-09 9:57 ` zhong jiang
1 sibling, 0 replies; 4+ messages in thread
From: zhong jiang @ 2017-05-09 9:57 UTC (permalink / raw)
To: Vlastimil Babka; +Cc: akpm, mgorman, hannes, kirill.shutemov, linux-mm
On 2017/5/9 16:42, Vlastimil Babka wrote:
> On 05/09/2017 10:12 AM, 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.
> Yeah, looks like the leak is there...
>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> mm/khugepaged.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 7cb9c88..3f5749e 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1080,6 +1080,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>> result = SCAN_SUCCEED;
>> out_up_write:
>> up_write(&mm->mmap_sem);
>> + put_page(new_page);
> This doesn't seem correct.
> - the put_page() will be called also on success, so a premature free?
> - the out_nolock: case should be also handled
> - collapse_shmem() seems to have the same problem
>
>> out_nolock:
>> trace_mm_collapse_huge_page(mm, isolated, result);
>> return;
>>
>
> .
>
>Subject: [PATCH v2] mm: fix the memory leak after collapsing the huge page
fails
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>
---
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 [flat|nested] 4+ messages in thread