| Message ID | 20200625064547.228448-3-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc/mm/radix: Memory unplug fixes | expand |
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > From: Bharata B Rao <bharata@linux.ibm.com> > > remove_pagetable() isn't freeing PUD table. This causes memory > leak during memory unplug. Fix this. Fixes: ?? cheers > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > --- > arch/powerpc/mm/book3s64/radix_pgtable.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c > index 80be96d3018f..af57604f295f 100644 > --- a/arch/powerpc/mm/book3s64/radix_pgtable.c > +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c > @@ -708,6 +708,21 @@ static void free_pmd_table(pmd_t *pmd_start, pud_t *pud) > pud_clear(pud); > } > > +static void free_pud_table(pud_t *pud_start, p4d_t *p4d) > +{ > + pud_t *pud; > + int i; > + > + for (i = 0; i < PTRS_PER_PUD; i++) { > + pud = pud_start + i; > + if (!pud_none(*pud)) > + return; > + } > + > + pud_free(&init_mm, pud_start); > + p4d_clear(p4d); > +} > + > struct change_mapping_params { > pte_t *pte; > unsigned long start; > @@ -882,6 +897,7 @@ static void __meminit remove_pagetable(unsigned long start, unsigned long end) > > pud_base = (pud_t *)p4d_page_vaddr(*p4d); > remove_pud_table(pud_base, addr, next); > + free_pud_table(pud_base, p4d); > } > > spin_unlock(&init_mm.page_table_lock); > -- > 2.26.2
On 7/8/20 7:42 AM, Michael Ellerman wrote: > "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: >> From: Bharata B Rao <bharata@linux.ibm.com> >> >> remove_pagetable() isn't freeing PUD table. This causes memory >> leak during memory unplug. Fix this. > Fixes: 4b5d62ca17a1 ("powerpc/mm: add radix__remove_section_mapping()") > Fixes: ?? > > cheers > >> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> >> --- >> arch/powerpc/mm/book3s64/radix_pgtable.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c >> index 80be96d3018f..af57604f295f 100644 >> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c >> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c >> @@ -708,6 +708,21 @@ static void free_pmd_table(pmd_t *pmd_start, pud_t *pud) >> pud_clear(pud); >> } >> >> +static void free_pud_table(pud_t *pud_start, p4d_t *p4d) >> +{ >> + pud_t *pud; >> + int i; >> + >> + for (i = 0; i < PTRS_PER_PUD; i++) { >> + pud = pud_start + i; >> + if (!pud_none(*pud)) >> + return; >> + } >> + >> + pud_free(&init_mm, pud_start); >> + p4d_clear(p4d); >> +} >> + >> struct change_mapping_params { >> pte_t *pte; >> unsigned long start; >> @@ -882,6 +897,7 @@ static void __meminit remove_pagetable(unsigned long start, unsigned long end) >> >> pud_base = (pud_t *)p4d_page_vaddr(*p4d); >> remove_pud_table(pud_base, addr, next); >> + free_pud_table(pud_base, p4d); >> } >> >> spin_unlock(&init_mm.page_table_lock); >> -- >> 2.26.2
On Thu, Jun 25, 2020 at 12:15:45PM +0530, Aneesh Kumar K.V wrote: >remove_pagetable() isn't freeing PUD table. This causes memory >leak during memory unplug. Fix this. This has come up before: https://lore.kernel.org/linuxppc-dev/20190731061920.GA18807@in.ibm.com/ tl;dr, x86 intentionally does not free, and it wasn't quite clear if their motivation also applies to us. Probably not, but I thought it was worth mentioning again.
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c index 80be96d3018f..af57604f295f 100644 --- a/arch/powerpc/mm/book3s64/radix_pgtable.c +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c @@ -708,6 +708,21 @@ static void free_pmd_table(pmd_t *pmd_start, pud_t *pud) pud_clear(pud); } +static void free_pud_table(pud_t *pud_start, p4d_t *p4d) +{ + pud_t *pud; + int i; + + for (i = 0; i < PTRS_PER_PUD; i++) { + pud = pud_start + i; + if (!pud_none(*pud)) + return; + } + + pud_free(&init_mm, pud_start); + p4d_clear(p4d); +} + struct change_mapping_params { pte_t *pte; unsigned long start; @@ -882,6 +897,7 @@ static void __meminit remove_pagetable(unsigned long start, unsigned long end) pud_base = (pud_t *)p4d_page_vaddr(*p4d); remove_pud_table(pud_base, addr, next); + free_pud_table(pud_base, p4d); } spin_unlock(&init_mm.page_table_lock);