[LTP] [PATCH V2 0/6] syscalls: Remove incorrect usage of libc structures

[LTP] [PATCH V2 0/6] syscalls: Remove incorrect usage of libc structures

[Viresh Kumar]

Hi,

It is incorrect to use the libc structures (timespec, timeval, timex),
to the non-time64 syscalls called via tst_syscall() as the kernel
expects the old structures in there and the libc structure definition
may change in the future.

V2:
- Use .restore_wallclock flag and remove unnecessary restoration code.
- Update tst_clock_*() syscall's implementation instead of its users.

Viresh Kumar (6):
  tst_safe_clocks: Remove safe_clock_adjtime()
  syscalls: settimeofday01: Set .restore_wallclock flag
  syscalls: settimeofday02: Remove time restoration code
  syscalls: settimeofday: Use gettimeofday()
  syscalls: Don't pass struct timespec to tst_syscall()
  syscalls: Don't pass struct timeval to tst_syscall()

 include/tst_safe_clocks.h                     | 18 ------
 include/tst_timer.h                           |  6 ++
 lib/tst_clocks.c                              | 59 ++++++++++++++++++-
 .../syscalls/clock_adjtime/clock_adjtime.h    |  5 --
 .../syscalls/gettimeofday/gettimeofday02.c    |  3 +-
 .../syscalls/settimeofday/settimeofday01.c    | 21 +------
 .../syscalls/settimeofday/settimeofday02.c    | 21 -------
 testcases/kernel/syscalls/stime/stime_var.h   |  3 +-
 8 files changed, 69 insertions(+), 67 deletions(-)

-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2 1/6] tst_safe_clocks: Remove safe_clock_adjtime()

[Viresh Kumar]

safe_clock_adjtime() isn't used anymore, remove it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/tst_safe_clocks.h | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/include/tst_safe_clocks.h b/include/tst_safe_clocks.h
index 27e8bda45589..4cb5f41ed82f 100644
--- a/include/tst_safe_clocks.h
+++ b/include/tst_safe_clocks.h
@@ -55,21 +55,6 @@ static inline void safe_clock_settime(const char *file, const int lineno,
 	}
 }
 
-static inline int safe_clock_adjtime(const char *file, const int lineno,
-	clockid_t clk_id, struct timex *txc)
-{
-	int rval;
-
-	rval = tst_syscall(__NR_clock_adjtime, clk_id, txc);
-	if (rval < 0) {
-		tst_brk(TBROK | TERRNO,
-			"%s:%d clock_adjtime(%s) failed %i",
-			file, lineno, tst_clock_name(clk_id), rval);
-	}
-
-	return rval;
-}
-
 #define SAFE_CLOCK_GETRES(clk_id, res)\
 	safe_clock_getres(__FILE__, __LINE__, (clk_id), (res))
 
@@ -79,7 +64,4 @@ static inline int safe_clock_adjtime(const char *file, const int lineno,
 #define SAFE_CLOCK_SETTIME(clk_id, tp)\
 	safe_clock_settime(__FILE__, __LINE__, (clk_id), (tp))
 
-#define SAFE_CLOCK_ADJTIME(clk_id, txc)\
-	safe_clock_adjtime(__FILE__, __LINE__, (clk_id), (txc))
-
 #endif /* SAFE_CLOCKS_H__ */
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2 2/6] syscalls: settimeofday01: Set .restore_wallclock flag

[Viresh Kumar]

Set the .restore_wallclock flag and get rid of some code.

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../syscalls/settimeofday/settimeofday01.c      | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/settimeofday/settimeofday01.c b/testcases/kernel/syscalls/settimeofday/settimeofday01.c
index 368fdebc0c8e..7ce3dc5a47b3 100644
--- a/testcases/kernel/syscalls/settimeofday/settimeofday01.c
+++ b/testcases/kernel/syscalls/settimeofday/settimeofday01.c
@@ -16,8 +16,6 @@
 #define ACCEPTABLE_DELTA 500
 #define USEC_PER_SEC    1000000L
 
-struct timeval tv_saved;
-
 static void verify_settimeofday(void)
 {
 	suseconds_t delta;
@@ -56,21 +54,8 @@ static void verify_settimeofday(void)
 		tst_res(TFAIL, "settimeofday() fail");
 }
 
-static void setup(void)
-{
-	if (tst_syscall(__NR_gettimeofday, &tv_saved, NULL) == -1)
-		tst_brk(TBROK | TERRNO, "gettimeofday(&tv_saved, NULL) failed");
-}
-
-static void cleanup(void)
-{
-	if ((settimeofday(&tv_saved, NULL)) == -1)
-		tst_brk(TBROK | TERRNO, "settimeofday(&tv_saved, NULL) failed");
-}
-
 static struct tst_test test = {
-	.setup = setup,
-	.cleanup = cleanup,
+	.restore_wallclock = 1,
 	.test_all = verify_settimeofday,
 	.needs_root = 1,
 };
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2 3/6] syscalls: settimeofday02: Remove time restoration code

[Viresh Kumar]

Unless the kernel is buggy, the system time shouldn't get updated by
this test and so there is no need to have the code to restore the clock.
Remove it.

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../syscalls/settimeofday/settimeofday02.c    | 21 -------------------
 1 file changed, 21 deletions(-)

diff --git a/testcases/kernel/syscalls/settimeofday/settimeofday02.c b/testcases/kernel/syscalls/settimeofday/settimeofday02.c
index 485a26b1d9c5..0fa8a147a212 100644
--- a/testcases/kernel/syscalls/settimeofday/settimeofday02.c
+++ b/testcases/kernel/syscalls/settimeofday/settimeofday02.c
@@ -12,9 +12,6 @@
 #include "tst_test.h"
 #include "lapi/syscalls.h"
 
