| Message ID | c5428a9c88d18fac80e364281cfd4e3aefa38d2c.1590130423.git.viresh.kumar@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | syscalls: Remove incorrect usage of libc structures | expand |
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
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 ?
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.
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.
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)
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(-)