2023-11-02 11:47:31

by Ryan Roberts

Subject: Re: [PATCH v33 6/6] selftests: mm: add pagemap ioctl tests

On 21/08/2023 15:15, Muhammad Usama Anjum wrote:

[...]

> +
> +
> +int init_uffd(void)
> +{
> + struct uffdio_api uffdio_api;
> +
> + uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
> + if (uffd == -1)
> + ksft_exit_fail_msg("uffd syscall failed\n");
> +
> + uffdio_api.api = UFFD_API;
> + uffdio_api.features = UFFD_FEATURE_WP_UNPOPULATED | UFFD_FEATURE_WP_ASYNC |
> + UFFD_FEATURE_WP_HUGETLBFS_SHMEM;
> + if (ioctl(uffd, UFFDIO_API, &uffdio_api))
> + ksft_exit_fail_msg("UFFDIO_API\n");
> +
> + if (!(uffdio_api.api & UFFDIO_REGISTER_MODE_WP) ||
> + !(uffdio_api.features & UFFD_FEATURE_WP_UNPOPULATED) ||
> + !(uffdio_api.features & UFFD_FEATURE_WP_ASYNC) ||
> + !(uffdio_api.features & UFFD_FEATURE_WP_HUGETLBFS_SHMEM))
> + ksft_exit_fail_msg("UFFDIO_API error %llu\n", uffdio_api.api);

Hi,

I've just noticed that this fails on arm64 because the required features are not
available. It's common practice to skip instead of fail for this sort of
condition (and that's how all the other uffd tests work). The current fail
approach creates noise in our CI.

I see this is already in mm-stable so perhaps we can add a patch to fix on top?

Thanks,
Ryan

2023-11-02 11:50:30

by Muhammad Usama Anjum

Subject: Re: [PATCH v33 6/6] selftests: mm: add pagemap ioctl tests

On 11/2/23 4:45 PM, Ryan Roberts wrote:
> On 21/08/2023 15:15, Muhammad Usama Anjum wrote:
>
> [...]
>
>> +
>> +
>> +int init_uffd(void)
>> +{
>> + struct uffdio_api uffdio_api;
>> +
>> + uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
>> + if (uffd == -1)
>> + ksft_exit_fail_msg("uffd syscall failed\n");
>> +
>> + uffdio_api.api = UFFD_API;
>> + uffdio_api.features = UFFD_FEATURE_WP_UNPOPULATED | UFFD_FEATURE_WP_ASYNC |
>> + UFFD_FEATURE_WP_HUGETLBFS_SHMEM;
>> + if (ioctl(uffd, UFFDIO_API, &uffdio_api))
>> + ksft_exit_fail_msg("UFFDIO_API\n");
>> +
>> + if (!(uffdio_api.api & UFFDIO_REGISTER_MODE_WP) ||
>> + !(uffdio_api.features & UFFD_FEATURE_WP_UNPOPULATED) ||
>> + !(uffdio_api.features & UFFD_FEATURE_WP_ASYNC) ||
>> + !(uffdio_api.features & UFFD_FEATURE_WP_HUGETLBFS_SHMEM))
>> + ksft_exit_fail_msg("UFFDIO_API error %llu\n", uffdio_api.api);
>
> Hi,
>
> I've just noticed that this fails on arm64 because the required features are not
> available. It's common practice to skip instead of fail for this sort of
> condition (and that's how all the other uffd tests work). The current fail
> approach creates noise in our CI.
>
> I see this is already in mm-stable so perhaps we can add a patch to fix on top?
Yeah, we can add a patch to skip all the tests instead of failing here. Let
me send a patch this week.

>
> Thanks,
> Ryan
>
>

--
BR,
Muhammad Usama Anjum

2023-11-02 12:07:13

by Ryan Roberts

Subject: Re: [PATCH v33 6/6] selftests: mm: add pagemap ioctl tests

On 02/11/2023 11:49, Muhammad Usama Anjum wrote:
> On 11/2/23 4:45 PM, Ryan Roberts wrote:
>> On 21/08/2023 15:15, Muhammad Usama Anjum wrote:
>>
>> [...]
>>
>>> +
>>> +
>>> +int init_uffd(void)
>>> +{
>>> + struct uffdio_api uffdio_api;
>>> +
>>> + uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
>>> + if (uffd == -1)
>>> + ksft_exit_fail_msg("uffd syscall failed\n");
>>> +
>>> + uffdio_api.api = UFFD_API;
>>> + uffdio_api.features = UFFD_FEATURE_WP_UNPOPULATED | UFFD_FEATURE_WP_ASYNC |
>>> + UFFD_FEATURE_WP_HUGETLBFS_SHMEM;
>>> + if (ioctl(uffd, UFFDIO_API, &uffdio_api))
>>> + ksft_exit_fail_msg("UFFDIO_API\n");
>>> +
>>> + if (!(uffdio_api.api & UFFDIO_REGISTER_MODE_WP) ||
>>> + !(uffdio_api.features & UFFD_FEATURE_WP_UNPOPULATED) ||
>>> + !(uffdio_api.features & UFFD_FEATURE_WP_ASYNC) ||
>>> + !(uffdio_api.features & UFFD_FEATURE_WP_HUGETLBFS_SHMEM))
>>> + ksft_exit_fail_msg("UFFDIO_API error %llu\n", uffdio_api.api);
>>
>> Hi,
>>
>> I've just noticed that this fails on arm64 because the required features are not
>> available. It's common practice to skip instead of fail for this sort of
>> condition (and that's how all the other uffd tests work). The current fail
>> approach creates noise in our CI.
>>
>> I see this is already in mm-stable so perhaps we can add a patch to fix on top?
> Yeah, we can add a patch to skip all the tests instead of failing here. Let
> me send a patch this week.

Thats great - thanks for the fast response!

>
>>
>> Thanks,
>> Ryan
>>
>>
>