Messages in this thread | | | Subject | Re: [PATCH] mm/hugetlb: suppress wrong warning info when alloc gigantic page | From | Mike Kravetz <> | Date | Fri, 19 Feb 2021 11:14:00 -0800 |
| |
On 2/19/21 4:39 AM, Chen Wandun wrote: > If hugetlb_cma is enabled, it will skip boot time allocation > when allocating gigantic page, that doesn't means allocation > failure, so suppress this warning info. >
Normally the addition of warning messages is discouraged. However, in this case the additional message provides value. Why?
Prior to the commit cf11e85fc08c, one could have a kernel command line that contains:
hugepagesz=1G hugepages=16
This would allocate 16 1G pages at boot time.
After the commit, someone could specify a command line containing:
hugepagesz=1G hugepages=16 hugetlb_cma=16G
In this case, 16G of CMA will be reserved for 1G huge page allocations after boot time. The parameter 'hugepages=16' is ignored, and the warning message is logged. The warning message should only be logged when the kernel parameter 'hugepages=' is ignored.
IMO, it make sense to log a warning if ignoring a user specified parameter. The user should not be attempting boot time allocation and CMA reservation for 1G pages.
I do not think we should drop the warning as the it tells the user thay have specified two incompatible allocation options. -- Mike Kravetz
> Fixes: cf11e85fc08c ("mm: hugetlb: optionally allocate gigantic hugepages using cma") > Signed-off-by: Chen Wandun <chenwandun@huawei.com> > --- > mm/hugetlb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index b6992297aa16..98a49cb9250c 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -2465,7 +2465,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h) > if (hstate_is_gigantic(h)) { > if (hugetlb_cma_size) { > pr_warn_once("HugeTLB: hugetlb_cma is enabled, skip boot time allocation\n"); > - break; > + goto free; > } > if (!alloc_bootmem_huge_page(h)) > break; > @@ -2483,7 +2483,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h) > h->max_huge_pages, buf, i); > h->max_huge_pages = i; > } > - > +free: > kfree(node_alloc_noretry); > } > >
| |