Re: [PATCH 1/6] mm/hugetlb: fix incorrect update of max_huge_pages

[Mike Kravetz]

On 08/16/22 21:05, Miaohe Lin wrote:
> There should be pages_per_huge_page(h) / pages_per_huge_page(target_hstate)
> pages incremented for target_hstate->max_huge_pages when page is demoted.
> Update max_huge_pages accordingly for consistency.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> ---
>  mm/hugetlb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index ea1c7bfa1cc3..e72052964fb5 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3472,7 +3472,8 @@ static int demote_free_huge_page(struct hstate *h, struct page *page)
>  	 * based on pool changes for the demoted page.
>  	 */
>  	h->max_huge_pages--;
> -	target_hstate->max_huge_pages += pages_per_huge_page(h);
> +	target_hstate->max_huge_pages +=
> +		pages_per_huge_page(h) / pages_per_huge_page(target_hstate);

Thanks!

That is indeed incorrect.  However the miscalculation should not have any 
consequences.  Correct?  The value is used when initially populating the
pools.  It is never read and used again.  It is written to in
set_max_huge_pages if someone changes the number of hugetlb pages.

I guess that is a long way of saying I am not sure why we care about trying
to keep max_huge_pages up to date?  I do not think it matters.

I also thought, if we are going to adjust max_huge_pages here we may
also want to adjust the node specific value: h->max_huge_pages_node[node].
There are a few other places where the global max_huge_pages is adjusted
without adjusting the node specific value.

The more I think about it, the more I think we should explore just
eliminating any adjustment of this/these values after initially
populating the pools.
-- 
Mike Kravetz

>  
>  	return rc;
>  }
> -- 
> 2.23.0
>