-struct timeval tv_saved;
-static int flag;
-
 static struct tcase {
 	struct timeval tv;
 	int exp_errno;
@@ -29,12 +26,10 @@ static void verify_settimeofday(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 
-	flag = 0;
 	tst_res(TINFO, "%s", tc->message);
 	TEST(settimeofday(&tc->tv, NULL));
 	if (TST_RET != -1) {
 		tst_res(TFAIL, "settimeofday() succeeded unexpectedly");
-		flag = 1;
 		return;
 	}
 
@@ -44,23 +39,7 @@ static void verify_settimeofday(unsigned int n)
 		tst_res(TPASS | TTERRNO, "Received expected errno");
 }
 
-static void setup(void)
-{
-	if (tst_syscall(__NR_gettimeofday, &tv_saved, NULL) == -1)
-		tst_brk(TBROK | TERRNO, "gettimeofday(&tv_saved, NULL) failed");
-}
-
-static void cleanup(void)
-{
-	if (!flag)
-		return;
-	if ((settimeofday(&tv_saved, NULL)) == -1)
-		tst_brk(TBROK | TERRNO, "settimeofday(&tv_saved, NULL) failed");
-}
-
 static struct tst_test test = {
-	.setup = setup,
-	.cleanup = cleanup,
 	.test = verify_settimeofday,
 	.tcnt = ARRAY_SIZE(tcases),
 	.caps = (struct tst_cap []) {
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2 4/6] syscalls: settimeofday: Use gettimeofday()

[Viresh Kumar]

Passing struct timeval to __NR_gettimeofday syscall is incompatible and
may cause issues as it must only be used with the libc gettimeofday()
syscall. Use gettimeofday() instead of calling it with tst_syscall() to
fix that.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 testcases/kernel/syscalls/settimeofday/settimeofday01.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/settimeofday/settimeofday01.c b/testcases/kernel/syscalls/settimeofday/settimeofday01.c
index 7ce3dc5a47b3..b7f84b00fdc8 100644
--- a/testcases/kernel/syscalls/settimeofday/settimeofday01.c
+++ b/testcases/kernel/syscalls/settimeofday/settimeofday01.c
@@ -21,7 +21,7 @@ static void verify_settimeofday(void)
 	suseconds_t delta;
 	struct timeval tv1, tv2;
 
-	if (tst_syscall(__NR_gettimeofday, &tv1, NULL) == -1)
+	if (gettimeofday(&tv1, NULL) == -1)
 		tst_brk(TBROK | TERRNO, "gettimeofday(&tv1, NULL) failed");
 
 	tv1.tv_sec += VAL_SEC;
@@ -35,7 +35,7 @@ static void verify_settimeofday(void)
 		return;
 	}
 
-	if (tst_syscall(__NR_gettimeofday, &tv2, NULL) == -1)
+	if (gettimeofday(&tv2, NULL) == -1)
 		tst_brk(TBROK | TERRNO, "gettimeofday(&tv2, NULL) failed");
 
 	if (tv2.tv_sec > tv1.tv_sec) {
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2 4/6] syscalls: settimeofday: Use gettimeofday()

[Cyril Hrubis]

Hi!
I've pushed the patchset up to this patch, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V2 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Viresh Kumar]

There are compatibility issues here as we are calling the direct
syscalls (with tst_syscall()) with the "struct timespec" (which is a
libc definition). Over that, an architecture may not define
__NR_clock_getres (for example) and so we must have the fallback version
in place.

This updates the tst_clock_*() routines in core libraries and adds
support for different syscall variants.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 lib/tst_clocks.c | 59 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
index 2eaa73b11abe..ed13f0af0c60 100644
--- a/lib/tst_clocks.c
+++ b/lib/tst_clocks.c
@@ -7,23 +7,76 @@
 
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
+#include "tst_timer.h"
 #include "tst_clocks.h"
 #include "lapi/syscalls.h"
 #include "lapi/posix_clocks.h"
 
 int tst_clock_getres(clockid_t clk_id, struct timespec *res)
 {
-	return tst_syscall(__NR_clock_getres, clk_id, res);
+	int (*func)(clockid_t clk_id, void *ts);
+	struct tst_ts tts = { 0, };
+	int ret;
+
+#if defined(__NR_clock_getres_time64)
+	tts.type = TST_KERN_TIMESPEC;
+	func = sys_clock_getres64;
+#elif defined(__NR_clock_getres)
+	tts.type = TST_KERN_OLD_TIMESPEC;
+	func = sys_clock_getres;
+#else
+	tts.type = TST_LIBC_TIMESPEC;
+	func = libc_clock_getres;
+#endif
+
+	ret = func(clk_id, tst_ts_get(&tts));
+	res->tv_sec = tst_ts_get_sec(tts);
+	res->tv_nsec = tst_ts_get_nsec(tts);
+	return ret;
 }
 
 int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
 {
-	return tst_syscall(__NR_clock_gettime, clk_id, ts);
+	int (*func)(clockid_t clk_id, void *ts);
+	struct tst_ts tts = { 0, };
+	int ret;
+
+#if defined(__NR_clock_gettime64)
+	tts.type = TST_KERN_TIMESPEC;
+	func = sys_clock_gettime64;
+#elif defined(__NR_clock_gettime)
+	tts.type = TST_KERN_OLD_TIMESPEC;
+	func = sys_clock_gettime;
+#else
+	tts.type = TST_LIBC_TIMESPEC;
+	func = libc_clock_gettime;
+#endif
+
+	ret = func(clk_id, tst_ts_get(&tts));
+	ts->tv_sec = tst_ts_get_sec(tts);
+	ts->tv_nsec = tst_ts_get_nsec(tts);
+	return ret;
 }
 
 int tst_clock_settime(clockid_t clk_id, struct timespec *ts)
 {
-	return tst_syscall(__NR_clock_settime, clk_id, ts);
+	int (*func)(clockid_t clk_id, void *ts);
+	struct tst_ts tts = { 0, };
+
+#if defined(__NR_clock_settime64)
+	tts.type = TST_KERN_TIMESPEC;
+	func = sys_clock_settime64;
+#elif defined(__NR_clock_settime)
+	tts.type = TST_KERN_OLD_TIMESPEC;
+	func = sys_clock_settime;
+#else
+	tts.type = TST_LIBC_TIMESPEC;
+	func = libc_clock_settime;
+#endif
+
+	tst_ts_set_sec(&tts, ts->tv_sec);
+	tst_ts_set_nsec(&tts, ts->tv_nsec);
+	return func(clk_id, tst_ts_get(&tts));
 }
 
 const char *tst_clock_name(clockid_t clk_id)
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Arnd Bergmann]

On Fri, May 22, 2020 at 8:54 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:

