Re: [PATCH v2 1/2] mm/hugetlb: create hstate_is_gigantic_no_runtime helper

[Usama Arif <usamaarif642@gmail.com>]

On 09/10/2025 20:11, SeongJae Park wrote:
> 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.
> 

I thought everything needed for the change is there in the commit message for patch 2,
and the first patch was trivial, so didnt add a cover letter. The change from v1 was
trivial as well.

>>  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.
> 

Yeah what you said in the followup email. I think should be ok now as Andrew has added v2
to mm-new.

Thanks!
Usama
> 
> Thanks,
> SJ