> -----Original Message----- > From: Mike Rapoport [mailto:rppt@xxxxxxxxxx] > Sent: Tuesday, July 7, 2020 7:38 PM > To: Roman Gushchin <guro@xxxxxx> > Cc: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>; > akpm@xxxxxxxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; Linuxarm <linuxarm@xxxxxxxxxx>; Mike > Kravetz <mike.kravetz@xxxxxxxxxx>; Jonathan Cameron > <jonathan.cameron@xxxxxxxxxx> > 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@xxxxxx> > > > Cc: Mike Kravetz <mike.kravetz@xxxxxxxxxx> > > > Cc: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx> > > > Signed-off-by: Barry Song <song.bao.hua@xxxxxxxxxxxxx> > > > --- > > > -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@xxxxxx> after that. > > > > Thank you! > > > > -- > Sincerely yours, > Mike. Thanks Barry
