* [PATCH v2] mm/hugetlb: avoid hardcoding while checking if cma is enable
@ 2020-07-07 3:11 Barry Song
2020-07-07 3:36 ` Roman Gushchin
0 siblings, 1 reply; 4+ messages in thread
From: Barry Song @ 2020-07-07 3:11 UTC (permalink / raw)
To: akpm
Cc: linux-mm, linux-kernel, linuxarm, Barry Song, Roman Gushchin,
Mike Kravetz, Jonathan Cameron
hugetlb_cma[0] can be NULL due to various reasons, for example, node0 has
no memory. so NULL hugetlb_cma[0] doesn't necessarily mean cma is not
enabled. gigantic pages might have been reserved on other nodes.
Fixes: cf11e85fc08c ("mm: hugetlb: optionally allocate gigantic hugepages using cma")
Cc: Roman Gushchin <guro@fb.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
-v2: add hugetlb_cma_enabled() helper to improve readability according to Roman
mm/hugetlb.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 57ece74e3aae..d5e98ed86bb9 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2546,6 +2546,20 @@ static void __init gather_bootmem_prealloc(void)
}
}
+bool __init hugetlb_cma_enabled(void)
+{
+ if (IS_ENABLED(CONFIG_CMA)) {
+ int node;
+
+ for_each_online_node(node) {
+ if (hugetlb_cma[node])
+ return true;
+ }
+ }
+
+ return false;
+}
+
static void __init hugetlb_hstate_alloc_pages(struct hstate *h)
{
unsigned long i;
@@ -2571,7 +2585,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h)
for (i = 0; i < h->max_huge_pages; ++i) {
if (hstate_is_gigantic(h)) {
- if (IS_ENABLED(CONFIG_CMA) && hugetlb_cma[0]) {
+ if (hugetlb_cma_enabled()) {
pr_warn_once("HugeTLB: hugetlb_cma is enabled, skip boot time allocation\n");
break;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] mm/hugetlb: avoid hardcoding while checking if cma is enable
2020-07-07 3:11 [PATCH v2] mm/hugetlb: avoid hardcoding while checking if cma is enable Barry Song
@ 2020-07-07 3:36 ` Roman Gushchin
2020-07-07 7:37 ` Mike Rapoport
0 siblings, 1 reply; 4+ messages in thread
From: Roman Gushchin @ 2020-07-07 3:36 UTC (permalink / raw)
To: Barry Song
Cc: akpm, linux-mm, linux-kernel, linuxarm, Mike Kravetz, Jonathan Cameron
On Tue, Jul 07, 2020 at 03:11:56PM +1200, Barry Song wrote:
> hugetlb_cma[0] can be NULL due to various reasons, for example, node0 has
> no memory. so NULL hugetlb_cma[0] doesn't necessarily mean cma is not
> enabled. gigantic pages might have been reserved on other nodes.
>
> Fixes: cf11e85fc08c ("mm: hugetlb: optionally allocate gigantic hugepages using cma")
> Cc: Roman Gushchin <guro@fb.com>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> ---
> -v2: add hugetlb_cma_enabled() helper to improve readability according to Roman
>
> mm/hugetlb.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 57ece74e3aae..d5e98ed86bb9 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2546,6 +2546,20 @@ static void __init gather_bootmem_prealloc(void)
> }
> }
>
> +bool __init hugetlb_cma_enabled(void)
> +{
> + if (IS_ENABLED(CONFIG_CMA)) {
> + int node;
> +
> + for_each_online_node(node) {
> + if (hugetlb_cma[node])
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
Can you, please, change it to a more canonical
#ifdef CONFIG_CMA
bool __init hugetlb_cma_enabled(void)
{
int node;
for_each_online_node(node)
if (hugetlb_cma[node])
return true;
return false;
}
#else
bool __init hugetlb_cma_enabled(void)
{
return false;
}
#endif
or maybe just
bool __init hugetlb_cma_enabled(void)
{
#ifdef CONFIG_CMA
int node;
for_each_online_node(node)
if (hugetlb_cma[node])
return true;
#endif
return false;
}
?
Please, feel free to add
Acked-by: Roman Gushchin <guro@fb.com> after that.
Thank you!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] mm/hugetlb: avoid hardcoding while checking if cma is enable
2020-07-07 3:36 ` Roman Gushchin
@ 2020-07-07 7:37 ` Mike Rapoport
2020-07-07 8:01 ` Song Bao Hua (Barry Song)
0 siblings, 1 reply; 4+ messages in thread
From: Mike Rapoport @ 2020-07-07 7:37 UTC (permalink / raw)
To: Roman Gushchin
Cc: Barry Song, akpm, linux-mm, linux-kernel, linuxarm, Mike Kravetz,
Jonathan Cameron
On Mon, Jul 06, 2020 at 08:36:31PM -0700, Roman Gushchin wrote:
> On Tue, Jul 07, 2020 at 03:11:56PM +1200, Barry Song wrote:
> > hugetlb_cma[0] can be NULL due to various reasons, for example, node0 has
> > no memory. so NULL hugetlb_cma[0] doesn't necessarily mean cma is not
> > enabled. gigantic pages might have been reserved on other nodes.
> >
> > Fixes: cf11e85fc08c ("mm: hugetlb: optionally allocate gigantic hugepages using cma")
> > Cc: Roman Gushchin <guro@fb.com>
> > Cc: Mike Kravetz <mike.kravetz@oracle.com>
> > Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> > ---
> > -v2: add hugetlb_cma_enabled() helper to improve readability according to Roman
> >
> > mm/hugetlb.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 57ece74e3aae..d5e98ed86bb9 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -2546,6 +2546,20 @@ static void __init gather_bootmem_prealloc(void)
> > }
> > }
> >
> > +bool __init hugetlb_cma_enabled(void)
> > +{
> > + if (IS_ENABLED(CONFIG_CMA)) {
> > + int node;
> > +
> > + for_each_online_node(node) {
> > + if (hugetlb_cma[node])
> > + return true;
> > + }
> > + }
> > +
> > + return false;
> > +}
> > +
>
> Can you, please, change it to a more canonical
>
> #ifdef CONFIG_CMA
> bool __init hugetlb_cma_enabled(void)
> {
> int node;
>
> for_each_online_node(node)
> if (hugetlb_cma[node])
> return true;
>
> return false;
> }
> #else
> bool __init hugetlb_cma_enabled(void)
> {
> return false;
> }
> #endif
>
> or maybe just
>
> bool __init hugetlb_cma_enabled(void)
> {
> #ifdef CONFIG_CMA
> int node;
>
> for_each_online_node(node)
> if (hugetlb_cma[node])
> return true;
> #endif
> return false;
> }
This one please.
> ?
>
> Please, feel free to add
> Acked-by: Roman Gushchin <guro@fb.com> after that.
>
> Thank you!
>
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] mm/hugetlb: avoid hardcoding while checking if cma is enable
2020-07-07 7:37 ` Mike Rapoport
@ 2020-07-07 8:01 ` Song Bao Hua (Barry Song)
0 siblings, 0 replies; 4+ messages in thread
From: Song Bao Hua (Barry Song) @ 2020-07-07 8:01 UTC (permalink / raw)
To: Mike Rapoport, Roman Gushchin
Cc: akpm, linux-mm, linux-kernel, Linuxarm, Mike Kravetz, Jonathan Cameron
> -----Original Message-----
> From: Mike Rapoport [mailto:rppt@kernel.org]
> Sent: Tuesday, July 7, 2020 7:38 PM
> To: Roman Gushchin <guro@fb.com>
> Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>;
> akpm@linux-foundation.org; linux-mm@kvack.org;
> linux-kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>; Mike
> Kravetz <mike.kravetz@oracle.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: Re: [PATCH v2] mm/hugetlb: avoid hardcoding while checking if cma
> is enable
>
> On Mon, Jul 06, 2020 at 08:36:31PM -0700, Roman Gushchin wrote:
> > On Tue, Jul 07, 2020 at 03:11:56PM +1200, Barry Song wrote:
> > > hugetlb_cma[0] can be NULL due to various reasons, for example, node0
> has
> > > no memory. so NULL hugetlb_cma[0] doesn't necessarily mean cma is not
> > > enabled. gigantic pages might have been reserved on other nodes.
> > >
> > > Fixes: cf11e85fc08c ("mm: hugetlb: optionally allocate gigantic hugepages
> using cma")
> > > Cc: Roman Gushchin <guro@fb.com>
> > > Cc: Mike Kravetz <mike.kravetz@oracle.com>
> > > Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
> > > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> > > ---
> > > -v2: add hugetlb_cma_enabled() helper to improve readability according
> to Roman
> > >
> > > mm/hugetlb.c | 16 +++++++++++++++-
> > > 1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > > index 57ece74e3aae..d5e98ed86bb9 100644
> > > --- a/mm/hugetlb.c
> > > +++ b/mm/hugetlb.c
> > > @@ -2546,6 +2546,20 @@ static void __init
> gather_bootmem_prealloc(void)
> > > }
> > > }
> > >
> > > +bool __init hugetlb_cma_enabled(void)
> > > +{
> > > + if (IS_ENABLED(CONFIG_CMA)) {
> > > + int node;
> > > +
> > > + for_each_online_node(node) {
> > > + if (hugetlb_cma[node])
> > > + return true;
> > > + }
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> >
> > Can you, please, change it to a more canonical
> >
> > #ifdef CONFIG_CMA
> > bool __init hugetlb_cma_enabled(void)
> > {
> > int node;
> >
> > for_each_online_node(node)
> > if (hugetlb_cma[node])
> > return true;
> >
> > return false;
> > }
> > #else
> > bool __init hugetlb_cma_enabled(void)
> > {
> > return false;
> > }
> > #endif
> >
> > or maybe just
> >
> > bool __init hugetlb_cma_enabled(void)
> > {
> > #ifdef CONFIG_CMA
> > int node;
> >
> > for_each_online_node(node)
> > if (hugetlb_cma[node])
> > return true;
> > #endif
> > return false;
> > }
>
> This one please.
Yep. Patch v3 is just the one you prefer:
https://marc.info/?l=linux-mm&m=159409465228477&w=2
>
> > ?
> >
> > Please, feel free to add
> > Acked-by: Roman Gushchin <guro@fb.com> after that.
> >
> > Thank you!
> >
>
> --
> Sincerely yours,
> Mike.
Thanks
Barry
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-07 8:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 3:11 [PATCH v2] mm/hugetlb: avoid hardcoding while checking if cma is enable Barry Song
2020-07-07 3:36 ` Roman Gushchin
2020-07-07 7:37 ` Mike Rapoport
2020-07-07 8:01 ` Song Bao Hua (Barry Song)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.