Re: [PATCH mm-unstable v18 10/14] mm/khugepaged: introduce collapse_allowable_orders helper function

[David Hildenbrand (Arm)]

On 6/1/26 16:35, Lorenzo Stoakes wrote:
> On Sun, May 31, 2026 at 10:18:16PM +0200, David Hildenbrand (Arm) wrote:
>> On 5/22/26 17:00, Nico Pache wrote:
>>> Add collapse_allowable_orders() to generalize THP order eligibility. The
>>> function determines which THP orders are permitted based on collapse
>>> context (khugepaged vs madv_collapse).
>>>
>>> This consolidates collapse configuration logic and provides a clean
>>> interface for future mTHP collapse support where the orders may be
>>> different.
>>
>> It would have been good to describe here that, for now, it only ever returns
>> PMDs, and that it will be extended next.
>>
>> Logically, this patch belongs to #12, not #11 ... so seeing it before #11 was a bit
>>
>> ... and there, it is clear that we don't even want to know the orders?
>>
>> So can we just call this function
>>
>> "collapse_possible" and make it return a boolean?

FWIW, I realized later that #11 has

enabled_orders = collapse_allowable_orders(vma, vma->vm_flags, tva_flags);

and forgot to delete that comment.

So we must have a variant (for #11) that returns the enabled orders.

> 
> Yeah agreed.
> 
> But I also don't love the naming, we now have thp_vma_allowable_orders(),
> __thp_vma_allowable_orders(), and then collapse_allowable_orders() and we also
> have 3 different ways of collapsing, one of which we call MADV_... COLLAPSE,
> and the other khugepaged + fault-in too for laughs.
> 
> It's like a big circle of confusion.
> 
> And of course we call THP collapse 'collapse' in general, so :)
> 
> Anyway I'm fine with collapse_possible() so we can move on and then maybe
> cleanup later.

We could simply have

collapse_possible_orders()

and

collapse_possible()

the latter being a simple wrapper around collapse_possible_orders().

> 
> Also - if I look at khugepaged.c I see thp_vma_allowable_orders() still used in
> hugepage_vma_revalidate().
> 
> Wouldn't it be better then to do the abstraction once mTHP order checking is
> properly introduced and change this also and have _every_ order check be
> consistent in khugepaged.c?

That'd also be nice, if easily possible.

-- 
Cheers,

David