>
>  int tst_clock_getres(clockid_t clk_id, struct timespec *res)
>  {
> -       return tst_syscall(__NR_clock_getres, clk_id, res);
> +       int (*func)(clockid_t clk_id, void *ts);
> +       struct tst_ts tts = { 0, };
> +       int ret;
> +
> +#if defined(__NR_clock_getres_time64)
> +       tts.type = TST_KERN_TIMESPEC;
> +       func = sys_clock_getres64;
> +#elif defined(__NR_clock_getres)
> +       tts.type = TST_KERN_OLD_TIMESPEC;
> +       func = sys_clock_getres;
> +#else
> +       tts.type = TST_LIBC_TIMESPEC;
> +       func = libc_clock_getres;
> +#endif
> +
> +       ret = func(clk_id, tst_ts_get(&tts));

This is not enough to run on old kernels that have __NR_clock_getres
but don't have __NR_clock_getres_time64, you need a runtime fallback
instead of a compile-time fallback.

As Cyril mentioned though, you don't need the libc fallback in
the end, since all kernels we would test can be expected to
have at least one of the other two.

       Arnd

[LTP] [PATCH V2 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Viresh Kumar]

On 22-05-20, 10:02, Arnd Bergmann wrote:
> On Fri, May 22, 2020 at 8:54 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> >
> >  int tst_clock_getres(clockid_t clk_id, struct timespec *res)
> >  {
> > -       return tst_syscall(__NR_clock_getres, clk_id, res);
> > +       int (*func)(clockid_t clk_id, void *ts);
> > +       struct tst_ts tts = { 0, };
> > +       int ret;
> > +
> > +#if defined(__NR_clock_getres_time64)
> > +       tts.type = TST_KERN_TIMESPEC;
> > +       func = sys_clock_getres64;
> > +#elif defined(__NR_clock_getres)
> > +       tts.type = TST_KERN_OLD_TIMESPEC;
> > +       func = sys_clock_getres;
> > +#else
> > +       tts.type = TST_LIBC_TIMESPEC;
> > +       func = libc_clock_getres;
> > +#endif
> > +
> > +       ret = func(clk_id, tst_ts_get(&tts));
> 
> This is not enough to run on old kernels that have __NR_clock_getres
> but don't have __NR_clock_getres_time64,

What about reversing the order of the two ? Check __NR_clock_getres
first ?

> you need a runtime fallback
> instead of a compile-time fallback.

Why so ?

-- 
viresh

[LTP] [PATCH V2 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Cyril Hrubis]

Hi!
> > This is not enough to run on old kernels that have __NR_clock_getres
> > but don't have __NR_clock_getres_time64,
> 
> What about reversing the order of the two ? Check __NR_clock_getres
> first ?

Moreover the __NR_ constants are always defined in order to avoid need
for excessive #ifdefs and the missing syscalls are defined to -1 in LTP.

So this will not work at all.

> > you need a runtime fallback
> > instead of a compile-time fallback.
> 
> Why so ?

Given that 32bit syscalls can be disabled in kernel config we cannot
really tell which ones are supported before we attempt to call the
syscall.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V2 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Cyril Hrubis]

Hi!
> > >  int tst_clock_getres(clockid_t clk_id, struct timespec *res)
> > >  {
> > > -       return tst_syscall(__NR_clock_getres, clk_id, res);
> > > +       int (*func)(clockid_t clk_id, void *ts);
> > > +       struct tst_ts tts = { 0, };
> > > +       int ret;
> > > +
> > > +#if defined(__NR_clock_getres_time64)
> > > +       tts.type = TST_KERN_TIMESPEC;
> > > +       func = sys_clock_getres64;
> > > +#elif defined(__NR_clock_getres)
> > > +       tts.type = TST_KERN_OLD_TIMESPEC;
> > > +       func = sys_clock_getres;
> > > +#else
> > > +       tts.type = TST_LIBC_TIMESPEC;
> > > +       func = libc_clock_getres;
> > > +#endif
> > > +
> > > +       ret = func(clk_id, tst_ts_get(&tts));
> > 
> > This is not enough to run on old kernels that have __NR_clock_getres
> > but don't have __NR_clock_getres_time64,
> 
> What about reversing the order of the two ? Check __NR_clock_getres
> first ?
>
> > you need a runtime fallback
> > instead of a compile-time fallback.
> 
> Why so ?

The existence of the __NR_... does not mean that particular syscall is
supported or even exists. As said previously LTP defines unimplemented
syscalls to -1 to avoid #ifdef hell.

Also even if the 64bit syscall is defined in headers on particular 32bit
platform calling it on old kernel will still fail because the
functionality is simply not there.

Hence we have to select the right function on the first call to the
tst_clock_* functions.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V3 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Viresh Kumar]

There are compatibility issues here as we are calling the direct
syscalls (with tst_syscall()) with the "struct timespec" (which is a
libc definition). Over that, an architecture may not define
__NR_clock_getres (for example) and so we must have the fallback version
in place.

This updates the tst_clock_*() routines in core libraries and adds
support for different syscall variants.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V3:
- Run the syscalls at least once to verify they are supported by the
  hardware.

 lib/tst_clocks.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 3 deletions(-)

diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
index 2eaa73b11abe..ddf54b903133 100644
--- a/lib/tst_clocks.c
+++ b/lib/tst_clocks.c
@@ -7,23 +7,107 @@
 
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
+#include "tst_timer.h"
 #include "tst_clocks.h"
 #include "lapi/syscalls.h"
 #include "lapi/posix_clocks.h"
 
+typedef int (*mysyscall)(clockid_t clk_id, void *ts);
+
+int syscall_supported_by_kernel(mysyscall func)
+{
+	int ret;
+
+	ret = func(0, NULL);
+	if (ret == ENOSYS)
+		return 0;
+
+	return 1;
+}
+
 int tst_clock_getres(clockid_t clk_id, struct timespec *res)
 {
-	return tst_syscall(__NR_clock_getres, clk_id, res);
+	static struct tst_ts tts = { 0, };
+	static mysyscall func;
+	int ret;
+
+#if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
+	if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
+		func = sys_clock_getres64;
+		tts.type = TST_KERN_TIMESPEC;
+	}
+#endif
+
+	if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
+		func = sys_clock_getres;
+		tts.type = TST_KERN_OLD_TIMESPEC;
+	}
+
+	if (!func) {
+		tst_res(TCONF, "clock_getres() not available");
+		return ENOSYS;
+	}
+
+	ret = func(clk_id, tst_ts_get(&tts));
+	res->tv_sec = tst_ts_get_sec(tts);
+	res->tv_nsec = tst_ts_get_nsec(tts);
+	return ret;
 }
 
 int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
 {
-	return tst_syscall(__NR_clock_gettime, clk_id, ts);
+	struct tst_ts tts = { 0, };
+	static mysyscall func;
+	int ret;
+
+#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
+	if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
+		func = sys_clock_gettime64;
+		tts.type = TST_KERN_TIMESPEC;
+	}
+#endif
+
+	if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
+		func = sys_clock_gettime;
+		tts.type = TST_KERN_OLD_TIMESPEC;
+	}
+
+	if (!func) {
+		tst_res(TCONF, "clock_gettime() not available");
+		return ENOSYS;
+	}
+
+	ret = func(clk_id, tst_ts_get(&tts));
+	ts->tv_sec = tst_ts_get_sec(tts);
+	ts->tv_nsec = tst_ts_get_nsec(tts);
+	return ret;
 }
 
 int tst_clock_settime(clockid_t clk_id, struct timespec *ts)
 {
-	return tst_syscall(__NR_clock_settime, clk_id, ts);
+	struct tst_ts tts = { 0, };
+	static mysyscall func;
+
+#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
+	if (!func && syscall_supported_by_kernel(sys_clock_settime64)) {
+		func = sys_clock_settime64;
+		tts.type = TST_KERN_TIMESPEC;
+	}
+#endif
+
+	if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
+		func = sys_clock_settime;
+		tts.type = TST_KERN_OLD_TIMESPEC;
+	}
+
+	if (!func) {
+		tst_res(TCONF, "clock_settime() not available");
+		return ENOSYS;
+	}
+
+	tst_ts_set_sec(&tts, ts->tv_sec);
+	tst_ts_set_nsec(&tts, ts->tv_nsec);
+	return func(clk_id, tst_ts_get(&tts));
 }
 
 const char *tst_clock_name(clockid_t clk_id)
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V3 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Arnd Bergmann]

On Wed, May 27, 2020 at 11:43 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> There are compatibility issues here as we are calling the direct
> syscalls (with tst_syscall()) with the "struct timespec" (which is a
> libc definition). Over that, an architecture may not define
> __NR_clock_getres (for example) and so we must have the fallback version
> in place.
>
> This updates the tst_clock_*() routines in core libraries and adds
> support for different syscall variants.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

I would have expected this to be simpler without going through struct
tst_ts, but the implementation looks correct, and I suppose this
is more consistent.

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

[LTP] [PATCH V3 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Cyril Hrubis]

