| Message ID | 20210830141051.64090-3-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanups and fixup for page_alloc | expand |
Hello. On pondělí 30. srpna 2021 16:10:47 CEST Miaohe Lin wrote: > Use helper macro K() to convert the pages to the corresponding size. > Minor readability improvement. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/page_alloc.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index dbb3338d9287..d3983244f56f 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, void > *end, int poison, const char } > > if (pages && s) > - pr_info("Freeing %s memory: %ldK\n", > - s, pages << (PAGE_SHIFT - 10)); > + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); > > return pages; > } > @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) > ", %luK highmem" > #endif > ")\n", > - nr_free_pages() << (PAGE_SHIFT - 10), > - physpages << (PAGE_SHIFT - 10), > + K(nr_free_pages()), K(physpages), > codesize >> 10, datasize >> 10, rosize >> 10, > (init_data_size + init_code_size) >> 10, bss_size >> 10, > - (physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT - 10), > - totalcma_pages << (PAGE_SHIFT - 10) > + K(physpages - totalram_pages() - totalcma_pages), > + K(totalcma_pages) > #ifdef CONFIG_HIGHMEM > - , totalhigh_pages() << (PAGE_SHIFT - 10) > + , K(totalhigh_pages()) > #endif > ); > } (my concern is not quite within the scope of this submission, but I'll ask anyway) Given we have this: ``` git grep '#define K(x)' v5.14 v5.14:drivers/base/node.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) v5.14:drivers/net/hamradio/scc.c:#define K(x) kiss->x v5.14:kernel/debug/kdb/kdb_main.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) v5.14:mm/backing-dev.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) v5.14:mm/memcontrol.c:#define K(x) ((x) << (PAGE_SHIFT-10)) v5.14:mm/oom_kill.c:#define K(x) ((x) << (PAGE_SHIFT-10)) v5.14:mm/page_alloc.c:#define K(x) ((x) << (PAGE_SHIFT-10)) ``` Shouldn't this macro go to some header file instead to get rid of define repetitions? Thanks.
On 2021/8/31 16:54, Oleksandr Natalenko wrote: > Hello. > > On pondělí 30. srpna 2021 16:10:47 CEST Miaohe Lin wrote: >> Use helper macro K() to convert the pages to the corresponding size. >> Minor readability improvement. >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/page_alloc.c | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index dbb3338d9287..d3983244f56f 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, void >> *end, int poison, const char } >> >> if (pages && s) >> - pr_info("Freeing %s memory: %ldK\n", >> - s, pages << (PAGE_SHIFT - 10)); >> + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); >> >> return pages; >> } >> @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) >> ", %luK highmem" >> #endif >> ")\n", >> - nr_free_pages() << (PAGE_SHIFT - 10), >> - physpages << (PAGE_SHIFT - 10), >> + K(nr_free_pages()), K(physpages), >> codesize >> 10, datasize >> 10, rosize >> 10, >> (init_data_size + init_code_size) >> 10, bss_size >> 10, >> - (physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT > - 10), >> - totalcma_pages << (PAGE_SHIFT - 10) >> + K(physpages - totalram_pages() - totalcma_pages), >> + K(totalcma_pages) >> #ifdef CONFIG_HIGHMEM >> - , totalhigh_pages() << (PAGE_SHIFT - 10) >> + , K(totalhigh_pages()) >> #endif >> ); >> } > > (my concern is not quite within the scope of this submission, but I'll ask > anyway) > > Given we have this: > > ``` > git grep '#define K(x)' v5.14 > v5.14:drivers/base/node.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > v5.14:drivers/net/hamradio/scc.c:#define K(x) kiss->x > v5.14:kernel/debug/kdb/kdb_main.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > v5.14:mm/backing-dev.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > v5.14:mm/memcontrol.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > v5.14:mm/oom_kill.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > v5.14:mm/page_alloc.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > ``` > > Shouldn't this macro go to some header file instead to get rid of define > repetitions? Many thanks for figuring this out. I think we should get rid of these repetitions too. But I'am not sure where this definition should be. Any suggestion? Thanks. > > Thanks. >
On Mon, Aug 30, 2021 at 10:10:47PM +0800, Miaohe Lin wrote: > Use helper macro K() to convert the pages to the corresponding size. > Minor readability improvement. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> Acked-by: Mel Gorman <mgorman@techsingularity.net>
On 30.08.21 16:10, Miaohe Lin wrote: > Use helper macro K() to convert the pages to the corresponding size. > Minor readability improvement. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/page_alloc.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index dbb3338d9287..d3983244f56f 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, void *end, int poison, const char > } > > if (pages && s) > - pr_info("Freeing %s memory: %ldK\n", > - s, pages << (PAGE_SHIFT - 10)); > + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); > > return pages; > } > @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) > ", %luK highmem" > #endif > ")\n", > - nr_free_pages() << (PAGE_SHIFT - 10), > - physpages << (PAGE_SHIFT - 10), > + K(nr_free_pages()), K(physpages), > codesize >> 10, datasize >> 10, rosize >> 10, > (init_data_size + init_code_size) >> 10, bss_size >> 10, > - (physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT - 10), > - totalcma_pages << (PAGE_SHIFT - 10) > + K(physpages - totalram_pages() - totalcma_pages), > + K(totalcma_pages) > #ifdef CONFIG_HIGHMEM > - , totalhigh_pages() << (PAGE_SHIFT - 10) > + , K(totalhigh_pages()) > #endif > ); > } > Reviewed-by: David Hildenbrand <david@redhat.com>
Hello. On úterý 31. srpna 2021 13:08:42 CEST Miaohe Lin wrote: > On 2021/8/31 16:54, Oleksandr Natalenko wrote: > > Hello. > > > > On pondělí 30. srpna 2021 16:10:47 CEST Miaohe Lin wrote: > >> Use helper macro K() to convert the pages to the corresponding size. > >> Minor readability improvement. > >> > >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > >> --- > >> > >> mm/page_alloc.c | 12 +++++------- > >> 1 file changed, 5 insertions(+), 7 deletions(-) > >> > >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c > >> index dbb3338d9287..d3983244f56f 100644 > >> --- a/mm/page_alloc.c > >> +++ b/mm/page_alloc.c > >> @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, void > >> *end, int poison, const char } > >> > >> if (pages && s) > >> > >> - pr_info("Freeing %s memory: %ldK\n", > >> - s, pages << (PAGE_SHIFT - 10)); > >> + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); > >> > >> return pages; > >> > >> } > >> > >> @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) > >> > >> ", %luK highmem" > >> > >> #endif > >> > >> ")\n", > >> > >> - nr_free_pages() << (PAGE_SHIFT - 10), > >> - physpages << (PAGE_SHIFT - 10), > >> + K(nr_free_pages()), K(physpages), > >> > >> codesize >> 10, datasize >> 10, rosize >> 10, > >> (init_data_size + init_code_size) >> 10, bss_size >> 10, > >> > >> - (physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT > > > > - 10), > > > >> - totalcma_pages << (PAGE_SHIFT - 10) > >> + K(physpages - totalram_pages() - totalcma_pages), > >> + K(totalcma_pages) > >> > >> #ifdef CONFIG_HIGHMEM > >> > >> - , totalhigh_pages() << (PAGE_SHIFT - 10) > >> + , K(totalhigh_pages()) > >> > >> #endif > >> > >> ); > >> > >> } > > > > (my concern is not quite within the scope of this submission, but I'll ask > > anyway) > > > > Given we have this: > > > > ``` > > git grep '#define K(x)' v5.14 > > v5.14:drivers/base/node.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > > v5.14:drivers/net/hamradio/scc.c:#define K(x) kiss->x > > v5.14:kernel/debug/kdb/kdb_main.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > > v5.14:mm/backing-dev.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > > v5.14:mm/memcontrol.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > > v5.14:mm/oom_kill.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > > v5.14:mm/page_alloc.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > > ``` > > > > Shouldn't this macro go to some header file instead to get rid of define > > repetitions? > > Many thanks for figuring this out. I think we should get rid of these > repetitions too. But I'am not sure where this definition should be. Any > suggestion? Thanks. I'm not sure what place suits best. At first I thought maybe linux/mm.h or linux/mm_inline.h, but since PAGE_SHIFT is declared in asm-generic/page.h, probably K(x) can also go there as well?
On 2021/8/31 22:19, Oleksandr Natalenko wrote: > Hello. > > On úterý 31. srpna 2021 13:08:42 CEST Miaohe Lin wrote: >> On 2021/8/31 16:54, Oleksandr Natalenko wrote: >>> Hello. >>> >>> On pondělí 30. srpna 2021 16:10:47 CEST Miaohe Lin wrote: >>>> Use helper macro K() to convert the pages to the corresponding size. >>>> Minor readability improvement. >>>> >>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >>>> --- >>>> >>>> mm/page_alloc.c | 12 +++++------- >>>> 1 file changed, 5 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>> index dbb3338d9287..d3983244f56f 100644 >>>> --- a/mm/page_alloc.c >>>> +++ b/mm/page_alloc.c >>>> @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, void >>>> *end, int poison, const char } >>>> >>>> if (pages && s) >>>> >>>> - pr_info("Freeing %s memory: %ldK\n", >>>> - s, pages << (PAGE_SHIFT - 10)); >>>> + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); >>>> >>>> return pages; >>>> >>>> } >>>> >>>> @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) >>>> >>>> ", %luK highmem" >>>> >>>> #endif >>>> >>>> ")\n", >>>> >>>> - nr_free_pages() << (PAGE_SHIFT - 10), >>>> - physpages << (PAGE_SHIFT - 10), >>>> + K(nr_free_pages()), K(physpages), >>>> >>>> codesize >> 10, datasize >> 10, rosize >> 10, >>>> (init_data_size + init_code_size) >> 10, bss_size >> 10, >>>> >>>> - (physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT >>> >>> - 10), >>> >>>> - totalcma_pages << (PAGE_SHIFT - 10) >>>> + K(physpages - totalram_pages() - totalcma_pages), >>>> + K(totalcma_pages) >>>> >>>> #ifdef CONFIG_HIGHMEM >>>> >>>> - , totalhigh_pages() << (PAGE_SHIFT - 10) >>>> + , K(totalhigh_pages()) >>>> >>>> #endif >>>> >>>> ); >>>> >>>> } >>> >>> (my concern is not quite within the scope of this submission, but I'll ask >>> anyway) >>> >>> Given we have this: >>> >>> ``` >>> git grep '#define K(x)' v5.14 >>> v5.14:drivers/base/node.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) >>> v5.14:drivers/net/hamradio/scc.c:#define K(x) kiss->x >>> v5.14:kernel/debug/kdb/kdb_main.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) >>> v5.14:mm/backing-dev.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) >>> v5.14:mm/memcontrol.c:#define K(x) ((x) << (PAGE_SHIFT-10)) >>> v5.14:mm/oom_kill.c:#define K(x) ((x) << (PAGE_SHIFT-10)) >>> v5.14:mm/page_alloc.c:#define K(x) ((x) << (PAGE_SHIFT-10)) >>> ``` >>> >>> Shouldn't this macro go to some header file instead to get rid of define >>> repetitions? >> >> Many thanks for figuring this out. I think we should get rid of these >> repetitions too. But I'am not sure where this definition should be. Any >> suggestion? Thanks. > > I'm not sure what place suits best. At first I thought maybe linux/mm.h or > linux/mm_inline.h, but since PAGE_SHIFT is declared in asm-generic/page.h, > probably K(x) can also go there as well? K(x) is relevant with PAGE_SHIFT. So I think K(x) can also go asm-generic/page.h too. Am I supposed to do this when free or will you kindly do this? Many thanks. >
Hello. On středa 1. září 2021 9:37:49 CEST Miaohe Lin wrote: > On 2021/8/31 22:19, Oleksandr Natalenko wrote: > > On úterý 31. srpna 2021 13:08:42 CEST Miaohe Lin wrote: > >> On 2021/8/31 16:54, Oleksandr Natalenko wrote: > >>> On pondělí 30. srpna 2021 16:10:47 CEST Miaohe Lin wrote: > >>>> Use helper macro K() to convert the pages to the corresponding size. > >>>> Minor readability improvement. > >>>> > >>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > >>>> --- > >>>> > >>>> mm/page_alloc.c | 12 +++++------- > >>>> 1 file changed, 5 insertions(+), 7 deletions(-) > >>>> > >>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c > >>>> index dbb3338d9287..d3983244f56f 100644 > >>>> --- a/mm/page_alloc.c > >>>> +++ b/mm/page_alloc.c > >>>> @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, > >>>> void > >>>> *end, int poison, const char } > >>>> > >>>> if (pages && s) > >>>> > >>>> - pr_info("Freeing %s memory: %ldK\n", > >>>> - s, pages << (PAGE_SHIFT - 10)); > >>>> + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); > >>>> > >>>> return pages; > >>>> > >>>> } > >>>> > >>>> @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) > >>>> > >>>> ", %luK highmem" > >>>> > >>>> #endif > >>>> > >>>> ")\n", > >>>> > >>>> - nr_free_pages() << (PAGE_SHIFT - 10), > >>>> - physpages << (PAGE_SHIFT - 10), > >>>> + K(nr_free_pages()), K(physpages), > >>>> > >>>> codesize >> 10, datasize >> 10, rosize >> 10, > >>>> (init_data_size + init_code_size) >> 10, bss_size >> 10, > >>>> > >>>> - (physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT > >>> > >>> - 10), > >>> > >>>> - totalcma_pages << (PAGE_SHIFT - 10) > >>>> + K(physpages - totalram_pages() - totalcma_pages), > >>>> + K(totalcma_pages) > >>>> > >>>> #ifdef CONFIG_HIGHMEM > >>>> > >>>> - , totalhigh_pages() << (PAGE_SHIFT - 10) > >>>> + , K(totalhigh_pages()) > >>>> > >>>> #endif > >>>> > >>>> ); > >>>> > >>>> } > >>> > >>> (my concern is not quite within the scope of this submission, but I'll > >>> ask > >>> anyway) > >>> > >>> Given we have this: > >>> > >>> ``` > >>> git grep '#define K(x)' v5.14 > >>> v5.14:drivers/base/node.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > >>> v5.14:drivers/net/hamradio/scc.c:#define K(x) kiss->x > >>> v5.14:kernel/debug/kdb/kdb_main.c:#define K(x) ((x) << (PAGE_SHIFT - > >>> 10)) > >>> v5.14:mm/backing-dev.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) > >>> v5.14:mm/memcontrol.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > >>> v5.14:mm/oom_kill.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > >>> v5.14:mm/page_alloc.c:#define K(x) ((x) << (PAGE_SHIFT-10)) > >>> ``` > >>> > >>> Shouldn't this macro go to some header file instead to get rid of define > >>> repetitions? > >> > >> Many thanks for figuring this out. I think we should get rid of these > >> repetitions too. But I'am not sure where this definition should be. Any > >> suggestion? Thanks. > > > > I'm not sure what place suits best. At first I thought maybe linux/mm.h or > > linux/mm_inline.h, but since PAGE_SHIFT is declared in asm-generic/page.h, > > probably K(x) can also go there as well? > > K(x) is relevant with PAGE_SHIFT. So I think K(x) can also go > asm-generic/page.h too. Actually, the comment in this file says: ``` 4 /* 5 * Generic page.h implementation, for NOMMU architectures. 6 * This provides the dummy definitions for the memory management. 7 */ ``` so it seems I was wrong about this being an appropriate place. > Am I supposed to do this when free or will you > kindly do this? Let me just try to cram this into mm.h and send it out, and then we'll see what comments people suggest. Thanks.
On 2021/9/1 15:46, Oleksandr Natalenko wrote: > Hello. > > On středa 1. září 2021 9:37:49 CEST Miaohe Lin wrote: >> On 2021/8/31 22:19, Oleksandr Natalenko wrote: >>> On úterý 31. srpna 2021 13:08:42 CEST Miaohe Lin wrote: >>>> On 2021/8/31 16:54, Oleksandr Natalenko wrote: >>>>> On pondělí 30. srpna 2021 16:10:47 CEST Miaohe Lin wrote: >>>>>> Use helper macro K() to convert the pages to the corresponding size. >>>>>> Minor readability improvement. >>>>>> >>>>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >>>>>> --- >>>>>> >>>>>> mm/page_alloc.c | 12 +++++------- >>>>>> 1 file changed, 5 insertions(+), 7 deletions(-) >>>>>> >>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>>> index dbb3338d9287..d3983244f56f 100644 >>>>>> --- a/mm/page_alloc.c >>>>>> +++ b/mm/page_alloc.c >>>>>> @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, >>>>>> void >>>>>> *end, int poison, const char } >>>>>> >>>>>> if (pages && s) >>>>>> >>>>>> - pr_info("Freeing %s memory: %ldK\n", >>>>>> - s, pages << (PAGE_SHIFT - 10)); >>>>>> + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); >>>>>> >>>>>> return pages; >>>>>> >>>>>> } >>>>>> >>>>>> @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) >>>>>> >>>>>> ", %luK highmem" >>>>>> >>>>>> #endif >>>>>> >>>>>> ")\n", >>>>>> >>>>>> - nr_free_pages() << (PAGE_SHIFT - 10), >>>>>> - physpages << (PAGE_SHIFT - 10), >>>>>> + K(nr_free_pages()), K(physpages), >>>>>> >>>>>> codesize >> 10, datasize >> 10, rosize >> 10, >>>>>> (init_data_size + init_code_size) >> 10, bss_size >> 10, >>>>>> >>>>>> - (physpages - totalram_pages() - totalcma_pages) << > (PAGE_SHIFT >>>>> >>>>> - 10), >>>>> >>>>>> - totalcma_pages << (PAGE_SHIFT - 10) >>>>>> + K(physpages - totalram_pages() - totalcma_pages), >>>>>> + K(totalcma_pages) >>>>>> >>>>>> #ifdef CONFIG_HIGHMEM >>>>>> >>>>>> - , totalhigh_pages() << (PAGE_SHIFT - 10) >>>>>> + , K(totalhigh_pages()) >>>>>> >>>>>> #endif >>>>>> >>>>>> ); >>>>>> >>>>>> } >>>>> >>>>> (my concern is not quite within the scope of this submission, but I'll >>>>> ask >>>>> anyway) >>>>> >>>>> Given we have this: >>>>> >>>>> ``` >>>>> git grep '#define K(x)' v5.14 >>>>> v5.14:drivers/base/node.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) >>>>> v5.14:drivers/net/hamradio/scc.c:#define K(x) kiss->x >>>>> v5.14:kernel/debug/kdb/kdb_main.c:#define K(x) ((x) << (PAGE_SHIFT - >>>>> 10)) >>>>> v5.14:mm/backing-dev.c:#define K(x) ((x) << (PAGE_SHIFT - 10)) >>>>> v5.14:mm/memcontrol.c:#define K(x) ((x) << (PAGE_SHIFT-10)) >>>>> v5.14:mm/oom_kill.c:#define K(x) ((x) << (PAGE_SHIFT-10)) >>>>> v5.14:mm/page_alloc.c:#define K(x) ((x) << (PAGE_SHIFT-10)) >>>>> ``` >>>>> >>>>> Shouldn't this macro go to some header file instead to get rid of define >>>>> repetitions? >>>> >>>> Many thanks for figuring this out. I think we should get rid of these >>>> repetitions too. But I'am not sure where this definition should be. Any >>>> suggestion? Thanks. >>> >>> I'm not sure what place suits best. At first I thought maybe linux/mm.h or >>> linux/mm_inline.h, but since PAGE_SHIFT is declared in asm-generic/page.h, >>> probably K(x) can also go there as well? >> >> K(x) is relevant with PAGE_SHIFT. So I think K(x) can also go >> asm-generic/page.h too. > > Actually, the comment in this file says: > > ``` > 4 /* > 5 * Generic page.h implementation, for NOMMU architectures. > 6 * This provides the dummy definitions for the memory management. > 7 */ > ``` > > so it seems I was wrong about this being an appropriate place. It's hard to find the appropriate place. > >> Am I supposed to do this when free or will you >> kindly do this? > > Let me just try to cram this into mm.h and send it out, and then we'll see > what comments people suggest. > Many thanks for doing this. :) > Thanks. >
From: Miaohe Lin > Sent: 01 September 2021 08:38 ... > >>> Shouldn't this macro go to some header file instead to get rid of define > >>> repetitions? > >> > >> Many thanks for figuring this out. I think we should get rid of these > >> repetitions too. But I'am not sure where this definition should be. Any > >> suggestion? Thanks. > > > > I'm not sure what place suits best. At first I thought maybe linux/mm.h or > > linux/mm_inline.h, but since PAGE_SHIFT is declared in asm-generic/page.h, > > probably K(x) can also go there as well? > > K(x) is relevant with PAGE_SHIFT. So I think K(x) can also go asm-generic/page.h too. > Am I supposed to do this when free or will you kindly do this? It is a bit of a short name to go in a public header file. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On 2021/9/2 5:25, David Laight wrote: > From: Miaohe Lin >> Sent: 01 September 2021 08:38 > ... >>>>> Shouldn't this macro go to some header file instead to get rid of define >>>>> repetitions? >>>> >>>> Many thanks for figuring this out. I think we should get rid of these >>>> repetitions too. But I'am not sure where this definition should be. Any >>>> suggestion? Thanks. >>> >>> I'm not sure what place suits best. At first I thought maybe linux/mm.h or >>> linux/mm_inline.h, but since PAGE_SHIFT is declared in asm-generic/page.h, >>> probably K(x) can also go there as well? >> >> K(x) is relevant with PAGE_SHIFT. So I think K(x) can also go asm-generic/page.h too. >> Am I supposed to do this when free or will you kindly do this? > > It is a bit of a short name to go in a public header file. > It seems K(x) is a bit of short and it needs a more self-annotated name. We can discuss this there: https://lkml.org/lkml/2021/9/1/334 Thanks. > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) >
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index dbb3338d9287..d3983244f56f 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -8134,8 +8134,7 @@ unsigned long free_reserved_area(void *start, void *end, int poison, const char } if (pages && s) - pr_info("Freeing %s memory: %ldK\n", - s, pages << (PAGE_SHIFT - 10)); + pr_info("Freeing %s memory: %ldK\n", s, K(pages)); return pages; } @@ -8180,14 +8179,13 @@ void __init mem_init_print_info(void) ", %luK highmem" #endif ")\n", - nr_free_pages() << (PAGE_SHIFT - 10), - physpages << (PAGE_SHIFT - 10), + K(nr_free_pages()), K(physpages), codesize >> 10, datasize >> 10, rosize >> 10, (init_data_size + init_code_size) >> 10, bss_size >> 10, - (physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT - 10), - totalcma_pages << (PAGE_SHIFT - 10) + K(physpages - totalram_pages() - totalcma_pages), + K(totalcma_pages) #ifdef CONFIG_HIGHMEM - , totalhigh_pages() << (PAGE_SHIFT - 10) + , K(totalhigh_pages()) #endif ); }
Use helper macro K() to convert the pages to the corresponding size. Minor readability improvement. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/page_alloc.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)