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