Hi!
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V3:
> - Run the syscalls at least once to verify they are supported by the
>   hardware.
> 
>  lib/tst_clocks.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
> index 2eaa73b11abe..ddf54b903133 100644
> --- a/lib/tst_clocks.c
> +++ b/lib/tst_clocks.c
> @@ -7,23 +7,107 @@
>  
>  #define TST_NO_DEFAULT_MAIN
>  #include "tst_test.h"
> +#include "tst_timer.h"
>  #include "tst_clocks.h"
>  #include "lapi/syscalls.h"
>  #include "lapi/posix_clocks.h"
>  
> +typedef int (*mysyscall)(clockid_t clk_id, void *ts);
> +
> +int syscall_supported_by_kernel(mysyscall func)
> +{
> +	int ret;
> +
> +	ret = func(0, NULL);
> +	if (ret == ENOSYS)
> +		return 0;

I guess that we will get -1 here and errno == ENOSYS instead since the
tst_syscall() calls syscall() that passes the error in errno.

> +	return 1;
> +}
> +
>  int tst_clock_getres(clockid_t clk_id, struct timespec *res)
>  {
> -	return tst_syscall(__NR_clock_getres, clk_id, res);
> +	static struct tst_ts tts = { 0, };
> +	static mysyscall func;
> +	int ret;
> +
> +#if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
> +	if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
> +		func = sys_clock_getres64;
> +		tts.type = TST_KERN_TIMESPEC;
> +	}
> +#endif
> +
> +	if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
> +		func = sys_clock_getres;
> +		tts.type = TST_KERN_OLD_TIMESPEC;
> +	}
> +
> +	if (!func) {
> +		tst_res(TCONF, "clock_getres() not available");
> +		return ENOSYS;

Here as well, the callers expects the error in errno, so we have to set
the errno to ENOSYS and return -1 instead.

> +	}
> +
> +	ret = func(clk_id, tst_ts_get(&tts));
> +	res->tv_sec = tst_ts_get_sec(tts);
> +	res->tv_nsec = tst_ts_get_nsec(tts);
> +	return ret;
>  }
>  
>  int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
>  {
> -	return tst_syscall(__NR_clock_gettime, clk_id, ts);
> +	struct tst_ts tts = { 0, };
> +	static mysyscall func;
> +	int ret;
> +
> +#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
> +	if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
> +		func = sys_clock_gettime64;
> +		tts.type = TST_KERN_TIMESPEC;
> +	}
> +#endif
> +
> +	if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
> +		func = sys_clock_gettime;
> +		tts.type = TST_KERN_OLD_TIMESPEC;
> +	}
> +
> +	if (!func) {
> +		tst_res(TCONF, "clock_gettime() not available");
> +		return ENOSYS;

Here as well.

> +	}
> +
> +	ret = func(clk_id, tst_ts_get(&tts));
> +	ts->tv_sec = tst_ts_get_sec(tts);
> +	ts->tv_nsec = tst_ts_get_nsec(tts);
> +	return ret;
>  }
>
>  int tst_clock_settime(clockid_t clk_id, struct timespec *ts)
>  {
> -	return tst_syscall(__NR_clock_settime, clk_id, ts);
> +	struct tst_ts tts = { 0, };
> +	static mysyscall func;
> +
> +#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
> +	if (!func && syscall_supported_by_kernel(sys_clock_settime64)) {
> +		func = sys_clock_settime64;
> +		tts.type = TST_KERN_TIMESPEC;
> +	}
> +#endif
> +
> +	if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
> +		func = sys_clock_settime;
> +		tts.type = TST_KERN_OLD_TIMESPEC;
> +	}
> +
> +	if (!func) {
> +		tst_res(TCONF, "clock_settime() not available");
> +		return ENOSYS;

And here.

> +	}
> +
> +	tst_ts_set_sec(&tts, ts->tv_sec);
> +	tst_ts_set_nsec(&tts, ts->tv_nsec);
> +	return func(clk_id, tst_ts_get(&tts));
>  }


Other than that it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Viresh Kumar]

There are compatibility issues here as we are calling the direct
syscalls (with tst_syscall()) with the "struct timespec" (which is a
libc definition). Over that, an architecture may not define
__NR_clock_getres (for example) and so we must have the fallback version
in place.

This updates the tst_clock_*() routines in core libraries and adds
support for different syscall variants.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V4: Properly use return value and errno.

 lib/tst_clocks.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 90 insertions(+), 3 deletions(-)

diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
index 2eaa73b11abe..bc0bef273e52 100644
--- a/lib/tst_clocks.c
+++ b/lib/tst_clocks.c
@@ -7,23 +7,110 @@
 
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
+#include "tst_timer.h"
 #include "tst_clocks.h"
 #include "lapi/syscalls.h"
 #include "lapi/posix_clocks.h"
 
+typedef int (*mysyscall)(clockid_t clk_id, void *ts);
+
+int syscall_supported_by_kernel(mysyscall func)
+{
+	int ret;
+
+	ret = func(0, NULL);
+	if (ret == -1 && errno == ENOSYS)
+		return 0;
+
+	return 1;
+}
+
 int tst_clock_getres(clockid_t clk_id, struct timespec *res)
 {
-	return tst_syscall(__NR_clock_getres, clk_id, res);
+	static struct tst_ts tts = { 0, };
+	static mysyscall func;
+	int ret;
+
+#if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
+	if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
+		func = sys_clock_getres64;
+		tts.type = TST_KERN_TIMESPEC;
+	}
+#endif
+
+	if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
+		func = sys_clock_getres;
+		tts.type = TST_KERN_OLD_TIMESPEC;
+	}
+
+	if (!func) {
+		tst_res(TCONF, "clock_getres() not available");
+		errno = ENOSYS;
+		return -1;
+	}
+
+	ret = func(clk_id, tst_ts_get(&tts));
+	res->tv_sec = tst_ts_get_sec(tts);
+	res->tv_nsec = tst_ts_get_nsec(tts);
+	return ret;
 }
 
 int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
 {
-	return tst_syscall(__NR_clock_gettime, clk_id, ts);
+	struct tst_ts tts = { 0, };
+	static mysyscall func;
+	int ret;
+
+#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
+	if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
+		func = sys_clock_gettime64;
+		tts.type = TST_KERN_TIMESPEC;
+	}
+#endif
+
+	if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
+		func = sys_clock_gettime;
+		tts.type = TST_KERN_OLD_TIMESPEC;
+	}
+
+	if (!func) {
+		tst_res(TCONF, "clock_gettime() not available");
+		errno = ENOSYS;
+		return -1;
+	}
+
+	ret = func(clk_id, tst_ts_get(&tts));
+	ts->tv_sec = tst_ts_get_sec(tts);
+	ts->tv_nsec = tst_ts_get_nsec(tts);
+	return ret;
 }
 
 int tst_clock_settime(clockid_t clk_id, struct timespec *ts)
 {
-	return tst_syscall(__NR_clock_settime, clk_id, ts);
+	struct tst_ts tts = { 0, };
+	static mysyscall func;
+
+#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
+	if (!func && syscall_supported_by_kernel(sys_clock_settime64)) {
+		func = sys_clock_settime64;
+		tts.type = TST_KERN_TIMESPEC;
+	}
+#endif
+
+	if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
+		func = sys_clock_settime;
+		tts.type = TST_KERN_OLD_TIMESPEC;
+	}
+
+	if (!func) {
+		tst_res(TCONF, "clock_settime() not available");
+		errno = ENOSYS;
+		return -1;
+	}
+
+	tst_ts_set_sec(&tts, ts->tv_sec);
+	tst_ts_set_nsec(&tts, ts->tv_nsec);
+	return func(clk_id, tst_ts_get(&tts));
 }
 
 const char *tst_clock_name(clockid_t clk_id)
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Li Wang]

