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

[Faiyaz Mohammed <faiyazm@codeaurora.org>]

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@codeaurora.org>
>> ---
>>   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);