Re: [PATCH v5 3/3] kselftest: mm: introduce alloc_isolated_mem()

[David Hildenbrand (Arm)]

On 9/10/26 14:16, Yeoreum Yun wrote:
>> On 9/10/26 13:30, Yeoreum Yun wrote:
>>>
>>> Yes. so if we remove ASSERT_FALSE(check_vmflag_guard(ptr)), TBH
>>> we don't need this patch unless other usage comes up to prevent unwanted
>>> VMA merge.
>>>
>>> Would it be better to drop ASSERT_FALSE(check_vmflag_guard(ptr)) in
>>> guard test?
>>
>> I guess there is value in asserting that not all VMAs by accident start
>> with an over-indication of maybe having guard pages, which is why Lorenzo
>> added that check :)
>>
>> But I think even alloc_isolated_mem() is wrong in that regard: if the
>> original VMA gets merged, we could inherit the guard-marker, no?
> 
> True. I overlooked guard bit is sticky.
> 
>>
>> Maybe the following would be good enough?
>>
>> diff --git a/tools/testing/selftests/mm/guard-regions.c b/tools/testing/selftests/mm/guard-regions.c
>> index 5c8ec3ca75d7d..791bf6a68b9e6 100644
>> --- a/tools/testing/selftests/mm/guard-regions.c
>> +++ b/tools/testing/selftests/mm/guard-regions.c
>> @@ -2257,10 +2257,20 @@ TEST_F(guard_regions, smaps)
>>         char *ptr, *ptr2;
>>         int i;
>>
>> -       /* Map a region. */
>> -       ptr = mmap_(self, variant, NULL, 10 * page_size, PROT_READ | PROT_WRITE, 0, 0);
>> +       /* Reserve a 10 page region with 1 page space to both sides. */
>> +       ptr = mmap_(self, variant, NULL, 12 * page_size, PROT_NONE, 0, 0);
>>         ASSERT_NE(ptr, MAP_FAILED);
>>
>> +       /* Map a new region that is guaranteed to not get merged in any way. */
>> +       ptr = mmap_(self, variant | MAP_FIXED, ptr + pagesize, 10 * page_size,
>> +                   PROT_READ | PROT_WRITE, 0, 0);
>> +       ASSERT_EQ(ptr, ptr + pagesize);
>> +
>> +       /* Clean up the excess pages left and right. */
>> +       munmap(ptr, pagesize);
>> +       munmap(ptr + 11, pagesize);
>> +       ptr + = pagesize;
>> +
>>         /* We shouldn't yet see a guard flag. */
>>         ASSERT_FALSE(check_vmflag_guard(ptr));
>>
>> -- 
>> Cheers,
> 
> It's enough but alloc_isolated_mem() could be modified to allocate with
> PROT_NONE first and then change the prot.
> 
> It seems whether make a helper or testcase should write the code
> properly in case of unwanted VMA merge.
> 
> IMHO, instead of testcase write them all

So far I am aware of exactly one test case, which others do absolutely need this?


-- 
Cheers,

David