Hi Viresh,
Seems this patch involved a new regression:(.

Viresh Kumar <viresh.kumar@linaro.org> wrote:

...
>
> +typedef int (*mysyscall)(clockid_t clk_id, void *ts);
> +
> +int syscall_supported_by_kernel(mysyscall func)
> +{
> +       int ret;
> +
> +       ret = func(0, NULL);

+       if (ret == -1 && errno == ENOSYS)
> +               return 0;
> +
> +       return 1;
> +}
> ... }
>
>  int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
>  {
> -       return tst_syscall(__NR_clock_gettime, clk_id, ts);
> +       struct tst_ts tts = { 0, };
> +       static mysyscall func;
> +       int ret;
> +
> +#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
> +       if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
>

To invoke sys_clock_gettime64 here makes no chance to choose the correct
syscall version since tst_syscall() will exit directly when getting ENOSYS.

We got many tests TCONF like the mmap18 did as below:
-------------------
# uname -rm
5.8.0-rc2+ aarch64
# ./mmap18
tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
../include/tst_timer.h:214: CONF: syscall(403) __NR_clock_gettime64 not
supported

the function call trace:
-----------------------------
testrun()
get_time_ms
...
tst_clock_gettime
syscall_supported_by_kernel
sys_clock_gettime64
tst_syscall(__NR_clock_gettime64, ...)


---- syscalls/regen.sh -----
#define tst_syscall(NR, ...) ({ \\
        int tst_ret; \\
        if (NR == __LTP__NR_INVALID_SYSCALL) { \\
                errno = ENOSYS; \\
                tst_ret = -1; \\
        } else { \\
                tst_ret = syscall(NR, ##__VA_ARGS__); \\
        } \\
        if (tst_ret == -1 && errno == ENOSYS) { \\
                tst_brk(TCONF, "syscall(%d) " #NR " not supported", NR); \\
        } \\
        tst_ret; \\
})



> +               func = sys_clock_gettime64;
> +               tts.type = TST_KERN_TIMESPEC;
> +       }
> +#endif
> +
> +       if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
> +               func = sys_clock_gettime;
> +               tts.type = TST_KERN_OLD_TIMESPEC;
> +       }
> +
> +       if (!func) {
> +               tst_res(TCONF, "clock_gettime() not available");
> +               errno = ENOSYS;
> +               return -1;
> +       }
> +
> +       ret = func(clk_id, tst_ts_get(&tts));
> +       ts->tv_sec = tst_ts_get_sec(tts);
> +       ts->tv_nsec = tst_ts_get_nsec(tts);
> +       return ret;
>  }
>


Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200703/8d574a78/attachment.htm>

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Harish]

Hi,

+1. I am also facing the same issue with many similar tests.

# uname -rm
4.18.0-211.el8.ppc64le ppc64le

Regards,
Harish

On 7/3/20 3:25 PM, Li Wang wrote:
> Hi Viresh,
> Seems this patch involved a new regression:(.
>
> Viresh Kumar <viresh.kumar@linaro.org 
> <mailto:viresh.kumar@linaro.org>> wrote:
>
>     ...
>
>     +typedef int (*mysyscall)(clockid_t clk_id, void *ts);
>     +
>     +int syscall_supported_by_kernel(mysyscall func)
>     +{
>     +? ? ? ?int ret;
>     +
>     +? ? ? ?ret = func(0, NULL); 
>
>     +? ? ? ?if (ret == -1 && errno == ENOSYS)
>     +? ? ? ? ? ? ? ?return 0;
>     +
>     +? ? ? ?return 1;
>     +}
>     ...?}
>
>     ?int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
>     ?{
>     -? ? ? ?return tst_syscall(__NR_clock_gettime, clk_id, ts);
>     +? ? ? ?struct tst_ts tts = { 0, };
>     +? ? ? ?static mysyscall func;
>     +? ? ? ?int ret;
>     +
>     +#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
>     +? ? ? ?if (!func &&
>     syscall_supported_by_kernel(sys_clock_gettime64)) {
>
>
> To invoke?sys_clock_gettime64 here?makes no chance to choose the 
> correct syscall version since tst_syscall() will?exit directly when 
> getting ENOSYS.
>
> We got many tests TCONF like the mmap18 did as below:
> -------------------
> # uname -rm
> 5.8.0-rc2+ aarch64
> # ./mmap18
> tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
> ../include/tst_timer.h:214: CONF: syscall(403) __NR_clock_gettime64 
> not supported
>
> the function call trace:
> -----------------------------
> testrun()
> get_time_ms
> ...
> tst_clock_gettime
> syscall_supported_by_kernel
> sys_clock_gettime64
> tst_syscall(__NR_clock_gettime64, ...)
>
>
> ---- syscalls/regen.sh -----
> #define tst_syscall(NR, ...) ({ \\
> ? ? ? ? int tst_ret; \\
> ? ? ? ? if (NR == __LTP__NR_INVALID_SYSCALL) { \\
> ? ? ? ? ? ? ? ? errno = ENOSYS; \\
> ? ? ? ? ? ? ? ? tst_ret = -1; \\
> ? ? ? ? } else { \\
> ? ? ? ? ? ? ? ? tst_ret = syscall(NR, ##__VA_ARGS__); \\
> ? ? ? ? } \\
> ? ? ? ? if (tst_ret == -1 && errno == ENOSYS) { \\
> ? ? ? ? ? ? ? ? tst_brk(TCONF, "syscall(%d) " #NR " not supported", 
> NR); \\
> ? ? ? ? } \\
> ? ? ? ? tst_ret; \\
> })
>
>     +? ? ? ? ? ? ? ?func = sys_clock_gettime64;
>     +? ? ? ? ? ? ? ?tts.type = TST_KERN_TIMESPEC;
>     +? ? ? ?}
>     +#endif
>     +
>     +? ? ? ?if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
>     +? ? ? ? ? ? ? ?func = sys_clock_gettime;
>     +? ? ? ? ? ? ? ?tts.type = TST_KERN_OLD_TIMESPEC;
>     +? ? ? ?}
>     +
>     +? ? ? ?if (!func) {
>     +? ? ? ? ? ? ? ?tst_res(TCONF, "clock_gettime() not available");
>     +? ? ? ? ? ? ? ?errno = ENOSYS;
>     +? ? ? ? ? ? ? ?return -1;
>     +? ? ? ?}
>     +
>     +? ? ? ?ret = func(clk_id, tst_ts_get(&tts));
>     +? ? ? ?ts->tv_sec = tst_ts_get_sec(tts);
>     +? ? ? ?ts->tv_nsec = tst_ts_get_nsec(tts);
>     +? ? ? ?return ret;
>     ?}
>
>
> Regards,
> Li Wang
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200703/d81b89ee/attachment.htm>

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Cyril Hrubis]

Hi!
I guess that we need:

diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
index bc0bef273..c0727a34c 100644
--- a/lib/tst_clocks.c
+++ b/lib/tst_clocks.c
@@ -14,11 +14,11 @@
 
 typedef int (*mysyscall)(clockid_t clk_id, void *ts);
 
