[PATCH 0/5] Miscellaneous trivial fixes

[PATCH 0/5] Miscellaneous trivial fixes

[Cristian Marussi]

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(-)

-- 
2.17.1

[PATCH 1/5] selftests: skip mincore.check_file_mmap when fs lacks needed support

[Cristian Marussi]

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);
 }
 
-- 
2.17.1

Re: [PATCH 1/5] selftests: skip mincore.check_file_mmap when fs lacks needed support

[Shuah Khan]

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

[PATCH 2/5] kselftest: Fix vdso_test_time to pass on skips

[Cristian Marussi]

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);
-- 
2.17.1

Re: [PATCH 2/5] kselftest: Fix vdso_test_time to pass on skips

[Vincenzo Frascino]

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 */
>  		printf("Could not find %s\n", name[2]);
> -		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);
> 

-- 
Regards,
Vincenzo

Re: [PATCH 2/5] kselftest: Fix vdso_test_time to pass on skips

[Cristian Marussi]

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 */
> >  		printf("Could not find %s\n", name[2]);
> > -		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

Re: [PATCH 2/5] kselftest: Fix vdso_test_time to pass on skips

[Vincenzo Frascino]

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!

-- 
Regards,
Vincenzo

[PATCH] kselftest: Fix vdso_test_abi return status

[Vincenzo Frascino]

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;
 }
-- 
2.34.1

Re: [PATCH] kselftest: Fix vdso_test_abi return status

[Shuah Khan]

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

Re: [PATCH] kselftest: Fix vdso_test_abi return status

[Vincenzo Frascino]

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

-- 
Regards,
Vincenzo

Re: [PATCH 2/5] kselftest: Fix vdso_test_time to pass on skips

[Shuah Khan]

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 */
>   		printf("Could not find %s\n", name[2]);
> -		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

[PATCH 3/5] selftests: openat2: Print also errno in failure messages

[Cristian Marussi]

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__)
-- 
2.17.1

Re: [PATCH 3/5] selftests: openat2: Print also errno in failure messages

[Shuah Khan]

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); \
>   	} while (0)
>   
>   #define E_asprintf(...)		E_func(asprintf,	__VA_ARGS__)
> 

Looks good. Will apply to linux-kselftest rc3

thanks,
-- Shuah

[PATCH 4/5] selftests: openat2: Add missing dependency in Makefile

[Cristian Marussi]

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"
-- 
2.17.1

Re: [PATCH 4/5] selftests: openat2: Add missing dependency in Makefile

[Shuah Khan]

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

[PATCH 5/5] selftests: openat2: Skip testcases that fail with EOPNOTSUPP

[Cristian Marussi]

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);
 	}
-- 
2.17.1

Re: [PATCH 5/5] selftests: openat2: Skip testcases that fail with EOPNOTSUPP

[Shuah Khan]

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

Re: [PATCH 0/5] Miscellaneous trivial fixes

[Shuah Khan]

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