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

[anthony.yznaga@oracle.com]

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@kernel.org>
> 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.