Re: [PATCH mm-unstable v18 11/14] mm/khugepaged: Introduce mTHP collapse support

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/3/26 14:16, Nico Pache wrote:
> On Wed, Jun 3, 2026 at 4:01 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>>
>>
>>>  next_order:
>>> -             if ((BIT(order) - 1) & enabled_orders) {
>>> -                     const u8 next_order = order - 1;
>>> -                     const u16 mid_offset = offset + (nr_ptes / 2);
>>> -
>>> -                     collapse_mthp_stack_push(cc, &stack_size, mid_offset,
>>> -                                              next_order);
>>> -                     collapse_mthp_stack_push(cc, &stack_size, offset,
>>> -                                              next_order);
>>> +             if (order > KHUGEPAGED_MIN_MTHP_ORDER &&
>>> +                     (BIT(order) - 1) & enabled_orders) {
>>
>> Why not a test_bit() ?
> 
> The test bit is at the top of the loop. This adds a exit if the lower
> orders are all disabled or we hit the last order.

Ah, now I understand what you want to do here.

I guess you should add a () around the & and maybe add a comment.

And likely using GENMASK is clearer?

/*
 * Continue with the next smaller order if there is still
 * any smaller order enabled.
 */
if (order > KHUGEPAGED_MIN_MTHP_ORDER &&
    (enabled_orders & GENMASK(order - 1, 0))) {
	...
}


> 
>>
>>
>> But, wouldn't you want to skip orders that are not enabled and try with the next
>> smaller one in any case before you advance the offset?
> 
> We are currently iterating through each order (not skipping them).
> There may be optimizations to avoid iterating through every order
> (like your changes suggest), but currently, every collapse, whether it
> succeeds or fails at the bottom order, must also iterate the offset.

Right, we currently miss opportunities to just skip, that we can optimize later.


-- 
Cheers,

David