Re: [PATCH] mm: memblock: always inline memblock_alloc

[Faiyaz Mohammed]

On 11/11/2020 12:11 AM, Mike Rapoport wrote:
> Hi,
>
> On Tue, Nov 10, 2020 at 05:50:17PM +0530, Faiyaz Mohammed wrote:
> > Since memblock_alloc is not getting inlined, memblock_reserve owner info
> > is lost. Below information is not enough for memory accounting.
> > for example:
> > [    0.000000] memblock_alloc_try_nid: 1490 bytes align=0x40 nid=-1 from=0x0000000000000000 max_addr=0x0000000000000000 memblock_alloc+0x20/0x2c
> > [    0.000000] memblock_reserve: [0x000000023f09a3c0-0x000000023f09a991] memblock_alloc_range_nid+0xc0/0x188
> >
> > Add "__always_inline" to make sure it get inlined and to get the exact
> > owner of the memblock_reserve.
> > After adding __always_inline:
> > [    0.000000] memblock_alloc_try_nid: 1490 bytes align=0x40 nid=-1 from=0x0000000000000000 max_addr=0x0000000000000000 start_kernel+0xa4/0x568
> > [    0.000000] memblock_reserve: [0x000000023f09a3c0-0x000000023f09a991] memblock_alloc_range_nid+0xc0/0x188
>   
> I agree that making memblock_alloc() inline as well as other similar
> wrappers would improve the debugability.
> Still, it has nothing to do with memory accounting and owner tracking.
> Please update the patch description to better explain what it actually
> improves.

As describe in other thread, do memblock reserved accounting to track 
owners

to know size of memory allocated by different drivers/owners through

memblock_reserve, which help in comparing different kernel version and in

optimizations.
> > Signed-off-by: Faiyaz Mohammed <faiyazm@xxxxxxxxxxxxxx>
> > ---
> >   include/linux/memblock.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index ef13125..54f9544 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -404,7 +404,7 @@ void *memblock_alloc_try_nid(phys_addr_t size, phys_addr_t align,
> >   			     phys_addr_t min_addr, phys_addr_t max_addr,
> >   			     int nid);
> >   
> > -static inline void * __init memblock_alloc(phys_addr_t size,  phys_addr_t align)
> > +static __always_inline void * __init memblock_alloc(phys_addr_t size,  phys_addr_t align)
> I think simply dropping __init here will make memblock_alloc() inline.
> There are also several more convenience wrappers marked __init, do you
> mind removing the __init annotation for them as well?
Yes, if we drop __init, memblock_alloc will get inline but would it not 
increase

kernel footprint as the function will no more be released after kernel 
init?

> >   {
> >   	return memblock_alloc_try_nid(size, align, MEMBLOCK_LOW_LIMIT,
> >   				      MEMBLOCK_ALLOC_ACCESSIBLE, NUMA_NO_NODE);