[bug report] hugetlbfs: extend the definition of hugepages parameter to support node allocation

[bug report] hugetlbfs: extend the definition of hugepages parameter to support node allocation

[Dan Carpenter]

Hello Zhenguo Yao,

The patch 67b2b441dc10: "hugetlbfs: extend the definition of
hugepages parameter to support node allocation" from Sep 29, 2021,
leads to the following
Smatch static checker warning:

	mm/hugetlb.c:2957 __alloc_bootmem_huge_page()
	warn: ignoring unreachable code.

mm/hugetlb.c
    2940 int __alloc_bootmem_huge_page(struct hstate *h, int nid)
    2941 {
    2942         struct huge_bootmem_page *m;
    2943         int nr_nodes, node;
    2944 
    2945         if (nid >= nr_online_nodes)
    2946                 return 0;
    2947         /* do node specific alloc */
    2948         if (nid != NUMA_NO_NODE) {
    2949                 m = memblock_alloc_try_nid_raw(huge_page_size(h), huge_page_size(h),
    2950                                 0, MEMBLOCK_ALLOC_ACCESSIBLE, nid);
    2951                 if (m)
    2952                         goto found;
    2953                 else
    2954                         return 0;
    2955         }
    2956         /* do all node balanced alloc */
                       ^^^^^^^^
This says that it's going to loop over "all nodes".

--> 2957         for_each_node_mask_to_alloc(h, nr_nodes, node, &node_states[N_MEMORY]) {
    2958                 m = memblock_alloc_try_nid_raw(
    2959                                 huge_page_size(h), huge_page_size(h),
    2960                                 0, MEMBLOCK_ALLOC_ACCESSIBLE, node);
    2961                 /*
    2962                  * Use the beginning of the huge page to store the
    2963                  * huge_bootmem_page struct (until gather_bootmem
    2964                  * puts them into the mem_map).
    2965                  */
    2966                 if (m)
    2967                         goto found;
    2968                 else
    2969                         return 0;

But it only checks the first node and then returns.

    2970         }
    2971 
    2972 found:
    2973         /* Put them into a private list first because mem_map is not up yet */
    2974         INIT_LIST_HEAD(&m->list);
    2975         list_add(&m->list, &huge_boot_pages);
    2976         m->hstate = h;
    2977         return 1;
    2978 }

regards,
dan carpenter

Re: [bug report] hugetlbfs: extend the definition of hugepages parameter to support node allocation

[Zhenguo Yao]

Thanks for your report,  I have already updated this patch with the fix.
Link: https://lore.kernel.org/lkml/20211005054729.86457-1-yaozhenguo1@gmail.com/

Dan Carpenter <dan.carpenter@oracle.com> 于2021年10月1日周五 下午9:23写道：
>
> Hello Zhenguo Yao,
>
> The patch 67b2b441dc10: "hugetlbfs: extend the definition of
> hugepages parameter to support node allocation" from Sep 29, 2021,
> leads to the following
> Smatch static checker warning:
>
>         mm/hugetlb.c:2957 __alloc_bootmem_huge_page()
>         warn: ignoring unreachable code.
>
> mm/hugetlb.c
>     2940 int __alloc_bootmem_huge_page(struct hstate *h, int nid)
>     2941 {
>     2942         struct huge_bootmem_page *m;
>     2943         int nr_nodes, node;
>     2944
>     2945         if (nid >= nr_online_nodes)
>     2946                 return 0;
>     2947         /* do node specific alloc */
>     2948         if (nid != NUMA_NO_NODE) {
>     2949                 m = memblock_alloc_try_nid_raw(huge_page_size(h), huge_page_size(h),
>     2950                                 0, MEMBLOCK_ALLOC_ACCESSIBLE, nid);
>     2951                 if (m)
>     2952                         goto found;
>     2953                 else
>     2954                         return 0;
>     2955         }
>     2956         /* do all node balanced alloc */
>                        ^^^^^^^^
> This says that it's going to loop over "all nodes".
>
> --> 2957         for_each_node_mask_to_alloc(h, nr_nodes, node, &node_states[N_MEMORY]) {
>     2958                 m = memblock_alloc_try_nid_raw(
>     2959                                 huge_page_size(h), huge_page_size(h),
>     2960                                 0, MEMBLOCK_ALLOC_ACCESSIBLE, node);
>     2961                 /*
>     2962                  * Use the beginning of the huge page to store the
>     2963                  * huge_bootmem_page struct (until gather_bootmem
>     2964                  * puts them into the mem_map).
>     2965                  */
>     2966                 if (m)
>     2967                         goto found;
>     2968                 else
>     2969                         return 0;
>
> But it only checks the first node and then returns.
>
>     2970         }
>     2971
>     2972 found:
>     2973         /* Put them into a private list first because mem_map is not up yet */
>     2974         INIT_LIST_HEAD(&m->list);
>     2975         list_add(&m->list, &huge_boot_pages);
>     2976         m->hstate = h;
>     2977         return 1;
>     2978 }
>
> regards,
> dan carpenter