-int syscall_supported_by_kernel(mysyscall func)
+int syscall_supported_by_kernel(long sysnr)
 {
 	int ret;
 
-	ret = func(0, NULL);
+	ret = syscall(sysnr, func(0, NULL);
 	if (ret == -1 && errno == ENOSYS)
 		return 0;
 
@@ -32,13 +32,13 @@ int tst_clock_getres(clockid_t clk_id, struct timespec *res)
 	int ret;
 
 #if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
-	if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
+	if (!func && syscall_supported_by_kernel(__NR_clock_getres64)) {
 		func = sys_clock_getres64;
 		tts.type = TST_KERN_TIMESPEC;
 	}
 #endif
 
-	if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
+	if (!func && syscall_supported_by_kernel(__NR_clock_getres)) {
 		func = sys_clock_getres;
 		tts.type = TST_KERN_OLD_TIMESPEC;
 	}
@@ -62,13 +62,13 @@ int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
 	int ret;
 
 #if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
-	if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
+	if (!func && syscall_supported_by_kernel(__NR_clock_gettime64)) {
 		func = sys_clock_gettime64;
 		tts.type = TST_KERN_TIMESPEC;
 	}
 #endif
 
-	if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
+	if (!func && syscall_supported_by_kernel(__NR_clock_gettime)) {
 		func = sys_clock_gettime;
 		tts.type = TST_KERN_OLD_TIMESPEC;
 	}
@@ -91,13 +91,13 @@ int tst_clock_settime(clockid_t clk_id, struct timespec *ts)
 	static mysyscall func;
 
 #if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
-	if (!func && syscall_supported_by_kernel(sys_clock_settime64)) {
+	if (!func && syscall_supported_by_kernel(__NR_clock_settime64)) {
 		func = sys_clock_settime64;
 		tts.type = TST_KERN_TIMESPEC;
 	}
 #endif
 
-	if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
+	if (!func && syscall_supported_by_kernel(__NR_clock_settime)) {
 		func = sys_clock_settime;
 		tts.type = TST_KERN_OLD_TIMESPEC;
 	}

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Li Wang]

Hi Cyril,

On Fri, Jul 3, 2020 at 8:59 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> I guess that we need:
>

This method works for me, plz could you correct some typos as below.


>
> diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
> index bc0bef273..c0727a34c 100644
> --- a/lib/tst_clocks.c
> +++ b/lib/tst_clocks.c
> @@ -14,11 +14,11 @@
>
>  typedef int (*mysyscall)(clockid_t clk_id, void *ts);
>
> -int syscall_supported_by_kernel(mysyscall func)
> +int syscall_supported_by_kernel(long sysnr)
>  {
>         int ret;
>
> -       ret = func(0, NULL);
> +       ret = syscall(sysnr, func(0, NULL);
>

This line should be: ret = syscall(sysnr, 0, NULL);


>         if (ret == -1 && errno == ENOSYS)
>                 return 0;
>
> @@ -32,13 +32,13 @@ int tst_clock_getres(clockid_t clk_id, struct timespec
> *res)
>         int ret;
>
>  #if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
> -       if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
> +       if (!func && syscall_supported_by_kernel(__NR_clock_getres64)) {


if (!func && syscall_supported_by_kernel(__NR_clock_getres_time64 )) {


>

                func = sys_clock_getres64;
>                 tts.type = TST_KERN_TIMESPEC;
>         }
>  #endif
>
> -       if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
> +       if (!func && syscall_supported_by_kernel(__NR_clock_getres)) {
>                 func = sys_clock_getres;
>                 tts.type = TST_KERN_OLD_TIMESPEC;
>         }
> @@ -62,13 +62,13 @@ int tst_clock_gettime(clockid_t clk_id, struct
> timespec *ts)
>         int ret;
>
>  #if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
> -       if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
> +       if (!func && syscall_supported_by_kernel(__NR_clock_gettime64)) {
>                 func = sys_clock_gettime64;
>                 tts.type = TST_KERN_TIMESPEC;
>         }
>  #endif
>
> -       if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
> +       if (!func && syscall_supported_by_kernel(__NR_clock_gettime)) {
>                 func = sys_clock_gettime;
>                 tts.type = TST_KERN_OLD_TIMESPEC;
>         }
> @@ -91,13 +91,13 @@ int tst_clock_settime(clockid_t clk_id, struct
> timespec *ts)
>         static mysyscall func;
>
>  #if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
> -       if (!func && syscall_supported_by_kernel(sys_clock_settime64)) {
> +       if (!func && syscall_supported_by_kernel(__NR_clock_settime64)) {
>                 func = sys_clock_settime64;
>                 tts.type = TST_KERN_TIMESPEC;
>         }
>  #endif
>
> -       if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
> +       if (!func && syscall_supported_by_kernel(__NR_clock_settime)) {
>                 func = sys_clock_settime;
>                 tts.type = TST_KERN_OLD_TIMESPEC;
>         }
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
>

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200704/a2a1894b/attachment-0001.htm>

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Harish]

Hi,

I tried the suggested patch, but was unsuccessful in running the test. 
Here is my diff.

diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
index bc0bef273..7b465b1f6 100644
--- a/lib/tst_clocks.c
+++ b/lib/tst_clocks.c
@@ -14,11 +14,11 @@

 ?typedef int (*mysyscall)(clockid_t clk_id, void *ts);

-int syscall_supported_by_kernel(mysyscall func)
+int syscall_supported_by_kernel(long sysnr)
 ?{
 ???? int ret;

-??? ret = func(0, NULL);
+??? ret = syscall(sysnr, 0, NULL);
 ???? if (ret == -1 && errno == ENOSYS)
 ???? ??? return 0;

@@ -32,13 +32,13 @@ int tst_clock_getres(clockid_t clk_id, struct 
timespec *res)
 ???? int ret;

 ?#if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
-??? if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
+??? if (!func && syscall_supported_by_kernel(__NR_clock_getres_time64)) {
 ???? ??? func = sys_clock_getres64;
 ???? ??? tts.type = TST_KERN_TIMESPEC;
 ???? }
 ?#endif

-??? if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
+??? if (!func && syscall_supported_by_kernel(__NR_clock_getres)) {
 ???? ??? func = sys_clock_getres;
 ???? ??? tts.type = TST_KERN_OLD_TIMESPEC;
 ???? }
@@ -62,13 +62,13 @@ int tst_clock_gettime(clockid_t clk_id, struct 
timespec *ts)
 ???? int ret;

 ?#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
-??? if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
+??? if (!func && syscall_supported_by_kernel(__NR_clock_gettime64)) {
 ???? ??? func = sys_clock_gettime64;
 ???? ??? tts.type = TST_KERN_TIMESPEC;
 ???? }
 ?#endif

-??? if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
+??? if (!func && syscall_supported_by_kernel(__NR_clock_gettime)) {
 ???? ??? func = sys_clock_gettime;
 ???? ??? tts.type = TST_KERN_OLD_TIMESPEC;
 ???? }
@@ -91,13 +91,13 @@ int tst_clock_settime(clockid_t clk_id, struct 
timespec *ts)
 ???? static mysyscall func;

 ?#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
-??? if (!func && syscall_supported_by_kernel(sys_clock_settime64)) {
+??? if (!func && syscall_supported_by_kernel(__NR_clock_settime64)) {
 ???? ??? func = sys_clock_settime64;
 ???? ??? tts.type = TST_KERN_TIMESPEC;
 ???? }
 ?#endif

-??? if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
+??? if (!func && syscall_supported_by_kernel(__NR_clock_settime)) {
 ???? ??? func = sys_clock_settime;
 ???? ??? tts.type = TST_KERN_OLD_TIMESPEC;
 ???? }


$ ./runltp -s max_map_count
...
...
Running tests.......
<<<test_start>>>
tag=max_map_count stime=1594019344
cmdline="max_map_count -i 10"
contacts=""
analysis=exit
<<<test_output>>>
incrementing stop
tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
../include/tst_timer.h:214: CONF: syscall(403) __NR_clock_gettime64 not 
supported

Summary:
passed?? 0
failed?? 0
skipped? 1
warnings 0
<<<execution_status>>>
initiation_status="ok"
duration=0 termination_type=exited termination_id=32 corefile=no
cutime=0 cstime=0
<<<test_end>>>

Is there anything I am missing here? Thanks in advance.

Regards,
Harish

On 7/4/20 12:44 PM, Li Wang wrote:
> Hi Cyril,
>
> On Fri, Jul 3, 2020 at 8:59 PM Cyril Hrubis <chrubis@suse.cz 
> <mailto:chrubis@suse.cz>> wrote:
>
>     Hi!
>     I guess that we need:
>
>
> This method works for me, plz could you correct some typos as below.
>
>
>     diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
>     index bc0bef273..c0727a34c 100644
>     --- a/lib/tst_clocks.c
>     +++ b/lib/tst_clocks.c
>     @@ -14,11 +14,11 @@
>
>     ?typedef int (*mysyscall)(clockid_t clk_id, void *ts);
>
>     -int syscall_supported_by_kernel(mysyscall func)
>     +int syscall_supported_by_kernel(long sysnr)
>     ?{
>     ? ? ? ? int ret;
>
>     -? ? ? ?ret = func(0, NULL);
>     +? ? ? ?ret = syscall(sysnr, func(0, NULL);
>
> This line should be: ret = syscall(sysnr, 0, NULL);
>
>     ? ? ? ? if (ret == -1 && errno == ENOSYS)
>     ? ? ? ? ? ? ? ? return 0;
>
>     @@ -32,13 +32,13 @@ int tst_clock_getres(clockid_t clk_id, struct
>     timespec *res)
>     ? ? ? ? int ret;
>
>     ?#if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
>     -? ? ? ?if (!func &&
>     syscall_supported_by_kernel(sys_clock_getres64)) {
>     +? ? ? ?if (!func &&
>     syscall_supported_by_kernel(__NR_clock_getres64)) {
>
> if (!func && syscall_supported_by_kernel(__NR_clock_getres_time64 )) {
>
>     ? ? ? ? ? ? ? ? func = sys_clock_getres64;
>     ? ? ? ? ? ? ? ? tts.type = TST_KERN_TIMESPEC;
>     ? ? ? ? }
>     ?#endif
>
>     -? ? ? ?if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
>     +? ? ? ?if (!func && syscall_supported_by_kernel(__NR_clock_getres)) {
>     ? ? ? ? ? ? ? ? func = sys_clock_getres;
>     ? ? ? ? ? ? ? ? tts.type = TST_KERN_OLD_TIMESPEC;
>     ? ? ? ? }
>     @@ -62,13 +62,13 @@ int tst_clock_gettime(clockid_t clk_id, struct
>     timespec *ts)
>     ? ? ? ? int ret;
>
>     ?#if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
>     -? ? ? ?if (!func &&
>     syscall_supported_by_kernel(sys_clock_gettime64)) {
>     +? ? ? ?if (!func &&
>     syscall_supported_by_kernel(__NR_clock_gettime64)) {
>     ? ? ? ? ? ? ? ? func = sys_clock_gettime64;
>     ? ? ? ? ? ? ? ? tts.type = TST_KERN_TIMESPEC;
>     ? ? ? ? }
>     ?#endif
>
>     -? ? ? ?if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
>     +? ? ? ?if (!func &&
>     syscall_supported_by_kernel(__NR_clock_gettime)) {
>     ? ? ? ? ? ? ? ? func = sys_clock_gettime;
>     ? ? ? ? ? ? ? ? tts.type = TST_KERN_OLD_TIMESPEC;
>     ? ? ? ? }
>     @@ -91,13 +91,13 @@ int tst_clock_settime(clockid_t clk_id, struct
>     timespec *ts)
>     ? ? ? ? static mysyscall func;
>
>     ?#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
>     -? ? ? ?if (!func &&
>     syscall_supported_by_kernel(sys_clock_settime64)) {
>     +? ? ? ?if (!func &&
>     syscall_supported_by_kernel(__NR_clock_settime64)) {
>     ? ? ? ? ? ? ? ? func = sys_clock_settime64;
>     ? ? ? ? ? ? ? ? tts.type = TST_KERN_TIMESPEC;
>     ? ? ? ? }
>     ?#endif
>
>     -? ? ? ?if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
>     +? ? ? ?if (!func &&
>     syscall_supported_by_kernel(__NR_clock_settime)) {
>     ? ? ? ? ? ? ? ? func = sys_clock_settime;
>     ? ? ? ? ? ? ? ? tts.type = TST_KERN_OLD_TIMESPEC;
>     ? ? ? ? }
>
>     -- 
>     Cyril Hrubis
>     chrubis@suse.cz <mailto:chrubis@suse.cz>
>
>
>
> -- 
> Regards,
> Li Wang
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200706/df87a1ec/attachment-0001.htm>

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Li Wang]

Harish <harish@linux.ibm.com> wrote:

> Hi,
>
> I tried the suggested patch, but was unsuccessful in running the test.
> Here is my diff.
> ...
> Is there anything I am missing here? Thanks in advance.
>
Your diff version looks correct.

I doubt have you rebuild your LTP or at least rebuild the ltp-lib?

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200706/5a8c3440/attachment.htm>

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Harish]

On 7/6/20 2:27 PM, Li Wang wrote:
>
> Harish <harish@linux.ibm.com <mailto:harish@linux.ibm.com>> wrote:
>
>     Hi,
>
>     I tried the suggested patch, but was unsuccessful in running the
>     test. Here is my diff.
>     ...
>     Is there anything I am missing here? Thanks in advance.
>
> Your diff version looks correct.
>
> I doubt have you rebuild your LTP or at least rebuild the ltp-lib?
My bad, I did rebuild the ltp with the diff. But it required a clean 
before I do so.
I can confirm tests are running now.

Thanks.
>
> -- 
> Regards,
> Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200706/cb163167/attachment.htm>

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Cyril Hrubis]

Hi!
> > diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
> > index bc0bef273..c0727a34c 100644
> > --- a/lib/tst_clocks.c
> > +++ b/lib/tst_clocks.c
> > @@ -14,11 +14,11 @@
> >
> >  typedef int (*mysyscall)(clockid_t clk_id, void *ts);
> >
> > -int syscall_supported_by_kernel(mysyscall func)
> > +int syscall_supported_by_kernel(long sysnr)
> >  {
> >         int ret;
> >
> > -       ret = func(0, NULL);
> > +       ret = syscall(sysnr, func(0, NULL);
> >
> 
> This line should be: ret = syscall(sysnr, 0, NULL);

This is obvious typo, sorry.

> >         if (ret == -1 && errno == ENOSYS)
> >                 return 0;
> >
> > @@ -32,13 +32,13 @@ int tst_clock_getres(clockid_t clk_id, struct timespec
> > *res)
> >         int ret;
> >
> >  #if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
> > -       if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
> > +       if (!func && syscall_supported_by_kernel(__NR_clock_getres64)) {
> 
> 
> if (!func && syscall_supported_by_kernel(__NR_clock_getres_time64 )) {

Huh, how come the syscall is called clock_getres_time64 while the rest
has only 64 appended such as clock_gettime64 and clock_settime64?

That's really strange...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Viresh Kumar]

On 07-07-20, 11:03, Cyril Hrubis wrote:
> Huh, how come the syscall is called clock_getres_time64 while the rest
> has only 64 appended such as clock_gettime64 and clock_settime64?
> 
> That's really strange...

That also made me wonder on how should I be naming routines.
Apparently they wanted to have "time64" in the name, if the syscall
already has "time" in it they just appended 64, else added "_time64".

-- 
viresh

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Cyril Hrubis]

Hi!
> > Huh, how come the syscall is called clock_getres_time64 while the rest
> > has only 64 appended such as clock_gettime64 and clock_settime64?
> > 
> > That's really strange...
> 
> That also made me wonder on how should I be naming routines.
> Apparently they wanted to have "time64" in the name, if the syscall
> already has "time" in it they just appended 64, else added "_time64".

If that is the case in upstream we should follow that convence for the
functions as well...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Viresh Kumar]

On 07-07-20, 13:49, Cyril Hrubis wrote:
> Hi!
> > > Huh, how come the syscall is called clock_getres_time64 while the rest
> > > has only 64 appended such as clock_gettime64 and clock_settime64?
> > > 
> > > That's really strange...
> > 
> > That also made me wonder on how should I be naming routines.
> > Apparently they wanted to have "time64" in the name, if the syscall
> > already has "time" in it they just appended 64, else added "_time64".
> 
> If that is the case in upstream we should follow that convence for the
> functions as well...

That's what I tried to do in my patches normally.

-- 
viresh

[LTP] [PATCH V4 5/6] syscalls: Don't pass struct timespec to tst_syscall()

[Viresh Kumar]

On 03-07-20, 14:59, Cyril Hrubis wrote:
> Hi!
> I guess that we need:
> 
> diff --git a/lib/tst_clocks.c b/lib/tst_clocks.c
> index bc0bef273..c0727a34c 100644
> --- a/lib/tst_clocks.c
> +++ b/lib/tst_clocks.c
> @@ -14,11 +14,11 @@
>  
>  typedef int (*mysyscall)(clockid_t clk_id, void *ts);
>  
> -int syscall_supported_by_kernel(mysyscall func)
> +int syscall_supported_by_kernel(long sysnr)
>  {
>  	int ret;
>  
> -	ret = func(0, NULL);
> +	ret = syscall(sysnr, func(0, NULL);
>  	if (ret == -1 && errno == ENOSYS)
>  		return 0;
>  
> @@ -32,13 +32,13 @@ int tst_clock_getres(clockid_t clk_id, struct timespec *res)
>  	int ret;
>  
>  #if (__NR_clock_getres_time64 != __LTP__NR_INVALID_SYSCALL)
> -	if (!func && syscall_supported_by_kernel(sys_clock_getres64)) {
> +	if (!func && syscall_supported_by_kernel(__NR_clock_getres64)) {
>  		func = sys_clock_getres64;
>  		tts.type = TST_KERN_TIMESPEC;
>  	}
>  #endif
>  
> -	if (!func && syscall_supported_by_kernel(sys_clock_getres)) {
> +	if (!func && syscall_supported_by_kernel(__NR_clock_getres)) {
>  		func = sys_clock_getres;
>  		tts.type = TST_KERN_OLD_TIMESPEC;
>  	}
> @@ -62,13 +62,13 @@ int tst_clock_gettime(clockid_t clk_id, struct timespec *ts)
>  	int ret;
>  
>  #if (__NR_clock_gettime64 != __LTP__NR_INVALID_SYSCALL)
> -	if (!func && syscall_supported_by_kernel(sys_clock_gettime64)) {
> +	if (!func && syscall_supported_by_kernel(__NR_clock_gettime64)) {
>  		func = sys_clock_gettime64;
>  		tts.type = TST_KERN_TIMESPEC;
>  	}
>  #endif
>  
> -	if (!func && syscall_supported_by_kernel(sys_clock_gettime)) {
> +	if (!func && syscall_supported_by_kernel(__NR_clock_gettime)) {
>  		func = sys_clock_gettime;
>  		tts.type = TST_KERN_OLD_TIMESPEC;
>  	}
> @@ -91,13 +91,13 @@ int tst_clock_settime(clockid_t clk_id, struct timespec *ts)
>  	static mysyscall func;
>  
>  #if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
> -	if (!func && syscall_supported_by_kernel(sys_clock_settime64)) {
> +	if (!func && syscall_supported_by_kernel(__NR_clock_settime64)) {
>  		func = sys_clock_settime64;
>  		tts.type = TST_KERN_TIMESPEC;
>  	}
>  #endif
>  
> -	if (!func && syscall_supported_by_kernel(sys_clock_settime)) {
> +	if (!func && syscall_supported_by_kernel(__NR_clock_settime)) {
>  		func = sys_clock_settime;
>  		tts.type = TST_KERN_OLD_TIMESPEC;
>  	}

Thanks, this will do.

-- 
viresh

[LTP] [PATCH V2 6/6] syscalls: Don't pass struct timeval to tst_syscall()

[Viresh Kumar]

There are compatibility issues here as we are calling the direct
syscalls with the "struct timeval" (which is a libc definition). We
must use struct __kernel_old_timeval instead.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/tst_timer.h                                     | 6 ++++++
 testcases/kernel/syscalls/clock_adjtime/clock_adjtime.h | 5 -----
 testcases/kernel/syscalls/gettimeofday/gettimeofday02.c | 3 ++-
 testcases/kernel/syscalls/stime/stime_var.h             | 3 ++-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/tst_timer.h b/include/tst_timer.h
index 256e1d71e1bc..62a0833b6cd9 100644
--- a/include/tst_timer.h
+++ b/include/tst_timer.h
@@ -12,6 +12,7 @@
 #ifndef TST_TIMER
 #define TST_TIMER
 
+#include <asm/posix_types.h>
 #include <sys/time.h>
 #include <time.h>
 #include "tst_test.h"
@@ -101,6 +102,11 @@ typedef long __kernel_long_t;
 
 typedef __kernel_long_t	__kernel_old_time_t;
 
+struct __kernel_old_timeval {
+	__kernel_old_time_t	tv_sec;		/* seconds */
+	__kernel_suseconds_t	tv_usec;	/* microseconds */
+};
+
 struct __kernel_old_timespec {
 	__kernel_old_time_t	tv_sec;		/* seconds */
 	__kernel_old_time_t	tv_nsec;	/* nanoseconds */
diff --git a/testcases/kernel/syscalls/clock_adjtime/clock_adjtime.h b/testcases/kernel/syscalls/clock_adjtime/clock_adjtime.h
index eb60f707f776..dbe0a561a3ab 100644
--- a/testcases/kernel/syscalls/clock_adjtime/clock_adjtime.h
+++ b/testcases/kernel/syscalls/clock_adjtime/clock_adjtime.h
@@ -18,11 +18,6 @@
 #include "lapi/timex.h"
 
 #ifndef __kernel_timex
-struct __kernel_old_timeval {
-	__kernel_old_time_t	tv_sec;		/* seconds */
-	__kernel_suseconds_t	tv_usec;	/* microseconds */
-};
-
 struct __kernel_old_timex {
 	unsigned int modes;	/* mode selector */
 	__kernel_long_t offset;	/* time offset (usec) */
diff --git a/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c b/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c
index b7687468d39d..b73bf129b116 100644
--- a/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c
+++ b/testcases/kernel/syscalls/gettimeofday/gettimeofday02.c
@@ -21,6 +21,7 @@
 #include <errno.h>
 
 #include "tst_test.h"
+#include "tst_timer.h"
 #include "lapi/syscalls.h"
 
 static volatile sig_atomic_t done;
@@ -39,7 +40,7 @@ static void breakout(int sig)
 
 static void verify_gettimeofday(void)
 {
-	struct timeval tv1, tv2;
+	struct __kernel_old_timeval tv1, tv2;
 	unsigned long long cnt = 0;
 
 	done = 0;
diff --git a/testcases/kernel/syscalls/stime/stime_var.h b/testcases/kernel/syscalls/stime/stime_var.h
index b33c5704e94a..708b80573167 100644
--- a/testcases/kernel/syscalls/stime/stime_var.h
+++ b/testcases/kernel/syscalls/stime/stime_var.h
@@ -9,6 +9,7 @@
 
 #include <sys/time.h>
 #include "config.h"
+#include "tst_timer.h"
 #include "lapi/syscalls.h"
 
 #define TEST_VARIANTS 3
@@ -26,7 +27,7 @@ static int do_stime(time_t *ntime)
 	case 1:
 		return tst_syscall(__NR_stime, ntime);
 	case 2: {
-		struct timeval tv;
+		struct __kernel_old_timeval tv;
 
 		tv.tv_sec = *ntime;
 		tv.tv_usec = 0;
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH V2 6/6] syscalls: Don't pass struct timeval to tst_syscall()

[Cyril Hrubis]

Hi!
> +#include <asm/posix_types.h>

I've removed this since the header already includes lapi/posix_types.h
and pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz