Re: [PATCH 1/7] mm: hugetlb: convert to alloc_fresh_hugetlb_hvo_folio()

[jane.chu@oracle.com]

On 8/5/2025 5:56 PM, Kefeng Wang wrote:
> 
> 
> On 2025/8/6 8:33, jane.chu@oracle.com wrote:
>>
>>
>> On 8/5/2025 3:18 PM, jane.chu@oracle.com wrote:
>>>
>>> On 8/2/2025 12:31 AM, Kefeng Wang wrote:
>>>> Now alloc_fresh_hugetlb_folio() is only called by
>>>> alloc_migrate_hugetlb_folio(), cleanup it by converting to
>>>> alloc_fresh_hugetlb_hvo_folio(), also simplify the
>>>> alloc_and_dissolve_hugetlb_folio() and alloc_surplus_hugetlb_folio()
>>>> too which help us to remove prep_new_hugetlb_folio() and
>>>> __prep_new_hugetlb_folio().
>>>>
>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>> ---
> 
> ...
> 
>>> Thanks for trying to clean up the clunky names and eliminate a couple 
>>> not really needed helpers.
>>>
>>> The decision to do HVO is baked in __hugetlb_vmemmap_optimize_folio(),
>>> ultimately all hugetlb pages go thru 
>>> __hugetlb_vmemmap_optimize_folio(), so let's not to add on 'hvo' in 
>>> function name to indicate additional decision.
>>>
>>> The difference between only_alloc_fresh_hugetlb_folio() and
>>> alloc_fresh_hugetlb_folio() is that the latter invokes 
>>> __hugetlb_vmemmap_optimize_folio(), while the former is used for huge
>>> folio allocation in batch, and then 
>>> __hugetlb_vmemmap_optimize_folio() is applied to a list of folios 
>>> back to back.
>>>
>>> Therefore, to hopefully make the names sound a bit more natural,
>>> how about
>>>    s/only_alloc_fresh_hugetlb_folio/alloc_hugetlb_folio_pre/
>>>    s/alloc_fresh_hugetlb_folio/alloc_hugetlb_folio/
>>> dropping "fresh", how is newly allocated folio not fresh?
>>
>> MY bad, didn't notice that alloc_hugetlb_folio() is already in use for 
>> allocating hugetlb folio from the reservation pool.
>>
>> So, how about just
>>     s/only_alloc_fresh_hugetlb_folio/alloc_fresh_hugetlb_folio_pre/
>> while keep alloc_fresh_hugetlb_folio() as is?
>>
> 
> 
> Hi Oscar/Jane, thanks for your comments, what about
> 
> s/only_alloc_fresh_hugetlb_folio/__alloc_fresh_hugetlb_folio/
> alloc_fresh_hugetlb_folio
> 
> 

IMHO, the __alloc_fresh_hugetlb_folio naming style typically means it's 
the function that completes the real deal, while in comparison, the 
alloc_fresh_hugetlb_folio name sounds more like a front end. So that's 
not what we're having here.

That said, it's just naming, the patch itself is great.
I'll defer to Oscar.

thanks,
-jane

> Thanks.