Re: [PATCH v2 3/5] mm: hugetlb: track CMA reservations by pool

[Muchun Song]

> On Oct 23, 2026, at 14:35, Qi Zheng <qi.zheng@xxxxxxxxx> wrote:
> 
> Hi Muchun,
> 
> On 10/22/26 7:21 PM, Muchun Song wrote:
>> On 2026/10/22 15:31, Qi Zheng wrote:
>>> From: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
>>> 
>>> Introduce fixed default and PMD pool slots in preparation for PMD-sized
>>> HugeTLB CMA support. Continue accepting only the architecture's existing
>>> CMA page size and keep all gigantic sizes sharing the default pool.
>>> PMD-sized pool support will be added in a subsequent patch.
>>> 
>>> Select the pool for allocation, CMA-only checks and boot-time accounting.
>>> Release folios through the CMA area that owns their physical memory,
>>> including after demotion.
>>> 
>>> Record each pool's successfully reserved capacity and remove
>>> hugetlb_cma_validate_params(), since the per-pool CMA-only check handles
>>> missing or empty reservations.
>>> 
>>> Assisted-by: LLM
>>> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
>>> ---
>>>   mm/hugetlb.c     |   6 +-
>>>   mm/hugetlb_cma.c | 154 ++++++++++++++++++++++++++++++++---------------
>>>   mm/hugetlb_cma.h |  11 ++--
>>>   3 files changed, 112 insertions(+), 59 deletions(-)
>>> 
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index b37d7d0db7fc9..6b08074d11f52 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -1368,7 +1368,7 @@ static struct folio *alloc_gigantic_frozen_folio(int order, gfp_t gfp_mask,
>>>       if (folio)
>>>           return folio;
>>> -    if (hugetlb_cma_exclusive_alloc())
>>> +    if (hugetlb_cma_exclusive_alloc(order))
>>>           return NULL;
>>>       folio = (struct folio *)alloc_contig_frozen_pages(1 << order, gfp_mask,
>>> @@ -3635,7 +3635,7 @@ static void __init hugetlb_hstate_alloc_pages(struct hstate *h)
>>>        * Skip gigantic hugepages allocation if early CMA
>>>        * reservations are not available.
>>>        */
>>> -    if (hstate_is_gigantic(h) && hugetlb_cma_total_size() &&
>>> +    if (hstate_is_gigantic(h) && hugetlb_cma_size(h) &&
>>>           !hugetlb_early_cma(h)) {
>>>           pr_warn_once("hugetlb_cma is enabled, skip boot time allocation\n");
>>>           return;
>>> @@ -4338,8 +4338,6 @@ static __init void hugetlb_parse_params(void)
>>>           hcp->setup(hcp->val);
>>>       }
>>> -
>>> -    hugetlb_cma_validate_params();
>>>   }
>>>   /*
>>> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
>>> index 4f27eb242dc3b..c279c3f548705 100644
>>> --- a/mm/hugetlb_cma.c
>>> +++ b/mm/hugetlb_cma.c
>>> @@ -19,6 +19,7 @@
>>>   struct hugetlb_cma_pool {
>>>       struct cma *cma[MAX_NUMNODES];
>>>       unsigned long size;
>>> +    unsigned int order;
>>>   };
>>>   struct hugetlb_cma_config {
>>> @@ -27,14 +28,34 @@ struct hugetlb_cma_config {
>>>       unsigned int percent_in_node[MAX_NUMNODES];
>>>   };
>>> -static struct hugetlb_cma_pool hugetlb_cma __ro_after_init;
>>> -static struct hugetlb_cma_config hugetlb_cma_config __initdata;
>>> +enum {
>>> +    HUGETLB_CMA_DEFAULT,
>>> +    HUGETLB_CMA_PMD,
>>> +    HUGETLB_CMA_POOLS,
>>> +};
>> Since there are exactly two pool roles, could we make that explicit with
>> a typed enum, for example:
>>       enum hugetlb_cma_pool_id {
>>             HUGETLB_CMA_POOL_DEFAULT,
>>             HUGETLB_CMA_POOL_PMD,
>>             HUGETLB_CMA_NR_POOLS,
>>       };
>> and name the array hugetlb_cma_pools[]?
> 
> Make sense, will do.
> 
>>> +
>>> +static struct hugetlb_cma_pool hugetlb_cma[HUGETLB_CMA_POOLS] __ro_after_init;
>>> +static struct hugetlb_cma_config hugetlb_cma_config[HUGETLB_CMA_POOLS] __initdata;
>>>   static bool hugetlb_cma_only __ro_after_init;
>>>   static char hugetlb_cma_cmdline[COMMAND_LINE_SIZE] __initdata;
>>>   static unsigned int hugetlb_cma_cmdline_len __initdata;
>>> +static struct hugetlb_cma_pool *hugetlb_cma_pool(unsigned int order)
>> How about order_to_cma_pool()?
> 
> will do.
> 
>>> +{
>>> +    struct hugetlb_cma_pool *pmd_pool = &hugetlb_cma[HUGETLB_CMA_PMD];
>>> +
>>> +    if (order == PMD_SHIFT - PAGE_SHIFT && pmd_pool->size)
>> PMD_ORDER is better.
> 
> OK.
> 
>> There's another question here: why does PMD-based page allocation
>> need to check the pool->size, but gigantic pages don't?
> 
> The PMD size pool needs to be checked here. The check for gigantic pool,
> is deferred to the caller.
> 
>>> +        return pmd_pool;
>>> +
>>> +    /* Preserve the default pool's use by gigantic sizes. */
>>> +    if (order_is_gigantic(order))
>>> +        return &hugetlb_cma[HUGETLB_CMA_DEFAULT];
>> If we also check size for gigantic pages here, would the code be simpler
>> that way for callers?
> 
> Yes, will do.
> 
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>>   #ifdef CONFIG_NUMA
>>>   static phys_addr_t __init memblock_node_memory_size(int nid)
>>>   {
>>> @@ -56,10 +77,16 @@ static phys_addr_t __init memblock_node_memory_size(int nid)
>>>   void hugetlb_cma_free_frozen_folio(struct folio *folio)
>>>   {
>>> -    struct hugetlb_cma_pool *pool = &hugetlb_cma;
>>> +    unsigned int i;
>>> +    int nid = folio_nid(folio);
>>> +    unsigned long nr_pages = folio_nr_pages(folio);
>>> +
>> Please add a short comment here explaining why all pools must be searched:
>> after demotion, the folio order does not identify the CMA pool that originally
>> owns the physical range.
> 
> How about the following comment:
> 
>     /*
>      * Demotion can change the folio order, so search all pools for the
>      * CMA area that owns the physical range.
>      */

LGTM.

Thanks,
Muchun