Hello,
A tester ran the upstream selftest on a distro kernel and sounded the alarm when
it reported failures for features which aren't included in that kernel.
This patch set improves the test behavior in that scenario.
Thiago Jung Bauermann (3):
userfaultfd: selftest: Fix checking of userfaultfd_open() result
userfaultfd: selftest: Skip test if a feature isn't supported
userfaultfd: selftest: Report XFAIL if shmem doesn't support zeropage
tools/testing/selftests/vm/userfaultfd.c | 49 ++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 12 deletions(-)
If userfaultfd runs on a system that doesn't support UFFDIO_ZEROPAGE for
shared memory, it currently ends with error code 1 which indicates test
failure:
# ./userfaultfd shmem 10 10
nr_pages: 160, nr_pages_per_cpu: 80
bounces: 9, mode: rnd poll, unexpected missing ioctl for anon memory
# echo $?
1
This is a real failure, but expected so signal that to the test harness:
# ./userfaultfd shmem 10 10
nr_pages: 160, nr_pages_per_cpu: 80
bounces: 9, mode: rnd poll, UFFDIO_ZEROPAGE unsupported in shmem VMAs
# echo $?
2
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
tools/testing/selftests/vm/userfaultfd.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index bc9ec38fbc34..686fe96f617f 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -1115,6 +1115,14 @@ static int userfaultfd_stress(void)
expected_ioctls = uffd_test_ops->expected_ioctls;
if ((uffdio_register.ioctls & expected_ioctls) !=
expected_ioctls) {
+ if (test_type == TEST_SHMEM &&
+ (uffdio_register.ioctls & expected_ioctls) ==
+ UFFD_API_RANGE_IOCTLS_BASIC) {
+ fprintf(stderr,
+ "UFFDIO_ZEROPAGE unsupported in shmem VMAs\n");
+ return KSFT_XFAIL;
+ }
+
fprintf(stderr,
"unexpected missing ioctl for anon memory\n");
return 1;
If the userfaultfd test is run on a kernel with CONFIG_USERFAULTFD=n, it
will report that the system call is not available yet go ahead and continue
anyway:
# ./userfaultfd anon 30 1
nr_pages: 480, nr_pages_per_cpu: 120
userfaultfd syscall not available in this kernel
bounces: 0, mode:, register failure
This is because userfaultfd_open() returns 0 on success and 1 on error but
all callers assume that it returns < 0 on error.
Since the convention of the test as a whole is the one used by
userfault_open(), fix its callers instead. Now the test behaves correctly:
# ./userfaultfd anon 30 1
nr_pages: 480, nr_pages_per_cpu: 120
userfaultfd syscall not available in this kernel
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
tools/testing/selftests/vm/userfaultfd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 7b8171e3128a..e4099afe7557 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -859,7 +859,7 @@ static int userfaultfd_zeropage_test(void)
if (uffd_test_ops->release_pages(area_dst))
return 1;
- if (userfaultfd_open(0) < 0)
+ if (userfaultfd_open(0))
return 1;
uffdio_register.range.start = (unsigned long) area_dst;
uffdio_register.range.len = nr_pages * page_size;
@@ -902,7 +902,7 @@ static int userfaultfd_events_test(void)
features = UFFD_FEATURE_EVENT_FORK | UFFD_FEATURE_EVENT_REMAP |
UFFD_FEATURE_EVENT_REMOVE;
- if (userfaultfd_open(features) < 0)
+ if (userfaultfd_open(features))
return 1;
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
@@ -961,7 +961,7 @@ static int userfaultfd_sig_test(void)
return 1;
features = UFFD_FEATURE_EVENT_FORK|UFFD_FEATURE_SIGBUS;
- if (userfaultfd_open(features) < 0)
+ if (userfaultfd_open(features))
return 1;
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
@@ -1027,7 +1027,7 @@ static int userfaultfd_stress(void)
if (!area_dst)
return 1;
- if (userfaultfd_open(0) < 0)
+ if (userfaultfd_open(0))
return 1;
count_verify = malloc(nr_pages * sizeof(unsigned long long));
Hi,
On Tue, Jul 24, 2018 at 11:42:07PM -0300, Thiago Jung Bauermann wrote:
> If the userfaultfd test is run on a kernel with CONFIG_USERFAULTFD=n, it
> will report that the system call is not available yet go ahead and continue
> anyway:
>
> # ./userfaultfd anon 30 1
> nr_pages: 480, nr_pages_per_cpu: 120
> userfaultfd syscall not available in this kernel
> bounces: 0, mode:, register failure
>
> This is because userfaultfd_open() returns 0 on success and 1 on error but
> all callers assume that it returns < 0 on error.
>
> Since the convention of the test as a whole is the one used by
> userfault_open(), fix its callers instead. Now the test behaves correctly:
>
> # ./userfaultfd anon 30 1
> nr_pages: 480, nr_pages_per_cpu: 120
> userfaultfd syscall not available in this kernel
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
It seems that this patch is superseded by the second patch in this series.
> ---
> tools/testing/selftests/vm/userfaultfd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 7b8171e3128a..e4099afe7557 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -859,7 +859,7 @@ static int userfaultfd_zeropage_test(void)
> if (uffd_test_ops->release_pages(area_dst))
> return 1;
>
> - if (userfaultfd_open(0) < 0)
> + if (userfaultfd_open(0))
> return 1;
> uffdio_register.range.start = (unsigned long) area_dst;
> uffdio_register.range.len = nr_pages * page_size;
> @@ -902,7 +902,7 @@ static int userfaultfd_events_test(void)
>
> features = UFFD_FEATURE_EVENT_FORK | UFFD_FEATURE_EVENT_REMAP |
> UFFD_FEATURE_EVENT_REMOVE;
> - if (userfaultfd_open(features) < 0)
> + if (userfaultfd_open(features))
> return 1;
> fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
>
> @@ -961,7 +961,7 @@ static int userfaultfd_sig_test(void)
> return 1;
>
> features = UFFD_FEATURE_EVENT_FORK|UFFD_FEATURE_SIGBUS;
> - if (userfaultfd_open(features) < 0)
> + if (userfaultfd_open(features))
> return 1;
> fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
>
> @@ -1027,7 +1027,7 @@ static int userfaultfd_stress(void)
> if (!area_dst)
> return 1;
>
> - if (userfaultfd_open(0) < 0)
> + if (userfaultfd_open(0))
> return 1;
>
> count_verify = malloc(nr_pages * sizeof(unsigned long long));
--
Sincerely yours,
Mike.
Hi,
On Tue, Jul 24, 2018 at 11:42:09PM -0300, Thiago Jung Bauermann wrote:
> If userfaultfd runs on a system that doesn't support UFFDIO_ZEROPAGE for
> shared memory, it currently ends with error code 1 which indicates test
> failure:
>
> # ./userfaultfd shmem 10 10
> nr_pages: 160, nr_pages_per_cpu: 80
> bounces: 9, mode: rnd poll, unexpected missing ioctl for anon memory
> # echo $?
> 1
>
> This is a real failure, but expected so signal that to the test harness:
I don't think its a real failure. If the kernel does not support
UFFDIO_ZEROPAGE for shared memory the userfaultfd_zeropage_test can be
simply skipped.
> # ./userfaultfd shmem 10 10
> nr_pages: 160, nr_pages_per_cpu: 80
> bounces: 9, mode: rnd poll, UFFDIO_ZEROPAGE unsupported in shmem VMAs
> # echo $?
> 2
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
> ---
> tools/testing/selftests/vm/userfaultfd.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index bc9ec38fbc34..686fe96f617f 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -1115,6 +1115,14 @@ static int userfaultfd_stress(void)
> expected_ioctls = uffd_test_ops->expected_ioctls;
> if ((uffdio_register.ioctls & expected_ioctls) !=
> expected_ioctls) {
> + if (test_type == TEST_SHMEM &&
> + (uffdio_register.ioctls & expected_ioctls) ==
> + UFFD_API_RANGE_IOCTLS_BASIC) {
> + fprintf(stderr,
> + "UFFDIO_ZEROPAGE unsupported in shmem VMAs\n");
> + return KSFT_XFAIL;
> + }
> +
By all means, this check should be moved to userfaultfd_zeropage_test().
Ideally, we should call here ksft_test_result_skip() and simply return from
the function.
> fprintf(stderr,
> "unexpected missing ioctl for anon memory\n");
> return 1;
--
Sincerely yours,
Mike.
Hello Mike,
Thanks for promptly reviewing the patches.
Mike Rapoport <[email protected]> writes:
> Hi,
>
> On Tue, Jul 24, 2018 at 11:42:07PM -0300, Thiago Jung Bauermann wrote:
>> If the userfaultfd test is run on a kernel with CONFIG_USERFAULTFD=n, it
>> will report that the system call is not available yet go ahead and continue
>> anyway:
>>
>> # ./userfaultfd anon 30 1
>> nr_pages: 480, nr_pages_per_cpu: 120
>> userfaultfd syscall not available in this kernel
>> bounces: 0, mode:, register failure
>>
>> This is because userfaultfd_open() returns 0 on success and 1 on error but
>> all callers assume that it returns < 0 on error.
>>
>> Since the convention of the test as a whole is the one used by
>> userfault_open(), fix its callers instead. Now the test behaves correctly:
>>
>> # ./userfaultfd anon 30 1
>> nr_pages: 480, nr_pages_per_cpu: 120
>> userfaultfd syscall not available in this kernel
>>
>> Signed-off-by: Thiago Jung Bauermann <[email protected]>
>
> It seems that this patch is superseded by the second patch in this series.
Yes, but since this is a simple bugfix while the other patch is a
proposed improvement which can be debated, I think it's worthwhile to
keep them separate.
--
Thiago Jung Bauermann
IBM Linux Technology Center
Mike Rapoport <[email protected]> writes:
> Hi,
>
> On Tue, Jul 24, 2018 at 11:42:09PM -0300, Thiago Jung Bauermann wrote:
>> If userfaultfd runs on a system that doesn't support UFFDIO_ZEROPAGE for
>> shared memory, it currently ends with error code 1 which indicates test
>> failure:
>>
>> # ./userfaultfd shmem 10 10
>> nr_pages: 160, nr_pages_per_cpu: 80
>> bounces: 9, mode: rnd poll, unexpected missing ioctl for anon memory
>> # echo $?
>> 1
>>
>> This is a real failure, but expected so signal that to the test harness:
>
> I don't think its a real failure. If the kernel does not support
> UFFDIO_ZEROPAGE for shared memory the userfaultfd_zeropage_test can be
> simply skipped.
Ok, good point. I'll make that change in v2.
>> # ./userfaultfd shmem 10 10
>> nr_pages: 160, nr_pages_per_cpu: 80
>> bounces: 9, mode: rnd poll, UFFDIO_ZEROPAGE unsupported in shmem VMAs
>> # echo $?
>> 2
>>
>> Signed-off-by: Thiago Jung Bauermann <[email protected]>
>> ---
>> tools/testing/selftests/vm/userfaultfd.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
>> index bc9ec38fbc34..686fe96f617f 100644
>> --- a/tools/testing/selftests/vm/userfaultfd.c
>> +++ b/tools/testing/selftests/vm/userfaultfd.c
>> @@ -1115,6 +1115,14 @@ static int userfaultfd_stress(void)
>> expected_ioctls = uffd_test_ops->expected_ioctls;
>> if ((uffdio_register.ioctls & expected_ioctls) !=
>> expected_ioctls) {
>> + if (test_type == TEST_SHMEM &&
>> + (uffdio_register.ioctls & expected_ioctls) ==
>> + UFFD_API_RANGE_IOCTLS_BASIC) {
>> + fprintf(stderr,
>> + "UFFDIO_ZEROPAGE unsupported in shmem VMAs\n");
>> + return KSFT_XFAIL;
>> + }
>> +
>
> By all means, this check should be moved to userfaultfd_zeropage_test().
I made that change in v2.
> Ideally, we should call here ksft_test_result_skip() and simply return from
> the function.
In my understanding, calling ksft_test_result_skip() would require
converting the testcase to use the functions that generate TAP output.
Also, returning here isn't actually necessary: from my testing
userfaultfd_stress() doesn't require zeropage support in shmem so if the
only bit missing from uffdio_register.ioctls is the one for
UFFDIO_ZEROPAGE then this error can simply be ignored and the test can
continue. Do you agree?
>> fprintf(stderr,
>> "unexpected missing ioctl for anon memory\n");
>> return 1;
--
Thiago Jung Bauermann
IBM Linux Technology Center