Re: [PATCH v3 2/2] selftests/mm: verify droppable mappings cannot be locked

[anthony . yznaga]

On 4/13/26 12:46 PM, David Hildenbrand (Arm) wrote:
> > > > +    if (munlockall()) {
> > > > +        ksft_test_result_fail("munlockall() %s\n", strerror(errno));
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
> > > > +           MAP_ANONYMOUS | MAP_DROPPABLE, -1, 0);
> > > > +    if (map == MAP_FAILED) {
> > > > +        if (errno == EOPNOTSUPP)
> > > > +            ksft_test_result_skip("%s: MAP_DROPPABLE not
> > > > supported\n", __func__);
> > > > +        else
> > > > +            ksft_test_result_fail("mmap error: %s\n", strerror(errno));
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    if (mlock2_(map, 2 * page_size, 0))
> > > Weird, is "mlock2_" actually correct? (not "mlock2") ?
> > It's correct though mlock2 would also work since it's been in glibc for
> > several years now. I just matched the existing tests. mlock2_ is a
> > simple wrapper around syscall in tools/testing/selftests/mm/mlock2.h,
> > and it was introduced when the mlock2 syscall was introduced. A trailing
> > rather than a preceding underscore is...unfortunate.
> Interesting ... and confusing :)
>
> > > > +        ksft_test_result_fail("mlock2(0): %s\n", strerror(errno));
> > > > +    else
> > > > +        ksft_test_result(!unlock_lock_check(map, false),
> > > > +                "%s: droppable memory not locked\n", __func__);
> > > > +
> > > > +    munmap(map, 2 * page_size);
> > > > +}
> > > > +
> > > > +static void test_mlockall_future_droppable(void)
> > > > +{
> > > > +    char *map;
> > > > +    unsigned long page_size = getpagesize();
> > > > +
> > > > +    if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
> > > > +        ksft_test_result_fail("mlockall(MCL_CURRENT | MCL_FUTURE):
> > > > %s\n", strerror(errno));
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
> > > > +           MAP_ANONYMOUS | MAP_DROPPABLE, -1, 0);
> > > > +
> > > > +    if (map == MAP_FAILED) {
> > > > +        if (errno == EOPNOTSUPP)
> > > > +            ksft_test_result_skip("%s: MAP_DROPPABLE not
> > > > supported\n", __func__);
> > > > +        else
> > > > +            ksft_test_result_fail("mmap error: %s\n", strerror(errno));
> > > > +        munlockall();
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    ksft_test_result(!unlock_lock_check(map, false), "%s: droppable
> > > > memory not locked\n",
> > > > +            __func__);
> > > > +
> > > > +    munlockall();
> > > >        munmap(map, 2 * page_size);
> > > >    }
> > > > +#else
> > > > +static void test_mlock_droppable(void)
> > > > +{
> > > > +    ksft_test_result_skip("%s: MAP_DROPPABLE not supported\n",
> > > > __func__);
> > > > +}
> > > > +
> > > > +static void test_mlockall_future_droppable(void)
> > > > +{
> > > > +    ksft_test_result_skip("%s: MAP_DROPPABLE not supported\n",
> > > > __func__);
> > > > +}
> > > > +#endif /* MAP_DROPPABLE */
> > > >    
> > > Why not a above a
> > >
> > > #ifndef MAP_DROPPABLE
> > > #define MAP_DROPPABLE    0x08
> > > #endif
> > >
> > > instead?
> > The intent was to skip the tests if compiled with headers where
> > MAP_DROPPABLE isn't defined rather than force the value and get EINVAL
> > because the kernel doesn't know about it. This way EINVAL can be flagged
> > as a test failure and not skipped since it would likely indicate a test
> > or kernel bug.
> Note that the kernel headers you are compiling against don't imply
> anything about the kernel that is actually running! So the argument
> regarding EINVAL doesn't really hold.

Yes, I see that now.


> But note that we, in general, try to compile against the in-tree headers.
>
> See
>
> commit 75d60eb30daafb966db0e45f38e4cdeb5e5ed79c
> Author: Lorenzo Stoakes <ljs@xxxxxxxxxx>
> Date:   Mon Oct 28 14:13:30 2024 +0000
>
>      tools: testing: update tools UAPI header for mman-common.h
>
>      Import the new MADV_GUARD_INSTALL/REMOVE madvise flags.
>
> And looking into it, I already see MAP_DROPPABLE there as well, so is
> any special handling here even needed?
I'd say no special handling is needed. If this test is compiled on with 
a kernel that has MAP_DROPPABLE but run on an older kernel without 
MAP_DROPPABLE, should it fail? Or should it interpret the EINVAL as a 
sufficient reason to assume MAP_DROPPABLE is not supported and to skip 
the result? My current thinking is it should fail because the EINVAL 
could have another cause.