public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Usama Arif <usamaarif642@gmail.com>
Cc: SeongJae Park <sj@kernel.org>,
	muchun.song@linux.dev, osalvador@suse.de, david@redhat.com,
	Andrew Morton <akpm@linux-foundation.org>,
	shakeel.butt@linux.dev, linux-mm@kvack.org, hannes@cmpxchg.org,
	riel@surriel.com, kas@kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@meta.com
Subject: Re: [PATCH v2 1/2] mm/hugetlb: create hstate_is_gigantic_no_runtime helper
Date: Thu,  9 Oct 2025 12:11:49 -0700	[thread overview]
Message-ID: <20251009191149.57652-1-sj@kernel.org> (raw)
In-Reply-To: <20251009172433.4158118-1-usamaarif642@gmail.com>

Hi Usama,

On Thu,  9 Oct 2025 18:24:30 +0100 Usama Arif <usamaarif642@gmail.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@linux-foundation.org>
> Signed-off-by: Usama Arif <usamaarif642@gmail.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

  parent reply	other threads:[~2025-10-09 19:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-09 17:24 [PATCH v2 1/2] mm/hugetlb: create hstate_is_gigantic_no_runtime helper Usama Arif
2025-10-09 17:24 ` [PATCH v2 2/2] mm/hugetlb: allow overcommitting gigantic hugepages Usama Arif
2025-10-10  0:32   ` Shakeel Butt
2025-10-13  8:00   ` Oscar Salvador
2025-10-13 12:56     ` Kefeng Wang
2025-10-09 19:11 ` SeongJae Park [this message]
2025-10-09 19:24   ` [PATCH v2 1/2] mm/hugetlb: create hstate_is_gigantic_no_runtime helper SeongJae Park
2025-10-10 11:53   ` Usama Arif
2025-10-10  0:31 ` Shakeel Butt
2025-10-13  7:56 ` Oscar Salvador
2025-10-13  8:04 ` David Hildenbrand
2025-10-13 12:49 ` Kefeng Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251009191149.57652-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=kas@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=muchun.song@linux.dev \
    --cc=osalvador@suse.de \
    --cc=riel@surriel.com \
    --cc=shakeel.butt@linux.dev \
    --cc=usamaarif642@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox