| Message ID | 1632819849-511-1-git-send-email-faiyazm@codeaurora.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] mm: page_alloc: Add debug log in free_reserved_area for static memory | expand |
On 28.09.21 11:04, Faiyaz Mohammed wrote: > For INITRD and initmem memory is reserved through "memblock_reserve" > during boot up but it is free via "free_reserved_area" instead > of "memblock_free". > For example: > [ 0.294848] Freeing initrd memory: 12K. > [ 0.696688] Freeing unused kernel memory: 4096K. > > To get the start and end address of the above freed memory and to account > proper memblock added memblock_dbg log in "free_reserved_area". > After adding log: > [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] free_initrd_mem+0x20/0x28 > [ 0.294848] Freeing initrd memory: 12K. > [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] free_initmem+0x70/0xc8 > [ 0.696688] Freeing unused kernel memory: 4096K. > > Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> > --- > mm/page_alloc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index b37435c..f85c3b2 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, void *end, int poison, const char > pr_info("Freeing %s memory: %ldK\n", > s, pages << (PAGE_SHIFT - 10)); > > +#ifdef CONFIG_HAVE_MEMBLOCK > + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", > + __pa(start), __pa(end), (void *)_RET_IP_); > +#endif IMHO, the "memblock_free" part is misleading. Something was allocated early via memblock, then we transitioned to the buddy, now we're freeing that early allocation via the buddy. I would not expect memblock_dbg once memblock might not actually be alive anymore.
On 9/28/2021 4:09 PM, David Hildenbrand wrote: > On 28.09.21 11:04, Faiyaz Mohammed wrote: >> For INITRD and initmem memory is reserved through "memblock_reserve" >> during boot up but it is free via "free_reserved_area" instead >> of "memblock_free". >> For example: >> [ 0.294848] Freeing initrd memory: 12K. >> [ 0.696688] Freeing unused kernel memory: 4096K. >> >> To get the start and end address of the above freed memory and to account >> proper memblock added memblock_dbg log in "free_reserved_area". >> After adding log: >> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >> free_initrd_mem+0x20/0x28 >> [ 0.294848] Freeing initrd memory: 12K. >> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >> free_initmem+0x70/0xc8 >> [ 0.696688] Freeing unused kernel memory: 4096K. >> >> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >> --- >> mm/page_alloc.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index b37435c..f85c3b2 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, >> void *end, int poison, const char >> pr_info("Freeing %s memory: %ldK\n", >> s, pages << (PAGE_SHIFT - 10)); >> +#ifdef CONFIG_HAVE_MEMBLOCK >> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >> + __pa(start), __pa(end), (void *)_RET_IP_); >> +#endif > > IMHO, the "memblock_free" part is misleading. Something was allocated > early via memblock, then we transitioned to the buddy, now we're freeing > that early allocation via the buddy. > Yes, we're freeing the early allocation via buddy, but for proper memblock accounting we need this debug print. > I would not expect memblock_dbg once memblock might not actually be > alive anymore. > Here we can add pr_info as well, but the intention is to get callsite and address info when the memblock=debug is pass through command line and it is for debugging purpose. Thanks and regards, Mohammed Faiyaz
On 28.09.21 12:53, Faiyaz Mohammed wrote: > > > On 9/28/2021 4:09 PM, David Hildenbrand wrote: >> On 28.09.21 11:04, Faiyaz Mohammed wrote: >>> For INITRD and initmem memory is reserved through "memblock_reserve" >>> during boot up but it is free via "free_reserved_area" instead >>> of "memblock_free". >>> For example: >>> [ 0.294848] Freeing initrd memory: 12K. >>> [ 0.696688] Freeing unused kernel memory: 4096K. >>> >>> To get the start and end address of the above freed memory and to account >>> proper memblock added memblock_dbg log in "free_reserved_area". >>> After adding log: >>> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >>> free_initrd_mem+0x20/0x28 >>> [ 0.294848] Freeing initrd memory: 12K. >>> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >>> free_initmem+0x70/0xc8 >>> [ 0.696688] Freeing unused kernel memory: 4096K. >>> >>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >>> --- >>> mm/page_alloc.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>> index b37435c..f85c3b2 100644 >>> --- a/mm/page_alloc.c >>> +++ b/mm/page_alloc.c >>> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, >>> void *end, int poison, const char >>> pr_info("Freeing %s memory: %ldK\n", >>> s, pages << (PAGE_SHIFT - 10)); >>> +#ifdef CONFIG_HAVE_MEMBLOCK >>> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >>> + __pa(start), __pa(end), (void *)_RET_IP_); >>> +#endif >> >> IMHO, the "memblock_free" part is misleading. Something was allocated >> early via memblock, then we transitioned to the buddy, now we're freeing >> that early allocation via the buddy. >> Yes, we're freeing the early allocation via buddy, but for proper > memblock accounting we need this debug print. > What do you mean with "accounting" ? These are debug statements.
On 9/28/2021 4:46 PM, David Hildenbrand wrote: > On 28.09.21 12:53, Faiyaz Mohammed wrote: >> >> >> On 9/28/2021 4:09 PM, David Hildenbrand wrote: >>> On 28.09.21 11:04, Faiyaz Mohammed wrote: >>>> For INITRD and initmem memory is reserved through "memblock_reserve" >>>> during boot up but it is free via "free_reserved_area" instead >>>> of "memblock_free". >>>> For example: >>>> [ 0.294848] Freeing initrd memory: 12K. >>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>> >>>> To get the start and end address of the above freed memory and to >>>> account >>>> proper memblock added memblock_dbg log in "free_reserved_area". >>>> After adding log: >>>> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >>>> free_initrd_mem+0x20/0x28 >>>> [ 0.294848] Freeing initrd memory: 12K. >>>> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >>>> free_initmem+0x70/0xc8 >>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>> >>>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >>>> --- >>>> mm/page_alloc.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>> index b37435c..f85c3b2 100644 >>>> --- a/mm/page_alloc.c >>>> +++ b/mm/page_alloc.c >>>> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, >>>> void *end, int poison, const char >>>> pr_info("Freeing %s memory: %ldK\n", >>>> s, pages << (PAGE_SHIFT - 10)); >>>> +#ifdef CONFIG_HAVE_MEMBLOCK >>>> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >>>> + __pa(start), __pa(end), (void *)_RET_IP_); >>>> +#endif >>> >>> IMHO, the "memblock_free" part is misleading. Something was allocated >>> early via memblock, then we transitioned to the buddy, now we're freeing >>> that early allocation via the buddy. >>> Yes, we're freeing the early allocation via buddy, but for proper >> memblock accounting we need this debug print. >> > > What do you mean with "accounting" ? These are debug statements. > > Yes, these are debug statements, which help to know the a-b address belongs to x callsite. This info is required when memblock=debug is passed through command line and CONFIG_HAVE_MEMBLOCK is enabled. Thanks, Mohammed Faiyaz
On 29.09.21 10:58, Faiyaz Mohammed wrote: > > > On 9/28/2021 4:46 PM, David Hildenbrand wrote: >> On 28.09.21 12:53, Faiyaz Mohammed wrote: >>> >>> >>> On 9/28/2021 4:09 PM, David Hildenbrand wrote: >>>> On 28.09.21 11:04, Faiyaz Mohammed wrote: >>>>> For INITRD and initmem memory is reserved through "memblock_reserve" >>>>> during boot up but it is free via "free_reserved_area" instead >>>>> of "memblock_free". >>>>> For example: >>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>> >>>>> To get the start and end address of the above freed memory and to >>>>> account >>>>> proper memblock added memblock_dbg log in "free_reserved_area". >>>>> After adding log: >>>>> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >>>>> free_initrd_mem+0x20/0x28 >>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >>>>> free_initmem+0x70/0xc8 >>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>> >>>>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >>>>> --- >>>>> mm/page_alloc.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>> index b37435c..f85c3b2 100644 >>>>> --- a/mm/page_alloc.c >>>>> +++ b/mm/page_alloc.c >>>>> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, >>>>> void *end, int poison, const char >>>>> pr_info("Freeing %s memory: %ldK\n", >>>>> s, pages << (PAGE_SHIFT - 10)); >>>>> +#ifdef CONFIG_HAVE_MEMBLOCK >>>>> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >>>>> + __pa(start), __pa(end), (void *)_RET_IP_); >>>>> +#endif >>>> >>>> IMHO, the "memblock_free" part is misleading. Something was allocated >>>> early via memblock, then we transitioned to the buddy, now we're freeing >>>> that early allocation via the buddy. >>>> Yes, we're freeing the early allocation via buddy, but for proper >>> memblock accounting we need this debug print. >>> >> >> What do you mean with "accounting" ? These are debug statements. >> >> > Yes, these are debug statements, which help to know the a-b address > belongs to x callsite. This info is required when memblock=debug is > passed through command line and CONFIG_HAVE_MEMBLOCK is enabled. The issue I'm having is talking in the name of memblock "memblock_dbg, memblock_free", when memblock might no longer be around. We have other places where we free early memblock allocations back to the buddy.
Hi, Sorry for delayed response. On 9/29/2021 10:33 PM, David Hildenbrand wrote: > On 29.09.21 10:58, Faiyaz Mohammed wrote: >> >> >> On 9/28/2021 4:46 PM, David Hildenbrand wrote: >>> On 28.09.21 12:53, Faiyaz Mohammed wrote: >>>> >>>> >>>> On 9/28/2021 4:09 PM, David Hildenbrand wrote: >>>>> On 28.09.21 11:04, Faiyaz Mohammed wrote: >>>>>> For INITRD and initmem memory is reserved through "memblock_reserve" >>>>>> during boot up but it is free via "free_reserved_area" instead >>>>>> of "memblock_free". >>>>>> For example: >>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>> >>>>>> To get the start and end address of the above freed memory and to >>>>>> account >>>>>> proper memblock added memblock_dbg log in "free_reserved_area". >>>>>> After adding log: >>>>>> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >>>>>> free_initrd_mem+0x20/0x28 >>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >>>>>> free_initmem+0x70/0xc8 >>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>> >>>>>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >>>>>> --- >>>>>> mm/page_alloc.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>>> index b37435c..f85c3b2 100644 >>>>>> --- a/mm/page_alloc.c >>>>>> +++ b/mm/page_alloc.c >>>>>> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, >>>>>> void *end, int poison, const char >>>>>> pr_info("Freeing %s memory: %ldK\n", >>>>>> s, pages << (PAGE_SHIFT - 10)); >>>>>> +#ifdef CONFIG_HAVE_MEMBLOCK >>>>>> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >>>>>> + __pa(start), __pa(end), (void *)_RET_IP_); >>>>>> +#endif >>>>> >>>>> IMHO, the "memblock_free" part is misleading. Something was allocated >>>>> early via memblock, then we transitioned to the buddy, now we're >>>>> freeing >>>>> that early allocation via the buddy. >>>>> Yes, we're freeing the early allocation via buddy, but for proper >>>> memblock accounting we need this debug print. >>>> >>> >>> What do you mean with "accounting" ? These are debug statements. >>> >>> >> Yes, these are debug statements, which help to know the a-b address >> belongs to x callsite. This info is required when memblock=debug is >> passed through command line and CONFIG_HAVE_MEMBLOCK is enabled. > > The issue I'm having is talking in the name of memblock "memblock_dbg, > memblock_free", when memblock might no longer be around. We have other > places where we free early memblock allocations back to the buddy. I didn't find place where we free early memblock allocation back to the buddy. Why "memblock_dbg" print with "memblock_free" string?. - After buddy took over, buddy will free memblock reserved memory through free_reserved_area and it will print the freed memory size, but the freed memory through buddy still be part of memblock.reserved.regions. - To know the address ranges, added the "memblock_dbg" print along with "membloc_free" string. - If it is misleading or confusing, we can remove the "memblock_free" string from the "memblock_dgb" print and we can just print the address range when "memlock=debug" pass through command line. Thanks and regards, Mohammed Faiyaz
On 06.10.21 14:13, Faiyaz Mohammed wrote: > Hi, > > Sorry for delayed response. > > On 9/29/2021 10:33 PM, David Hildenbrand wrote: >> On 29.09.21 10:58, Faiyaz Mohammed wrote: >>> >>> >>> On 9/28/2021 4:46 PM, David Hildenbrand wrote: >>>> On 28.09.21 12:53, Faiyaz Mohammed wrote: >>>>> >>>>> >>>>> On 9/28/2021 4:09 PM, David Hildenbrand wrote: >>>>>> On 28.09.21 11:04, Faiyaz Mohammed wrote: >>>>>>> For INITRD and initmem memory is reserved through "memblock_reserve" >>>>>>> during boot up but it is free via "free_reserved_area" instead >>>>>>> of "memblock_free". >>>>>>> For example: >>>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>>> >>>>>>> To get the start and end address of the above freed memory and to >>>>>>> account >>>>>>> proper memblock added memblock_dbg log in "free_reserved_area". >>>>>>> After adding log: >>>>>>> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >>>>>>> free_initrd_mem+0x20/0x28 >>>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>>> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >>>>>>> free_initmem+0x70/0xc8 >>>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>>> >>>>>>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >>>>>>> --- >>>>>>> mm/page_alloc.c | 5 +++++ >>>>>>> 1 file changed, 5 insertions(+) >>>>>>> >>>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>>>> index b37435c..f85c3b2 100644 >>>>>>> --- a/mm/page_alloc.c >>>>>>> +++ b/mm/page_alloc.c >>>>>>> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, >>>>>>> void *end, int poison, const char >>>>>>> pr_info("Freeing %s memory: %ldK\n", >>>>>>> s, pages << (PAGE_SHIFT - 10)); >>>>>>> +#ifdef CONFIG_HAVE_MEMBLOCK >>>>>>> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >>>>>>> + __pa(start), __pa(end), (void *)_RET_IP_); >>>>>>> +#endif >>>>>> >>>>>> IMHO, the "memblock_free" part is misleading. Something was allocated >>>>>> early via memblock, then we transitioned to the buddy, now we're >>>>>> freeing >>>>>> that early allocation via the buddy. >>>>>> Yes, we're freeing the early allocation via buddy, but for proper >>>>> memblock accounting we need this debug print. >>>>> >>>> >>>> What do you mean with "accounting" ? These are debug statements. >>>> >>>> >>> Yes, these are debug statements, which help to know the a-b address >>> belongs to x callsite. This info is required when memblock=debug is >>> passed through command line and CONFIG_HAVE_MEMBLOCK is enabled. >> >> The issue I'm having is talking in the name of memblock "memblock_dbg, >> memblock_free", when memblock might no longer be around. We have other >> places where we free early memblock allocations back to the buddy. > I didn't find place where we free early memblock allocation back to the > buddy. One example I know is section_deactivate()->free_map_bootmem()->vmemmap_free()-> ... free_pagetable()->free_reserved_page(). when we free the vmemmap allocated via memblock back to the buddy. > > Why "memblock_dbg" print with "memblock_free" string?. > - After buddy took over, buddy will free memblock reserved memory > through free_reserved_area and it will print the freed memory size, but > the freed memory through buddy still be part of memblock.reserved.regions. > - To know the address ranges, added the "memblock_dbg" print along with > "membloc_free" string. > - If it is misleading or confusing, we can remove the "memblock_free" > string from the "memblock_dgb" print and we can just print the address > range when "memlock=debug" pass through command line. That would be better, but do we really have to depend on "memlock=debug"? Can't we do pr_debug() ?
On 10/6/2021 5:48 PM, David Hildenbrand wrote: > On 06.10.21 14:13, Faiyaz Mohammed wrote: >> Hi, >> >> Sorry for delayed response. >> >> On 9/29/2021 10:33 PM, David Hildenbrand wrote: >>> On 29.09.21 10:58, Faiyaz Mohammed wrote: >>>> >>>> >>>> On 9/28/2021 4:46 PM, David Hildenbrand wrote: >>>>> On 28.09.21 12:53, Faiyaz Mohammed wrote: >>>>>> >>>>>> >>>>>> On 9/28/2021 4:09 PM, David Hildenbrand wrote: >>>>>>> On 28.09.21 11:04, Faiyaz Mohammed wrote: >>>>>>>> For INITRD and initmem memory is reserved through >>>>>>>> "memblock_reserve" >>>>>>>> during boot up but it is free via "free_reserved_area" instead >>>>>>>> of "memblock_free". >>>>>>>> For example: >>>>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>>>> >>>>>>>> To get the start and end address of the above freed memory and to >>>>>>>> account >>>>>>>> proper memblock added memblock_dbg log in "free_reserved_area". >>>>>>>> After adding log: >>>>>>>> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >>>>>>>> free_initrd_mem+0x20/0x28 >>>>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>>>> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >>>>>>>> free_initmem+0x70/0xc8 >>>>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>>>> >>>>>>>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >>>>>>>> --- >>>>>>>> mm/page_alloc.c | 5 +++++ >>>>>>>> 1 file changed, 5 insertions(+) >>>>>>>> >>>>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>>>>> index b37435c..f85c3b2 100644 >>>>>>>> --- a/mm/page_alloc.c >>>>>>>> +++ b/mm/page_alloc.c >>>>>>>> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void >>>>>>>> *start, >>>>>>>> void *end, int poison, const char >>>>>>>> pr_info("Freeing %s memory: %ldK\n", >>>>>>>> s, pages << (PAGE_SHIFT - 10)); >>>>>>>> +#ifdef CONFIG_HAVE_MEMBLOCK >>>>>>>> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >>>>>>>> + __pa(start), __pa(end), (void *)_RET_IP_); >>>>>>>> +#endif >>>>>>> >>>>>>> IMHO, the "memblock_free" part is misleading. Something was >>>>>>> allocated >>>>>>> early via memblock, then we transitioned to the buddy, now we're >>>>>>> freeing >>>>>>> that early allocation via the buddy. >>>>>>> Yes, we're freeing the early allocation via buddy, but for proper >>>>>> memblock accounting we need this debug print. >>>>>> >>>>> >>>>> What do you mean with "accounting" ? These are debug statements. >>>>> >>>>> >>>> Yes, these are debug statements, which help to know the a-b address >>>> belongs to x callsite. This info is required when memblock=debug is >>>> passed through command line and CONFIG_HAVE_MEMBLOCK is enabled. >>> >>> The issue I'm having is talking in the name of memblock "memblock_dbg, >>> memblock_free", when memblock might no longer be around. We have other >>> places where we free early memblock allocations back to the buddy. >> I didn't find place where we free early memblock allocation back to the >> buddy. > > One example I know is > > section_deactivate()->free_map_bootmem()->vmemmap_free()-> ... > free_pagetable()->free_reserved_page(). > > when we free the vmemmap allocated via memblock back to the buddy. > >> >> Why "memblock_dbg" print with "memblock_free" string?. >> - After buddy took over, buddy will free memblock reserved memory >> through free_reserved_area and it will print the freed memory size, but >> the freed memory through buddy still be part of >> memblock.reserved.regions. >> - To know the address ranges, added the "memblock_dbg" print along with >> "membloc_free" string. >> - If it is misleading or confusing, we can remove the "memblock_free" >> string from the "memblock_dgb" print and we can just print the address >> range when "memlock=debug" pass through command line. > > That would be better, but do we really have to depend on > "memlock=debug"? Yes, because we need that print when memblock=debug is pass through command line, like other memblock debug messages. Thanks and regards, Mohammed Faiyaz
On 10/6/2021 5:48 PM, David Hildenbrand wrote: > On 06.10.21 14:13, Faiyaz Mohammed wrote: >> Hi, >> >> Sorry for delayed response. >> >> On 9/29/2021 10:33 PM, David Hildenbrand wrote: >>> On 29.09.21 10:58, Faiyaz Mohammed wrote: >>>> >>>> >>>> On 9/28/2021 4:46 PM, David Hildenbrand wrote: >>>>> On 28.09.21 12:53, Faiyaz Mohammed wrote: >>>>>> >>>>>> >>>>>> On 9/28/2021 4:09 PM, David Hildenbrand wrote: >>>>>>> On 28.09.21 11:04, Faiyaz Mohammed wrote: >>>>>>>> For INITRD and initmem memory is reserved through >>>>>>>> "memblock_reserve" >>>>>>>> during boot up but it is free via "free_reserved_area" instead >>>>>>>> of "memblock_free". >>>>>>>> For example: >>>>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>>>> >>>>>>>> To get the start and end address of the above freed memory and to >>>>>>>> account >>>>>>>> proper memblock added memblock_dbg log in "free_reserved_area". >>>>>>>> After adding log: >>>>>>>> [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] >>>>>>>> free_initrd_mem+0x20/0x28 >>>>>>>> [ 0.294848] Freeing initrd memory: 12K. >>>>>>>> [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] >>>>>>>> free_initmem+0x70/0xc8 >>>>>>>> [ 0.696688] Freeing unused kernel memory: 4096K. >>>>>>>> >>>>>>>> Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> >>>>>>>> --- >>>>>>>> mm/page_alloc.c | 5 +++++ >>>>>>>> 1 file changed, 5 insertions(+) >>>>>>>> >>>>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>>>>> index b37435c..f85c3b2 100644 >>>>>>>> --- a/mm/page_alloc.c >>>>>>>> +++ b/mm/page_alloc.c >>>>>>>> @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void >>>>>>>> *start, >>>>>>>> void *end, int poison, const char >>>>>>>> pr_info("Freeing %s memory: %ldK\n", >>>>>>>> s, pages << (PAGE_SHIFT - 10)); >>>>>>>> +#ifdef CONFIG_HAVE_MEMBLOCK >>>>>>>> + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", >>>>>>>> + __pa(start), __pa(end), (void *)_RET_IP_); >>>>>>>> +#endif >>>>>>> >>>>>>> IMHO, the "memblock_free" part is misleading. Something was >>>>>>> allocated >>>>>>> early via memblock, then we transitioned to the buddy, now we're >>>>>>> freeing >>>>>>> that early allocation via the buddy. >>>>>>> Yes, we're freeing the early allocation via buddy, but for proper >>>>>> memblock accounting we need this debug print. >>>>>> >>>>> >>>>> What do you mean with "accounting" ? These are debug statements. >>>>> >>>>> >>>> Yes, these are debug statements, which help to know the a-b address >>>> belongs to x callsite. This info is required when memblock=debug is >>>> passed through command line and CONFIG_HAVE_MEMBLOCK is enabled. >>> >>> The issue I'm having is talking in the name of memblock "memblock_dbg, >>> memblock_free", when memblock might no longer be around. We have other >>> places where we free early memblock allocations back to the buddy. >> I didn't find place where we free early memblock allocation back to the >> buddy. > > One example I know is > > section_deactivate()->free_map_bootmem()->vmemmap_free()-> ... > free_pagetable()->free_reserved_page(). > > when we free the vmemmap allocated via memblock back to the buddy. > >> >> Why "memblock_dbg" print with "memblock_free" string?. >> - After buddy took over, buddy will free memblock reserved memory >> through free_reserved_area and it will print the freed memory size, but >> the freed memory through buddy still be part of >> memblock.reserved.regions. >> - To know the address ranges, added the "memblock_dbg" print along with >> "membloc_free" string. >> - If it is misleading or confusing, we can remove the "memblock_free" >> string from the "memblock_dgb" print and we can just print the address >> range when "memlock=debug" pass through command line. > > That would be better, but do we really have to depend on > "memlock=debug"? Can't we do pr_debug() ? > Yes, I think we can do pr_debug. I will update the patch and push again. Thanks and regards, Mohammed Faiyaz
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index b37435c..f85c3b2 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -8129,6 +8129,11 @@ unsigned long free_reserved_area(void *start, void *end, int poison, const char pr_info("Freeing %s memory: %ldK\n", s, pages << (PAGE_SHIFT - 10)); +#ifdef CONFIG_HAVE_MEMBLOCK + memblock_dbg("memblock_free: [%#016llx-%#016llx] %pS\n", + __pa(start), __pa(end), (void *)_RET_IP_); +#endif + return pages; }
For INITRD and initmem memory is reserved through "memblock_reserve" during boot up but it is free via "free_reserved_area" instead of "memblock_free". For example: [ 0.294848] Freeing initrd memory: 12K. [ 0.696688] Freeing unused kernel memory: 4096K. To get the start and end address of the above freed memory and to account proper memblock added memblock_dbg log in "free_reserved_area". After adding log: [ 0.294837] memblock_free: [0x00000083600000-0x00000083603000] free_initrd_mem+0x20/0x28 [ 0.294848] Freeing initrd memory: 12K. [ 0.695246] memblock_free: [0x00000081600000-0x00000081a00000] free_initmem+0x70/0xc8 [ 0.696688] Freeing unused kernel memory: 4096K. Signed-off-by: Faiyaz Mohammed <faiyazm@codeaurora.org> --- mm/page_alloc.c | 5 +++++ 1 file changed, 5 insertions(+)