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

[Yeoreum Yun <yeoreum.yun@arm.com>]

> > On 9/10/26 13:30, Yeoreum Yun wrote:
> > >> On 9/10/26 13:22, Yeoreum Yun wrote:
> > >>>
> > >>> As I mentioned in my previous reply, what I’m trying to prevent here is
> > >>> a failure when checking, immediately after memory allocation,
> > >>> that a specific vm_flag is not set.
> > >>>
> > >>> Yes, I agree that this could have been a problem even before
> > >>> the internal changes to memalign(). An unwanted VMA merge could already
> > >>> occur at the time of memory allocation.
> > >>>
> > >>> So what I’m trying to avoid is a test failure where, due to such an
> > >>> unexpected VMA merge during allocation, the subsequent check that
> > >>> a specific vm_flag is not present fails.
> > >> Which is only a guard-region marker problem?
> > > 
> > > 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.

Ah, but if merged with PROT_NONE | GURAD still problem...
Hmm.. might above enough.

-- 
Sincerely,
Yeoreum Yun