Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: faiyazm=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 3A0CDC433C8
Subject: Re: [PATCH] mm: memblock: always inline memblock_alloc
To:     Mike Rapoport <rppt@kernel.org>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        vinmenon@codeaurora.org
References: <1605010817-21065-1-git-send-email-faiyazm@codeaurora.org>
 <20201110184157.GD4758@kernel.org>
From:   Faiyaz Mohammed <faiyazm@codeaurora.org>
Message-ID: <97ca3445-c405-cdc1-b9e6-6ed2386c9c57@codeaurora.org>
Date:   Wed, 11 Nov 2020 15:15:11 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.1
MIME-Version: 1.0
In-Reply-To: <20201110184157.GD4758@kernel.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


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);