On Thu, Sep 10, 2026 at 02:25:40PM +0200, David Hildenbrand (Arm) wrote: > 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?
True. I'll respin with your suggestion. Thanks! -- Sincerely, Yeoreum Yun
