Hi Shuah,
a small series collecting a few trivial fixes that I have already sent previously (~Dec2021) as distinct patches.
They are mostly trivial patches addressing failures that seemed more sensible to be marked as skips instead. (at least to me ...). Original developers are in Cc. (but not heard back from anyone :D)
Thanks, Cristian
Cristian Marussi (5): selftests: skip mincore.check_file_mmap when fs lacks needed support kselftest: Fix vdso_test_time to pass on skips selftests: openat2: Print also errno in failure messages selftests: openat2: Add missing dependency in Makefile selftests: openat2: Skip testcases that fail with EOPNOTSUPP
.../selftests/mincore/mincore_selftest.c | 20 +++++++++++++------ tools/testing/selftests/openat2/Makefile | 2 +- tools/testing/selftests/openat2/helpers.h | 12 ++++++----- .../testing/selftests/openat2/openat2_test.c | 12 ++++++++++- tools/testing/selftests/vDSO/vdso_test_abi.c | 3 ++- 5 files changed, 35 insertions(+), 14 deletions(-)
Report mincore.check_file_mmap as SKIP instead of FAIL if the underlying filesystem lacks support of O_TMPFILE or fallocate since such failures are not really related to mincore functionality.
Cc: Ricardo Cañuelo ricardo.canuelo@collabora.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com --- This can happen especially on test-automation systems where rootfs can be configured as being on NFS or virtual disks. --- .../selftests/mincore/mincore_selftest.c | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/mincore/mincore_selftest.c b/tools/testing/selftests/mincore/mincore_selftest.c index e54106643337..4c88238fc8f0 100644 --- a/tools/testing/selftests/mincore/mincore_selftest.c +++ b/tools/testing/selftests/mincore/mincore_selftest.c @@ -207,15 +207,21 @@ TEST(check_file_mmap)
errno = 0; fd = open(".", O_TMPFILE | O_RDWR, 0600); - ASSERT_NE(-1, fd) { - TH_LOG("Can't create temporary file: %s", - strerror(errno)); + if (fd < 0) { + ASSERT_EQ(errno, EOPNOTSUPP) { + TH_LOG("Can't create temporary file: %s", + strerror(errno)); + } + SKIP(goto out_free, "O_TMPFILE not supported by filesystem."); } errno = 0; retval = fallocate(fd, 0, 0, FILE_SIZE); - ASSERT_EQ(0, retval) { - TH_LOG("Error allocating space for the temporary file: %s", - strerror(errno)); + if (retval) { + ASSERT_EQ(errno, EOPNOTSUPP) { + TH_LOG("Error allocating space for the temporary file: %s", + strerror(errno)); + } + SKIP(goto out_close, "fallocate not supported by filesystem."); }
/* @@ -271,7 +277,9 @@ TEST(check_file_mmap) }
munmap(addr, FILE_SIZE); +out_close: close(fd); +out_free: free(vec); }
When a vDSO symbol is not found, all the testcases in vdso_test_abi usually report a SKIP, which, in turn, is reported back to Kselftest as a PASS.
Testcase vdso_test_time, instead, reporting a SKIP, causes the whole set of tests within vdso_test_abi to be considered FAIL when symbol is not found.
Fix it reporting a PASS when vdso_test_time cannot find the vdso symbol.
Cc: Vincenzo Frascino vincenzo.frascino@arm.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com --- Seen as a failure on both a JUNO and a Dragonboard on both recent and old kernels/testruns:
root@deb-buster-arm64:~# /opt/ksft/vDSO/vdso_test_abi [vDSO kselftest] VDSO_VERSION: LINUX_2.6.39 The time is 1637922136.675304 The time is 1637922136.675361000 The resolution is 0 1 clock_id: CLOCK_REALTIME [PASS] The time is 1927.760604900 The resolution is 0 1 clock_id: CLOCK_BOOTTIME [PASS] The time is 1637922136.675649700 The resolution is 0 1 clock_id: CLOCK_TAI [PASS] The time is 1637922136.672000000 The resolution is 0 4000000 clock_id: CLOCK_REALTIME_COARSE [PASS] The time is 1927.761005600 The resolution is 0 1 clock_id: CLOCK_MONOTONIC [PASS] The time is 1927.761132780 The resolution is 0 1 clock_id: CLOCK_MONOTONIC_RAW [PASS] The time is 1927.757093740 The resolution is 0 4000000 clock_id: CLOCK_MONOTONIC_COARSE [PASS] Could not find __kernel_time <<< This caused a FAIL as a whole root@deb-buster-arm64:~# echo $? 1
e.g.: https://lkft.validation.linaro.org/scheduler/job/2192570#L27778 --- tools/testing/selftests/vDSO/vdso_test_abi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_abi.c b/tools/testing/selftests/vDSO/vdso_test_abi.c index 3d603f1394af..7dcc66d1cecf 100644 --- a/tools/testing/selftests/vDSO/vdso_test_abi.c +++ b/tools/testing/selftests/vDSO/vdso_test_abi.c @@ -90,8 +90,9 @@ static int vdso_test_time(void) (vdso_time_t)vdso_sym(version, name[2]);
if (!vdso_time) { + /* Skip if symbol not found: consider skipped tests as passed */ printf("Could not find %s\n", name[2]); - return KSFT_SKIP; + return KSFT_PASS; }
long ret = vdso_time(NULL);
In E_func() macro, on error, print also errno in order to aid debugging.
Cc: Aleksa Sarai cyphar@cyphar.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com --- tools/testing/selftests/openat2/helpers.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/openat2/helpers.h b/tools/testing/selftests/openat2/helpers.h index a6ea27344db2..ad5d0ba5b6ce 100644 --- a/tools/testing/selftests/openat2/helpers.h +++ b/tools/testing/selftests/openat2/helpers.h @@ -62,11 +62,12 @@ bool needs_openat2(const struct open_how *how); (similar to chroot(2)). */ #endif /* RESOLVE_IN_ROOT */
-#define E_func(func, ...) \ - do { \ - if (func(__VA_ARGS__) < 0) \ - ksft_exit_fail_msg("%s:%d %s failed\n", \ - __FILE__, __LINE__, #func);\ +#define E_func(func, ...) \ + do { \ + errno = 0; \ + if (func(__VA_ARGS__) < 0) \ + ksft_exit_fail_msg("%s:%d %s failed - errno:%d\n", \ + __FILE__, __LINE__, #func, errno); \ } while (0)
#define E_asprintf(...) E_func(asprintf, __VA_ARGS__)
Add a dependency on header helpers.h to the main target; while at that add to helpers.h also a missing include for bool types.
Cc: Aleksa Sarai cyphar@cyphar.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com --- tools/testing/selftests/openat2/Makefile | 2 +- tools/testing/selftests/openat2/helpers.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/openat2/Makefile b/tools/testing/selftests/openat2/Makefile index 4b93b1417b86..843ba56d8e49 100644 --- a/tools/testing/selftests/openat2/Makefile +++ b/tools/testing/selftests/openat2/Makefile @@ -5,4 +5,4 @@ TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test
include ../lib.mk
-$(TEST_GEN_PROGS): helpers.c +$(TEST_GEN_PROGS): helpers.c helpers.h diff --git a/tools/testing/selftests/openat2/helpers.h b/tools/testing/selftests/openat2/helpers.h index ad5d0ba5b6ce..7056340b9339 100644 --- a/tools/testing/selftests/openat2/helpers.h +++ b/tools/testing/selftests/openat2/helpers.h @@ -9,6 +9,7 @@
#define _GNU_SOURCE #include <stdint.h> +#include <stdbool.h> #include <errno.h> #include <linux/types.h> #include "../kselftest.h"
Skip testcases that fail since the requested valid flags combination is not supported by the underlying filesystem.
Cc: Aleksa Sarai cyphar@cyphar.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com --- tools/testing/selftests/openat2/openat2_test.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c index 1bddbe934204..7fb902099de4 100644 --- a/tools/testing/selftests/openat2/openat2_test.c +++ b/tools/testing/selftests/openat2/openat2_test.c @@ -259,6 +259,16 @@ void test_openat2_flags(void) unlink(path);
fd = sys_openat2(AT_FDCWD, path, &test->how); + if (fd < 0 && fd == -EOPNOTSUPP) { + /* + * Skip the testcase if it failed because not supported + * by FS. (e.g. a valid O_TMPFILE combination on NFS) + */ + ksft_test_result_skip("openat2 with %s fails with %d (%s)\n", + test->name, fd, strerror(-fd)); + goto next; + } + if (test->err >= 0) failed = (fd < 0); else @@ -303,7 +313,7 @@ void test_openat2_flags(void) else resultfn("openat2 with %s fails with %d (%s)\n", test->name, test->err, strerror(-test->err)); - +next: free(fdpath); fflush(stdout); }
Hi Cristian,
On 1/26/22 10:27 AM, Cristian Marussi wrote:
When a vDSO symbol is not found, all the testcases in vdso_test_abi usually report a SKIP, which, in turn, is reported back to Kselftest as a PASS.
Testcase vdso_test_time, instead, reporting a SKIP, causes the whole set of tests within vdso_test_abi to be considered FAIL when symbol is not found.
Fix it reporting a PASS when vdso_test_time cannot find the vdso symbol.
Cc: Vincenzo Frascino vincenzo.frascino@arm.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com
Seen as a failure on both a JUNO and a Dragonboard on both recent and old kernels/testruns:
root@deb-buster-arm64:~# /opt/ksft/vDSO/vdso_test_abi [vDSO kselftest] VDSO_VERSION: LINUX_2.6.39 The time is 1637922136.675304 The time is 1637922136.675361000 The resolution is 0 1 clock_id: CLOCK_REALTIME [PASS] The time is 1927.760604900 The resolution is 0 1 clock_id: CLOCK_BOOTTIME [PASS] The time is 1637922136.675649700 The resolution is 0 1 clock_id: CLOCK_TAI [PASS] The time is 1637922136.672000000 The resolution is 0 4000000 clock_id: CLOCK_REALTIME_COARSE [PASS] The time is 1927.761005600 The resolution is 0 1 clock_id: CLOCK_MONOTONIC [PASS] The time is 1927.761132780 The resolution is 0 1 clock_id: CLOCK_MONOTONIC_RAW [PASS] The time is 1927.757093740 The resolution is 0 4000000 clock_id: CLOCK_MONOTONIC_COARSE [PASS] Could not find __kernel_time <<< This caused a FAIL as a whole root@deb-buster-arm64:~# echo $? 1
e.g.: https://lkft.validation.linaro.org/scheduler/job/2192570#L27778
tools/testing/selftests/vDSO/vdso_test_abi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_abi.c b/tools/testing/selftests/vDSO/vdso_test_abi.c index 3d603f1394af..7dcc66d1cecf 100644 --- a/tools/testing/selftests/vDSO/vdso_test_abi.c +++ b/tools/testing/selftests/vDSO/vdso_test_abi.c @@ -90,8 +90,9 @@ static int vdso_test_time(void) (vdso_time_t)vdso_sym(version, name[2]); if (!vdso_time) {
/* Skip if symbol not found: consider skipped tests as passed */
return KSFT_SKIP;
return KSFT_PASS;
My preference would be to keep "KSFT_SKIP" here and verify separately the return status of each test. This would maintain compliance with the kselftest API. Could you please test the patch in-reply-to this one (will be sent shortly) and let me know if it works for you?
If it does feel free to fold it in the next version of your series with your "Tested-by:" otherwise let me know.
Thanks!
} long ret = vdso_time(NULL);
vdso_test_abi contains a batch of tests that verify the validity of the vDSO ABI.
When a vDSO symbol is not found the relevant test is skipped reporting KSFT_SKIP. All the tests return values are then added in a single variable which is checked to verify failures. This approach can have side effects which result in reporting the wrong kselftest exit status.
Fix vdso_test_abi verifying the return code of each test separately.
Cc: Shuah Khan shuah@kernel.org Cc: Andy Lutomirski luto@kernel.org Cc: Thomas Gleixner tglx@linutronix.de Reported-by: Cristian Marussi cristian.marussi@arm.com Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com --- tools/testing/selftests/vDSO/vdso_test_abi.c | 27 +++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_abi.c b/tools/testing/selftests/vDSO/vdso_test_abi.c index 3d603f1394af..3a4efb91b9b2 100644 --- a/tools/testing/selftests/vDSO/vdso_test_abi.c +++ b/tools/testing/selftests/vDSO/vdso_test_abi.c @@ -184,10 +184,12 @@ static inline int vdso_test_clock(clockid_t clock_id) return ret0; }
+#define VDSO_TESTS_MAX 9 + int main(int argc, char **argv) { unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); - int ret; + int ret[VDSO_TESTS_MAX] = {0};
if (!sysinfo_ehdr) { printf("AT_SYSINFO_EHDR is not present!\n"); @@ -201,44 +203,45 @@ int main(int argc, char **argv)
vdso_init_from_sysinfo_ehdr(getauxval(AT_SYSINFO_EHDR));
- ret = vdso_test_gettimeofday(); + ret[0] = vdso_test_gettimeofday();
#if _POSIX_TIMERS > 0
#ifdef CLOCK_REALTIME - ret += vdso_test_clock(CLOCK_REALTIME); + ret[1] = vdso_test_clock(CLOCK_REALTIME); #endif
#ifdef CLOCK_BOOTTIME - ret += vdso_test_clock(CLOCK_BOOTTIME); + ret[2] = vdso_test_clock(CLOCK_BOOTTIME); #endif
#ifdef CLOCK_TAI - ret += vdso_test_clock(CLOCK_TAI); + ret[3] = vdso_test_clock(CLOCK_TAI); #endif
#ifdef CLOCK_REALTIME_COARSE - ret += vdso_test_clock(CLOCK_REALTIME_COARSE); + ret[4] = vdso_test_clock(CLOCK_REALTIME_COARSE); #endif
#ifdef CLOCK_MONOTONIC - ret += vdso_test_clock(CLOCK_MONOTONIC); + ret[5] = vdso_test_clock(CLOCK_MONOTONIC); #endif
#ifdef CLOCK_MONOTONIC_RAW - ret += vdso_test_clock(CLOCK_MONOTONIC_RAW); + ret[6] = vdso_test_clock(CLOCK_MONOTONIC_RAW); #endif
#ifdef CLOCK_MONOTONIC_COARSE - ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE); + ret[7] = vdso_test_clock(CLOCK_MONOTONIC_COARSE); #endif
#endif
- ret += vdso_test_time(); + ret[8] = vdso_test_time();
- if (ret > 0) - return KSFT_FAIL; + for (int i = 0; i < VDSO_TESTS_MAX; i++) + if (ret[i] == KSFT_FAIL) + return KSFT_FAIL;
return KSFT_PASS; }
On Wed, Jan 26, 2022 at 12:22:45PM +0000, Vincenzo Frascino wrote:
Hi Cristian,
Hi Vincenzo,
thanks for the feedback.
On 1/26/22 10:27 AM, Cristian Marussi wrote:
When a vDSO symbol is not found, all the testcases in vdso_test_abi usually report a SKIP, which, in turn, is reported back to Kselftest as a PASS.
Testcase vdso_test_time, instead, reporting a SKIP, causes the whole set of tests within vdso_test_abi to be considered FAIL when symbol is not found.
Fix it reporting a PASS when vdso_test_time cannot find the vdso symbol.
Cc: Vincenzo Frascino vincenzo.frascino@arm.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com
Seen as a failure on both a JUNO and a Dragonboard on both recent and old kernels/testruns:
root@deb-buster-arm64:~# /opt/ksft/vDSO/vdso_test_abi [vDSO kselftest] VDSO_VERSION: LINUX_2.6.39 The time is 1637922136.675304 The time is 1637922136.675361000 The resolution is 0 1 clock_id: CLOCK_REALTIME [PASS] The time is 1927.760604900 The resolution is 0 1 clock_id: CLOCK_BOOTTIME [PASS] The time is 1637922136.675649700 The resolution is 0 1 clock_id: CLOCK_TAI [PASS] The time is 1637922136.672000000 The resolution is 0 4000000 clock_id: CLOCK_REALTIME_COARSE [PASS] The time is 1927.761005600 The resolution is 0 1 clock_id: CLOCK_MONOTONIC [PASS] The time is 1927.761132780 The resolution is 0 1 clock_id: CLOCK_MONOTONIC_RAW [PASS] The time is 1927.757093740 The resolution is 0 4000000 clock_id: CLOCK_MONOTONIC_COARSE [PASS] Could not find __kernel_time <<< This caused a FAIL as a whole root@deb-buster-arm64:~# echo $? 1
e.g.: https://lkft.validation.linaro.org/scheduler/job/2192570#L27778
tools/testing/selftests/vDSO/vdso_test_abi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_abi.c b/tools/testing/selftests/vDSO/vdso_test_abi.c index 3d603f1394af..7dcc66d1cecf 100644 --- a/tools/testing/selftests/vDSO/vdso_test_abi.c +++ b/tools/testing/selftests/vDSO/vdso_test_abi.c @@ -90,8 +90,9 @@ static int vdso_test_time(void) (vdso_time_t)vdso_sym(version, name[2]); if (!vdso_time) {
/* Skip if symbol not found: consider skipped tests as passed */
return KSFT_SKIP;
return KSFT_PASS;My preference would be to keep "KSFT_SKIP" here and verify separately the return status of each test. This would maintain compliance with the kselftest API. Could you please test the patch in-reply-to this one (will be sent shortly) and let me know if it works for you?
Sure, I was indeed not sure my solution was what you wanted.
If it does feel free to fold it in the next version of your series with your "Tested-by:" otherwise let me know.
Sure, I'll do and keep you on CC.
Thanks, Cristian
Hi Cristian,
On 1/26/22 12:34 PM, Cristian Marussi wrote:
Sure, I was indeed not sure my solution was what you wanted.
Not a problem and more then anything thank you for reporting the issue.
If it does feel free to fold it in the next version of your series with your "Tested-by:" otherwise let me know.
Sure, I'll do and keep you on CC.
Thanks!
On 1/26/22 3:27 AM, Cristian Marussi wrote:
When a vDSO symbol is not found, all the testcases in vdso_test_abi usually report a SKIP, which, in turn, is reported back to Kselftest as a PASS.
Testcase vdso_test_time, instead, reporting a SKIP, causes the whole set of tests within vdso_test_abi to be considered FAIL when symbol is not found.
Fix it reporting a PASS when vdso_test_time cannot find the vdso symbol.
Cc: Vincenzo Frascino vincenzo.frascino@arm.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com
Seen as a failure on both a JUNO and a Dragonboard on both recent and old kernels/testruns:
root@deb-buster-arm64:~# /opt/ksft/vDSO/vdso_test_abi [vDSO kselftest] VDSO_VERSION: LINUX_2.6.39 The time is 1637922136.675304 The time is 1637922136.675361000 The resolution is 0 1 clock_id: CLOCK_REALTIME [PASS] The time is 1927.760604900 The resolution is 0 1 clock_id: CLOCK_BOOTTIME [PASS] The time is 1637922136.675649700 The resolution is 0 1 clock_id: CLOCK_TAI [PASS] The time is 1637922136.672000000 The resolution is 0 4000000 clock_id: CLOCK_REALTIME_COARSE [PASS] The time is 1927.761005600 The resolution is 0 1 clock_id: CLOCK_MONOTONIC [PASS] The time is 1927.761132780 The resolution is 0 1 clock_id: CLOCK_MONOTONIC_RAW [PASS] The time is 1927.757093740 The resolution is 0 4000000 clock_id: CLOCK_MONOTONIC_COARSE [PASS] Could not find __kernel_time <<< This caused a FAIL as a whole root@deb-buster-arm64:~# echo $? 1
e.g.: https://lkft.validation.linaro.org/scheduler/job/2192570#L27778
tools/testing/selftests/vDSO/vdso_test_abi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_abi.c b/tools/testing/selftests/vDSO/vdso_test_abi.c index 3d603f1394af..7dcc66d1cecf 100644 --- a/tools/testing/selftests/vDSO/vdso_test_abi.c +++ b/tools/testing/selftests/vDSO/vdso_test_abi.c @@ -90,8 +90,9 @@ static int vdso_test_time(void) (vdso_time_t)vdso_sym(version, name[2]); if (!vdso_time) {
/* Skip if symbol not found: consider skipped tests as passed */
return KSFT_SKIP;
return KSFT_PASS;
Skip is a the right option here. Pass indicates that the functionality has been tested and it passed. There is a clear message that says that the symbol isn't found
thanks, -- Shuah
On 1/26/22 3:27 AM, Cristian Marussi wrote:
Hi Shuah,
a small series collecting a few trivial fixes that I have already sent previously (~Dec2021) as distinct patches.
They are mostly trivial patches addressing failures that seemed more sensible to be marked as skips instead. (at least to me ...). Original developers are in Cc. (but not heard back from anyone :D)
Thanks, Cristian
Cristian Marussi (5): selftests: skip mincore.check_file_mmap when fs lacks needed support kselftest: Fix vdso_test_time to pass on skips selftests: openat2: Print also errno in failure messages selftests: openat2: Add missing dependency in Makefile selftests: openat2: Skip testcases that fail with EOPNOTSUPP
.../selftests/mincore/mincore_selftest.c | 20 +++++++++++++------ tools/testing/selftests/openat2/Makefile | 2 +- tools/testing/selftests/openat2/helpers.h | 12 ++++++----- .../testing/selftests/openat2/openat2_test.c | 12 ++++++++++- tools/testing/selftests/vDSO/vdso_test_abi.c | 3 ++- 5 files changed, 35 insertions(+), 14 deletions(-)
Thank you Cristian. Please see responses for individual patches.
thanks, -- Shuah
On 1/26/22 3:27 AM, Cristian Marussi wrote:
In E_func() macro, on error, print also errno in order to aid debugging.
Cc: Aleksa Sarai cyphar@cyphar.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com
tools/testing/selftests/openat2/helpers.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/openat2/helpers.h b/tools/testing/selftests/openat2/helpers.h index a6ea27344db2..ad5d0ba5b6ce 100644 --- a/tools/testing/selftests/openat2/helpers.h +++ b/tools/testing/selftests/openat2/helpers.h @@ -62,11 +62,12 @@ bool needs_openat2(const struct open_how *how); (similar to chroot(2)). */ #endif /* RESOLVE_IN_ROOT */ -#define E_func(func, ...) \
- do { \
if (func(__VA_ARGS__) < 0) \ksft_exit_fail_msg("%s:%d %s failed\n", \__FILE__, __LINE__, #func);\+#define E_func(func, ...) \
- do { \
errno = 0; \if (func(__VA_ARGS__) < 0) \ksft_exit_fail_msg("%s:%d %s failed - errno:%d\n", \__FILE__, __LINE__, #func, errno); \#define E_asprintf(...) E_func(asprintf, __VA_ARGS__)
Looks good. Will apply to linux-kselftest rc3
thanks, -- Shuah
On 1/26/22 3:27 AM, Cristian Marussi wrote:
Add a dependency on header helpers.h to the main target; while at that add to helpers.h also a missing include for bool types.
Cc: Aleksa Sarai cyphar@cyphar.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com
tools/testing/selftests/openat2/Makefile | 2 +- tools/testing/selftests/openat2/helpers.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/openat2/Makefile b/tools/testing/selftests/openat2/Makefile index 4b93b1417b86..843ba56d8e49 100644 --- a/tools/testing/selftests/openat2/Makefile +++ b/tools/testing/selftests/openat2/Makefile @@ -5,4 +5,4 @@ TEST_GEN_PROGS := openat2_test resolve_test rename_attack_test include ../lib.mk -$(TEST_GEN_PROGS): helpers.c +$(TEST_GEN_PROGS): helpers.c helpers.h diff --git a/tools/testing/selftests/openat2/helpers.h b/tools/testing/selftests/openat2/helpers.h index ad5d0ba5b6ce..7056340b9339 100644 --- a/tools/testing/selftests/openat2/helpers.h +++ b/tools/testing/selftests/openat2/helpers.h @@ -9,6 +9,7 @@ #define _GNU_SOURCE #include <stdint.h> +#include <stdbool.h> #include <errno.h> #include <linux/types.h> #include "../kselftest.h"
Thanks for the patch. Will apply for linux-kselftest fixes branch for rc3
thanks, -- Shuah
On 1/26/22 3:27 AM, Cristian Marussi wrote:
Skip testcases that fail since the requested valid flags combination is not supported by the underlying filesystem.
Cc: Aleksa Sarai cyphar@cyphar.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com
tools/testing/selftests/openat2/openat2_test.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c index 1bddbe934204..7fb902099de4 100644 --- a/tools/testing/selftests/openat2/openat2_test.c +++ b/tools/testing/selftests/openat2/openat2_test.c @@ -259,6 +259,16 @@ void test_openat2_flags(void) unlink(path); fd = sys_openat2(AT_FDCWD, path, &test->how);
if (fd < 0 && fd == -EOPNOTSUPP) {/** Skip the testcase if it failed because not supported* by FS. (e.g. a valid O_TMPFILE combination on NFS)*/ksft_test_result_skip("openat2 with %s fails with %d (%s)\n",test->name, fd, strerror(-fd));goto next;}- if (test->err >= 0) failed = (fd < 0); else
@@ -303,7 +313,7 @@ void test_openat2_flags(void) else resultfn("openat2 with %s fails with %d (%s)\n", test->name, test->err, strerror(-test->err));
+next: free(fdpath); fflush(stdout); }
Thanks for the patch. Will apply to linux-kselftest fixes branc for rc3
thanks, -- Shuah
On 1/26/22 5:26 AM, Vincenzo Frascino wrote:
vdso_test_abi contains a batch of tests that verify the validity of the vDSO ABI.
When a vDSO symbol is not found the relevant test is skipped reporting KSFT_SKIP. All the tests return values are then added in a single variable which is checked to verify failures. This approach can have side effects which result in reporting the wrong kselftest exit status.
Fix vdso_test_abi verifying the return code of each test separately.
Cc: Shuah Khan shuah@kernel.org Cc: Andy Lutomirski luto@kernel.org Cc: Thomas Gleixner tglx@linutronix.de Reported-by: Cristian Marussi cristian.marussi@arm.com Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com
tools/testing/selftests/vDSO/vdso_test_abi.c | 27 +++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/vDSO/vdso_test_abi.c b/tools/testing/selftests/vDSO/vdso_test_abi.c index 3d603f1394af..3a4efb91b9b2 100644 --- a/tools/testing/selftests/vDSO/vdso_test_abi.c +++ b/tools/testing/selftests/vDSO/vdso_test_abi.c @@ -184,10 +184,12 @@ static inline int vdso_test_clock(clockid_t clock_id) return ret0; } +#define VDSO_TESTS_MAX 9
- int main(int argc, char **argv) { unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
- int ret;
- int ret[VDSO_TESTS_MAX] = {0};
if (!sysinfo_ehdr) { printf("AT_SYSINFO_EHDR is not present!\n"); @@ -201,44 +203,45 @@ int main(int argc, char **argv) vdso_init_from_sysinfo_ehdr(getauxval(AT_SYSINFO_EHDR));
- ret = vdso_test_gettimeofday();
- ret[0] = vdso_test_gettimeofday();
#if _POSIX_TIMERS > 0 #ifdef CLOCK_REALTIME
- ret += vdso_test_clock(CLOCK_REALTIME);
- ret[1] = vdso_test_clock(CLOCK_REALTIME); #endif
#ifdef CLOCK_BOOTTIME
- ret += vdso_test_clock(CLOCK_BOOTTIME);
- ret[2] = vdso_test_clock(CLOCK_BOOTTIME); #endif
#ifdef CLOCK_TAI
- ret += vdso_test_clock(CLOCK_TAI);
- ret[3] = vdso_test_clock(CLOCK_TAI); #endif
#ifdef CLOCK_REALTIME_COARSE
- ret += vdso_test_clock(CLOCK_REALTIME_COARSE);
- ret[4] = vdso_test_clock(CLOCK_REALTIME_COARSE); #endif
#ifdef CLOCK_MONOTONIC
- ret += vdso_test_clock(CLOCK_MONOTONIC);
- ret[5] = vdso_test_clock(CLOCK_MONOTONIC); #endif
#ifdef CLOCK_MONOTONIC_RAW
- ret += vdso_test_clock(CLOCK_MONOTONIC_RAW);
- ret[6] = vdso_test_clock(CLOCK_MONOTONIC_RAW); #endif
#ifdef CLOCK_MONOTONIC_COARSE
- ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE);
- ret[7] = vdso_test_clock(CLOCK_MONOTONIC_COARSE); #endif
#endif
- ret += vdso_test_time();
- ret[8] = vdso_test_time();
- if (ret > 0)
return KSFT_FAIL;
- for (int i = 0; i < VDSO_TESTS_MAX; i++)
if (ret[i] == KSFT_FAIL)return KSFT_FAIL;return KSFT_PASS; }
You can use the ksft_* counts interfaces for this instead of adding counts here. ksft_test_result_*() can be used to increment the right result counters and then print counts at the end.
Either if there is a failure in any of the tests it will be fail with clear indication on which tests failed. vdso_test_clock() test for example is reporting false positives by overriding the Skip return with a pass.
thanks, -- Shuah
On 1/26/22 3:27 AM, Cristian Marussi wrote:
Report mincore.check_file_mmap as SKIP instead of FAIL if the underlying filesystem lacks support of O_TMPFILE or fallocate since such failures are not really related to mincore functionality.
Cc: Ricardo Cañuelo ricardo.canuelo@collabora.com Signed-off-by: Cristian Marussi cristian.marussi@arm.com
Thanks. Will apply to linux-kselftest fixes for rc3
thanks, -- Shuah
Hi Shuah,
On 1/27/22 11:18 PM, Shuah Khan wrote:
You can use the ksft_* counts interfaces for this instead of adding counts here. ksft_test_result_*() can be used to increment the right result counters and then print counts at the end.
Either if there is a failure in any of the tests it will be fail with clear indication on which tests failed. vdso_test_clock() test for example is reporting false positives by overriding the Skip return with a pass.
Good point. I missed one condition in updating the test. I will post v2 that will be compliant with the interface you mentioned.
Thanks.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Cristian Marussi -
Shuah Khan -
Vincenzo Frascino