* [PATCH] mm: memblock: always inline memblock_alloc @ 2020-11-10 12:20 Faiyaz Mohammed 2020-11-10 18:41 ` Mike Rapoport 0 siblings, 1 reply; 4+ messages in thread From: Faiyaz Mohammed @ 2020-11-10 12:20 UTC (permalink / raw) To: linux-mm, linux-kernel, rppt; +Cc: vinmenon, Faiyaz Mohammed 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 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) { return memblock_alloc_try_nid(size, align, MEMBLOCK_LOW_LIMIT, MEMBLOCK_ALLOC_ACCESSIBLE, NUMA_NO_NODE); -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: memblock: always inline memblock_alloc 2020-11-10 12:20 [PATCH] mm: memblock: always inline memblock_alloc Faiyaz Mohammed @ 2020-11-10 18:41 ` Mike Rapoport 2020-11-11 9:45 ` Faiyaz Mohammed 0 siblings, 1 reply; 4+ messages in thread From: Mike Rapoport @ 2020-11-10 18:41 UTC (permalink / raw) To: Faiyaz Mohammed; +Cc: linux-mm, linux-kernel, vinmenon 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. > 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? > { > return memblock_alloc_try_nid(size, align, MEMBLOCK_LOW_LIMIT, > MEMBLOCK_ALLOC_ACCESSIBLE, NUMA_NO_NODE); -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: memblock: always inline memblock_alloc 2020-11-10 18:41 ` Mike Rapoport @ 2020-11-11 9:45 ` Faiyaz Mohammed 2020-11-11 12:34 ` Mike Rapoport 0 siblings, 1 reply; 4+ messages in thread From: Faiyaz Mohammed @ 2020-11-11 9:45 UTC (permalink / raw) To: Mike Rapoport; +Cc: linux-mm, linux-kernel, vinmenon 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); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: memblock: always inline memblock_alloc 2020-11-11 9:45 ` Faiyaz Mohammed @ 2020-11-11 12:34 ` Mike Rapoport 0 siblings, 0 replies; 4+ messages in thread From: Mike Rapoport @ 2020-11-11 12:34 UTC (permalink / raw) To: Faiyaz Mohammed; +Cc: linux-mm, linux-kernel, vinmenon On Wed, Nov 11, 2020 at 03:15:11PM +0530, Faiyaz Mohammed wrote: > > 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? If memblock_alloc(), or any other function for that matter, is inlined, it does not have an entry in the object file, but it is rather inserted at the call sites. The functions that use memblock_alloc() may change, but they all should be __init anyway so I won't expect increase in the kernel post-init memory footprint. > > > { > > > return memblock_alloc_try_nid(size, align, MEMBLOCK_LOW_LIMIT, > > > MEMBLOCK_ALLOC_ACCESSIBLE, NUMA_NO_NODE); -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-11 12:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-10 12:20 [PATCH] mm: memblock: always inline memblock_alloc Faiyaz Mohammed 2020-11-10 18:41 ` Mike Rapoport 2020-11-11 9:45 ` Faiyaz Mohammed 2020-11-11 12:34 ` Mike Rapoport
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).