On 6/3/26 14:16, Nico Pache wrote:
> On Wed, Jun 3, 2026 at 4:01 AM David Hildenbrand (Arm) <[email protected]>
> 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
