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

On 6/3/26 14:16, Nico Pache wrote:

On Wed, Jun 3, 2026 at 4:01 AM David Hildenbrand (Arm) [off-list ref] wrote:

quoted

quoted

 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))) {
	...
}

quoted

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