* [PATCH v2] mm: fix the memory leak after collapsing the huge page fails @ 2017-05-09 10:55 zhongjiang 2017-05-09 11:34 ` Vlastimil Babka 0 siblings, 1 reply; 10+ messages in thread From: zhongjiang @ 2017-05-09 10:55 UTC (permalink / raw) To: akpm, vbabka; +Cc: kirill.shutemov, hannes, mgorman, linux-mm 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 | 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 */ } -- 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> ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 10:55 [PATCH v2] mm: fix the memory leak after collapsing the huge page fails zhongjiang @ 2017-05-09 11:34 ` Vlastimil Babka 2017-05-09 12:20 ` zhong jiang 0 siblings, 1 reply; 10+ messages in thread From: Vlastimil Babka @ 2017-05-09 11:34 UTC (permalink / raw) To: zhongjiang, akpm; +Cc: kirill.shutemov, hannes, mgorman, linux-mm 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(). > --- > 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] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 11:34 ` Vlastimil Babka @ 2017-05-09 12:20 ` zhong jiang 2017-05-09 12:41 ` Vlastimil Babka 0 siblings, 1 reply; 10+ messages in thread From: zhong jiang @ 2017-05-09 12:20 UTC (permalink / raw) To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm 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. I do not see the count increment when scan success. it save the memory, only when page fault happen. 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 [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 12:20 ` zhong jiang @ 2017-05-09 12:41 ` Vlastimil Babka 2017-05-09 13:31 ` zhong jiang 2017-05-09 13:54 ` zhong jiang 0 siblings, 2 replies; 10+ messages in thread From: Vlastimil Babka @ 2017-05-09 12:41 UTC (permalink / raw) To: zhong jiang; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm 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 [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 12:41 ` Vlastimil Babka @ 2017-05-09 13:31 ` zhong jiang 2017-05-09 13:46 ` Vlastimil Babka 2017-05-09 13:54 ` zhong jiang 1 sibling, 1 reply; 10+ messages in thread From: zhong jiang @ 2017-05-09 13:31 UTC (permalink / raw) To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm 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? I mean that whether collapse huge page success or nor, preallocate page will alway be release. by the above description. I seems to true. Thanks zhongjiang >> 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 [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 13:31 ` zhong jiang @ 2017-05-09 13:46 ` Vlastimil Babka 2017-05-09 13:56 ` zhong jiang 0 siblings, 1 reply; 10+ messages in thread From: Vlastimil Babka @ 2017-05-09 13:46 UTC (permalink / raw) To: zhong jiang; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm On 05/09/2017 03:31 PM, zhong jiang wrote: > 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? > I mean that whether collapse huge page success or nor, preallocate page will > alway be release. by the above description. I seems to true. Ah, no. In case of success, collapse_huge_page() does *hpage = NULL; so the put_page() won't get to it anymore. > > Thanks > zhongjiang >>> 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 [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 13:46 ` Vlastimil Babka @ 2017-05-09 13:56 ` zhong jiang 0 siblings, 0 replies; 10+ messages in thread From: zhong jiang @ 2017-05-09 13:56 UTC (permalink / raw) To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm On 2017/5/9 21:46, Vlastimil Babka wrote: > On 05/09/2017 03:31 PM, zhong jiang wrote: >> 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? >> I mean that whether collapse huge page success or nor, preallocate page will >> alway be release. by the above description. I seems to true. > Ah, no. In case of success, collapse_huge_page() does *hpage = NULL; so > the put_page() won't get to it anymore. Thanks a lots, it's complicated logical. Thanks zhongjiang >> Thanks >> zhongjiang >>>> 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 [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 12:41 ` Vlastimil Babka 2017-05-09 13:31 ` zhong jiang @ 2017-05-09 13:54 ` zhong jiang 2017-05-10 6:51 ` Vlastimil Babka 1 sibling, 1 reply; 10+ messages in thread From: zhong jiang @ 2017-05-09 13:54 UTC (permalink / raw) To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm 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. 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 [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-09 13:54 ` zhong jiang @ 2017-05-10 6:51 ` Vlastimil Babka 2017-05-10 7:59 ` zhong jiang 0 siblings, 1 reply; 10+ messages in thread From: Vlastimil Babka @ 2017-05-10 6:51 UTC (permalink / raw) To: zhong jiang; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm 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 [flat|nested] 10+ messages in thread
* Re: [PATCH v2] mm: fix the memory leak after collapsing the huge page fails 2017-05-10 6:51 ` Vlastimil Babka @ 2017-05-10 7:59 ` zhong jiang 0 siblings, 0 replies; 10+ messages in thread From: zhong jiang @ 2017-05-10 7:59 UTC (permalink / raw) To: Vlastimil Babka; +Cc: akpm, kirill.shutemov, hannes, mgorman, linux-mm On 2017/5/10 14:51, Vlastimil Babka wrote: > 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(). Thank you for clarification. I should consider more before sending the patch. Thanks zhongjiang >> 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 [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-05-10 8:10 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-09 10:55 [PATCH v2] mm: fix the memory leak after collapsing the huge page fails zhongjiang 2017-05-09 11:34 ` Vlastimil Babka 2017-05-09 12:20 ` zhong jiang 2017-05-09 12:41 ` Vlastimil Babka 2017-05-09 13:31 ` zhong jiang 2017-05-09 13:46 ` Vlastimil Babka 2017-05-09 13:56 ` zhong jiang 2017-05-09 13:54 ` zhong jiang 2017-05-10 6:51 ` Vlastimil Babka 2017-05-10 7:59 ` zhong jiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).