Re: [PATCH] hugetlb: Fix hugepages_setup when deal with pernode

["liupeng (DM)" <liupeng256@xxxxxxxxxx>]

On 2022/3/25 5:57, Mike Kravetz wrote:

> On 3/24/22 00:40, Peng Liu wrote:
> > Hugepages can be specified to pernode since "hugetlbfs: extend
> > the definition of hugepages parameter to support node allocation",
> > but the following two problems are observed.
> >
> > 1) Confusing behavior is observed when both 1G and 2M hugepage
> > is set after "numa=off".
> >  cmdline hugepage settings:
> >   hugepagesz=1G hugepages=0:3,1:3
> >   hugepagesz=2M hugepages=0:1024,1:1024
> >  results:
> >   HugeTLB registered 1.00 GiB page size, pre-allocated 0 pages
> >   HugeTLB registered 2.00 MiB page size, pre-allocated 1024 pages
> >
> > 2) Using invalid option values causes the entire kernel boot option
> > string to be reported as Unknown.
> >  Unknown kernel command line parameters "hugepages=0:1024,1:1024"
> >       
> Thank you for debugging and sending the patch!
>
> My first thought was "If someone is specifying 'numa=off' as well as
> numa node specific allocations on the same command line, we should just
> fail the allocation request".  However, this same situation could exist
> without the 'numa=off' option as long as an invalid node is included in
> the list.
>     
    We will "specifying 'numa=off' as well as
      numa node specific allocations"
    for some debugging and test cases. If the
      original command line can be
     partly effective, this will be convenient.
      Yet, we also test "an invalid
     node is included in the list", the behavior
      is the same with "numa=off".
    
> With your patch, the node specific allocations are parsed (and processed)
> until there is an error.  So, in the example above 3 1G pages and 1024 2M
> pages are allocated on node 0.  That seems correct.
>
> Now suppose the node specific allocations are specified as:
> hugepagesz=1G hugepages=1:3,0:3
> hugepagesz=2M hugepages=1:1024,0:1024
>     
    For this case, with/without this patch, huge
      page will be not allocated
    on any node.
> Since node 1 is invalid, we experience an error here and do not allocate
> any pages on node 0.
>
> I am wondering if we should just error and ignore the entire string if
> ANY of the specified nodes are invalid?  Thoughts?
>

Thank you for your response.

    This patch only to be consistent between
      2M/1G behavior, and repair "return 0"
    as 1d02b444b8d1 ("tracing: Fix return value
      of __setup handlers").
    With this patch, a node could allocate huge
      pages until there is an error, and it
     will print the invalid parameter from the
      first parse error. So, I think this 
    is acceptable.

The following tests is added with this patch.

    Case1:
    settings:
    hugepagesz=1G hugepages=0:3,1:3,2:3
    hugepagesz=2M hugepages=0:1024,1:1024,3:1024
    
      results:
      HugeTLB: Invalid hugepages parameter 1:3,2:3
      HugeTLB: Invalid hugepages parameter 1:1024,3:1024
      HugeTLB registered 1.00 GiB page size, pre-allocated 3 pages
    HugeTLB registered 2.00 MiB page size,
      pre-allocated 1024 pages

Case2:

    settings:
    hugepagesz=1G hugepages=1:3,0:3 
    hugepagesz=2M hugepages=1:1024,0:1024
    
    results:
      HugeTLB: Invalid hugepages parameter 1:3,0:3
      HugeTLB: Invalid hugepages parameter 1:1024,0:1024
      HugeTLB registered 1.00 GiB page size, pre-allocated 0 pages
      HugeTLB registered 2.00 MiB page size, pre-allocated 0 pages

Peng Liu.

>