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.

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, it would be better to use
helper (but in guard-region.c, might need to wrapper function, munmap_()
like mmap_()).

-- 
Sincerely,
Yeoreum Yun