[PATCH] mm: fix the memory leak after collapsing the huge page fails

[PATCH] mm: fix the memory leak after collapsing the huge page fails

[zhongjiang]

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>
---
 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);
 out_nolock:
 	trace_mm_collapse_huge_page(mm, isolated, result);
 	return;
-- 
1.8.3.1

--
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>

Re: [PATCH] mm: fix the memory leak after collapsing the huge page fails

[Vlastimil Babka]

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;
> 

--
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>

Re: [PATCH] mm: fix the memory leak after collapsing the huge page fails

[zhong jiang]

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>

Re: [PATCH] mm: fix the memory leak after collapsing the huge page fails

[zhong jiang]

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>