linux-kernel - Re: [PATCH v2 1/2] mm/hugetlb: create hstate_is_gigantic_no

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]
Message-Id: <20251009191149.57652-1-sj@kernel.org>
Date: Thu,  9 Oct 2025 12:11:49 -0700
From: SeongJae Park <sj@...nel.org>
To: Usama Arif <usamaarif642@...il.com>
Cc: SeongJae Park <sj@...nel.org>,
	muchun.song@...ux.dev,
	osalvador@...e.de,
	david@...hat.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	shakeel.butt@...ux.dev,
	linux-mm@...ck.org,
	hannes@...xchg.org,
	riel@...riel.com,
	kas@...nel.org,
	linux-kernel@...r.kernel.org,
	kernel-team@...a.com
Subject: Re: [PATCH v2 1/2] mm/hugetlb: create hstate_is_gigantic_no_runtime helper

Hi Usama,

On Thu,  9 Oct 2025 18:24:30 +0100 Usama Arif <usamaarif642@...il.com> wrote:

> This is a common condition used to skip operations that cannot
> be performed on gigantic pages when runtime support is disabled.
> This helper is introduced as the condition will exist even more
> when allowing "overcommit" of gigantic hugepages.
> No functional change intended with this patch.

The change looks good to me.  I have a couple of trivial comments below.

> 
> Suggested-by: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Usama Arif <usamaarif642@...il.com>
> ---

I think adding a change log since v1 here, or adding a cover letter with it
would be nice.

>  mm/hugetlb.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index c07b7192aff26..e74e41386b100 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -134,6 +134,17 @@ static void hugetlb_free_folio(struct folio *folio)
>  	folio_put(folio);
>  }
>  
> +/*
> + * Check if the hstate represents gigantic pages but gigantic page
> + * runtime support is not available. This is a common condition used to
> + * skip operations that cannot be performed on gigantic pages when runtime
> + * support is disabled.
> + */
> +static inline bool hstate_is_gigantic_no_runtime(struct hstate *h)
> +{
> +	return hstate_is_gigantic(h) && !gigantic_page_runtime_supported();
> +}
> +
>  static inline bool subpool_is_free(struct hugepage_subpool *spool)
>  {
>  	if (spool->count)
> @@ -1555,7 +1566,7 @@ static void remove_hugetlb_folio(struct hstate *h, struct folio *folio,
>  	VM_BUG_ON_FOLIO(hugetlb_cgroup_from_folio_rsvd(folio), folio);
>  
>  	lockdep_assert_held(&hugetlb_lock);
> -	if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
> +	if (hstate_is_gigantic_no_runtime(h))
>  		return;
>  
>  	list_del(&folio->lru);
> @@ -1617,7 +1628,7 @@ static void __update_and_free_hugetlb_folio(struct hstate *h,
>  {
>  	bool clear_flag = folio_test_hugetlb_vmemmap_optimized(folio);
>  
> -	if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
> +	if (hstate_is_gigantic_no_runtime(h))
>  		return;
>  
>  	/*
> @@ -2511,7 +2522,7 @@ static void return_unused_surplus_pages(struct hstate *h,
>  	/* Uncommit the reservation */
>  	h->resv_huge_pages -= unused_resv_pages;
>  
> -	if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
> +	if (hstate_is_gigantic_no_runtime(h))
>  		goto out;
>  
>  	/*
> @@ -3725,7 +3736,7 @@ static void __init hugetlb_init_hstates(void)
>  		 * - If CMA allocation is possible, we can not demote
>  		 *   HUGETLB_PAGE_ORDER or smaller size pages.
>  		 */
> -		if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
> +		if (hstate_is_gigantic_no_runtime(h))
>  			continue;
>  		if (hugetlb_cma_total_size() && h->order <= HUGETLB_PAGE_ORDER)
>  			continue;
> @@ -4202,7 +4213,7 @@ static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
>  	int err;
>  	nodemask_t nodes_allowed, *n_mask;
>  
> -	if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
> +	if (hstate_is_gigantic_no_runtime(h))
>  		return -EINVAL;
>  
>  	if (nid == NUMA_NO_NODE) {
> -- 
> 2.47.3

It seems the new helper could be used for three more cases.

On mm-new:

    $ git grep gigantic_page_runtime_supported mm/hugetlb.c
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:           if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (write && hstate_is_gigantic(h) && !gigantic_page_runtime_supported())

After applying this patch on top of mm-new:

    $ git grep gigantic_page_runtime_supported mm/hugetlb.c
    mm/hugetlb.c:   return hstate_is_gigantic(h) && !gigantic_page_runtime_supported();
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
    mm/hugetlb.c:   if (write && hstate_is_gigantic(h) && !gigantic_page_runtime_supported())

I'm curious if you are planning to do the conversion later, or there is a
reason why this patch is keeping those as is but I'm missing.


Thanks,
SJ