Re: [PATCH v5 3/3] kselftest: mm: introduce alloc_isolated_mem()
From: Yeoreum Yun
Date: Thu Sep 10 2026 - 08:35:16 EST
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