Re: [PATCH v3 11/13] selftests/mm: fix double increment in linked list cleanup in compaction_test

[David Hildenbrand (Arm)]

On 4/1/26 16:32, Sayali Patil wrote:
> 
> On 27/03/26 12:46, Sayali Patil wrote:
>> The cleanup loop of allocated memory currently uses:
>>
>>      for (entry = list; entry != NULL; entry = entry->next) {
>>          munmap(entry->map, MAP_SIZE);
>>          if (!entry->next)
>>              break;
>>          entry = entry->next;
>>      }
>>
>> The inner entry = entry->next causes the loop to skip every
>> other node, resulting in only half of the mapped regions being
>> unmapped.
>>
>> Remove the redundant increment to ensure every entry is visited
>> and unmapped during cleanup.
>>
>> Fixes: bd67d5c15cc1 ("Test compaction of mlocked memory")
>> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>
>> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>> Tested-by: Venkat Rao Bagalkote <venkat88@xxxxxxxxxxxxx>
>> Signed-off-by: Sayali Patil <sayalip@xxxxxxxxxxxxx>
>> ---
>>   tools/testing/selftests/mm/compaction_test.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/
>> testing/selftests/mm/compaction_test.c
>> index 30209c40b697..f73930706bd0 100644
>> --- a/tools/testing/selftests/mm/compaction_test.c
>> +++ b/tools/testing/selftests/mm/compaction_test.c
>> @@ -263,9 +263,6 @@ int main(int argc, char **argv)
>>         for (entry = list; entry != NULL; entry = entry->next) {
>>           munmap(entry->map, MAP_SIZE);
>> -        if (!entry->next)
>> -            break;
>> -        entry = entry->next;
>>       }
>>         if (check_compaction(mem_free, hugepage_size,
> 
> Sorry, this change is not valid.
> 
> The goal of this test is to verify the kernel’s ability to compact
> unevictable (MAP_LOCKED) pages. The loop is intentionally written to
> unmap every other chunk, thereby creating fragmentation with locked pages
> before check_compaction() is invoked.
> 
> With the proposed change (removing the double increment), the loop ends up
> unmapping all allocated locked pages instead of leaving a fragmented
> pattern. This results in memory being effectively unfragmented.

Ahhh, we should really make that clearer in a comment. I missed it myself :(

-- 
Cheers,

David