linux-kernel - Re: [PATCH 1/6] mm/hugetlb: fix incorrect update of max_huge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 Aug 2022 15:52:47 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     akpm@...ux-foundation.org, songmuchun@...edance.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] mm/hugetlb: fix incorrect update of max_huge_pages

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@...wei.com>
> ---
>  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
>