| Message ID | 9621b4f97b539f2e080b00491eb9ba4973878028.1591760262.git.viresh.kumar@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V6,01/17] syscalls/timer_gettime: Add support for time64 tests | expand |
Hi! > This adds support for time64 tests to the existing timer_gettime() > syscall tests. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > V6: Drop the binary files included by mistake. > > include/tst_timer.h | 45 +++++++ > .../syscalls/timer_gettime/timer_gettime01.c | 124 ++++++++---------- > 2 files changed, 97 insertions(+), 72 deletions(-) > > diff --git a/include/tst_timer.h b/include/tst_timer.h > index 256e1d71e1bc..708a1e9e9a7a 100644 > --- a/include/tst_timer.h > +++ b/include/tst_timer.h > @@ -15,6 +15,7 @@ > #include <sys/time.h> > #include <time.h> > #include "tst_test.h" > +#include "lapi/common_timers.h" > #include "lapi/syscalls.h" > > /* > @@ -112,6 +113,16 @@ struct __kernel_timespec { > __kernel_time64_t tv_sec; /* seconds */ > long long tv_nsec; /* nanoseconds */ > }; > + > +struct __kernel_old_itimerspec { > + struct __kernel_old_timespec it_interval; /* timer period */ > + struct __kernel_old_timespec it_value; /* timer expiration */ > +}; > + > +struct __kernel_itimerspec { > + struct __kernel_timespec it_interval; /* timer period */ > + struct __kernel_timespec it_value; /* timer expiration */ > +}; > #endif > > enum tst_ts_type { > @@ -129,6 +140,14 @@ struct tst_ts { > } ts; > }; > > +struct tst_its { > + enum tst_ts_type type; > + union { > + struct __kernel_old_itimerspec kern_old_its; > + struct __kernel_itimerspec kern_its; > + } ts; > +}; > + > static inline void *tst_ts_get(struct tst_ts *t) > { > if (!t) > @@ -147,6 +166,22 @@ static inline void *tst_ts_get(struct tst_ts *t) > } > } > > +static inline void *tst_its_get(struct tst_its *t) > +{ > + if (!t) > + return NULL; > + > + switch (t->type) { > + case TST_KERN_OLD_TIMESPEC: > + return &t->ts.kern_old_its; > + case TST_KERN_TIMESPEC: > + return &t->ts.kern_its; > + default: > + tst_brk(TBROK, "Invalid type: %d", t->type); > + return NULL; > + } > +} > + > static inline int libc_clock_getres(clockid_t clk_id, void *ts) > { > return clock_getres(clk_id, ts); > @@ -212,6 +247,16 @@ static inline int sys_clock_nanosleep64(clockid_t clk_id, int flags, > request, remain); > } > > +static inline int sys_timer_gettime(timer_t timerid, void *its) > +{ > + return tst_syscall(__NR_timer_gettime, timerid, its); > +} > + > +static inline int sys_timer_gettime64(timer_t timerid, void *its) > +{ > + return tst_syscall(__NR_timer_gettime64, timerid, its); > +} > + > /* > * Returns tst_ts seconds. > */ > diff --git a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c > index 1c75f1cf0e45..d2b89eab4223 100644 > --- a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c > +++ b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c > @@ -1,24 +1,5 @@ > -/****************************************************************************** > - * Copyright (c) Crackerjack Project., 2007 * > - * Porting from Crackerjack to LTP is done by: * > - * Manas Kumar Nayak <maknayak@in.ibm.com> * > - * Copyright (c) 2013 Cyril Hrubis <chrubis@suse.cz> * > - * * > - * This program is free software; you can redistribute it and/or modify * > - * it under the terms of the GNU General Public License as published by * > - * the Free Software Foundation; either version 2 of the License, or * > - * (at your option) any later version. * > - * * > - * This program is distributed in the hope that it will be useful, * > - * but WITHOUT ANY WARRANTY; without even the implied warranty of * > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See * > - * the GNU General Public License for more details. * > - * * > - * You should have received a copy of the GNU General Public License * > - * along with this program; if not, write to the Free Software Foundation, * > - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * > - * * > - ******************************************************************************/ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* Copyright (c) Crackerjack Project., 2007 */ You have dropped two copyright from there... > #include <time.h> > #include <signal.h> > @@ -26,71 +7,70 @@ > #include <stdio.h> > #include <errno.h> > > -#include "test.h" > -#include "lapi/syscalls.h" > +#include "tst_timer.h" > > -char *TCID = "timer_gettime01"; > -int TST_TOTAL = 3; > +static struct test_variants { > + int (*func)(timer_t timer, void *its); > + enum tst_ts_type type; > + char *desc; > +} variants[] = { > +#if (__NR_timer_gettime != __LTP__NR_INVALID_SYSCALL) > + { .func = sys_timer_gettime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"}, > +#endif > > -static void cleanup(void) > -{ > - tst_rmdir(); > -} > +#if (__NR_timer_gettime64 != __LTP__NR_INVALID_SYSCALL) > + { .func = sys_timer_gettime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"}, > +#endif > +}; > > -static void setup(void) > -{ > - TEST_PAUSE; > - tst_tmpdir(); > -} > +static timer_t timer; > > -int main(int ac, char **av) > +static void setup(void) > { > - int lc; > - > struct sigevent ev; > - struct itimerspec spec; > - int timer; > > - tst_parse_opts(ac, av, NULL, NULL); > - > - setup(); > + tst_res(TINFO, "Testing variant: %s", variants[tst_variant].desc); > > ev.sigev_value = (union sigval) 0; > ev.sigev_signo = SIGALRM; > ev.sigev_notify = SIGEV_SIGNAL; > - TEST(ltp_syscall(__NR_timer_create, CLOCK_REALTIME, &ev, &timer)); > - > - if (TEST_RETURN != 0) > - tst_brkm(TBROK | TERRNO, cleanup, "Failed to create timer"); > > - for (lc = 0; TEST_LOOPING(lc); ++lc) { > - tst_count = 0; > + TEST(tst_syscall(__NR_timer_create, CLOCK_REALTIME, &ev, &timer)); > > - TEST(ltp_syscall(__NR_timer_gettime, timer, &spec)); > - if (TEST_RETURN == 0) { > - tst_resm(TPASS, "timer_gettime(CLOCK_REALTIME) Passed"); > - } else { > - tst_resm(TFAIL | TERRNO, > - "timer_gettime(CLOCK_REALTIME) Failed"); > - } > - > - TEST(ltp_syscall(__NR_timer_gettime, -1, &spec)); > - if (TEST_RETURN == -1 && TEST_ERRNO == EINVAL) { > - tst_resm(TPASS, "timer_gettime(-1) Failed: EINVAL"); > - } else { > - tst_resm(TFAIL | TERRNO, > - "timer_gettime(-1) = %li", TEST_RETURN); > - } > + if (TST_RET) { > + tst_res(TFAIL | TTERRNO, "timer_create() failed"); > + return; > + } > +} > > - TEST(ltp_syscall(__NR_timer_gettime, timer, NULL)); > - if (TEST_RETURN == -1 && TEST_ERRNO == EFAULT) { > - tst_resm(TPASS, "timer_gettime(NULL) Failed: EFAULT"); > - } else { > - tst_resm(TFAIL | TERRNO, > - "timer_gettime(-1) = %li", TEST_RETURN); > - } > +static void verify(void) > +{ > + struct test_variants *tv = &variants[tst_variant]; > + struct tst_its spec = {.type = tv->type, }; > + > + TEST(tv->func(timer, tst_its_get(&spec))); > + if (TST_RET == 0) { > + tst_res(TPASS, "timer_gettime() Passed"); > + } else { > + tst_res(TFAIL | TTERRNO, "timer_gettime() Failed"); > } Looking at manuals it seems that: * Newly created timer is disarmed * For disarmed timers the clock_gettime() should get itimer with zeroes So we check here that the returned itimer has zeroes for both oneshoot and interval timers here. Other than that it looks good.
On 24-06-20, 16:23, Cyril Hrubis wrote: > > +static void verify(void) > > +{ > > + struct test_variants *tv = &variants[tst_variant]; > > + struct tst_its spec = {.type = tv->type, }; > > + > > + TEST(tv->func(timer, tst_its_get(&spec))); > > + if (TST_RET == 0) { > > + tst_res(TPASS, "timer_gettime() Passed"); > > + } else { > > + tst_res(TFAIL | TTERRNO, "timer_gettime() Failed"); > > } > > Looking at manuals it seems that: > > * Newly created timer is disarmed > > * For disarmed timers the clock_gettime() should get itimer with zeroes > > So we check here that the returned itimer has zeroes for both oneshoot > and interval timers here. I am not sure what oneshoot/interval timers you are talking about :) This is what I came to, is this sufficient ? diff --git a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c index ea7cc6b59f7e..4a949486a920 100644 --- a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c +++ b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c @@ -55,7 +55,19 @@ static void verify(void) TEST(tv->func(timer, tst_its_get(&spec))); if (TST_RET == 0) { - tst_res(TPASS, "timer_gettime() Passed"); + if ((spec.type == TST_KERN_OLD_TIMESPEC && + (spec.ts.kern_old_its.it_interval.tv_sec || + spec.ts.kern_old_its.it_interval.tv_nsec || + spec.ts.kern_old_its.it_value.tv_sec || + spec.ts.kern_old_its.it_value.tv_nsec)) || + (spec.type == TST_KERN_TIMESPEC && + (spec.ts.kern_its.it_interval.tv_sec || + spec.ts.kern_its.it_interval.tv_nsec || + spec.ts.kern_its.it_value.tv_sec || + spec.ts.kern_its.it_value.tv_nsec))) + tst_res(TFAIL, "timespec should have been zeroed"); + else + tst_res(TPASS, "timer_gettime() Passed"); } else { tst_res(TFAIL | TTERRNO, "timer_gettime() Failed"); }
Hi! > > > +static void verify(void) > > > +{ > > > + struct test_variants *tv = &variants[tst_variant]; > > > + struct tst_its spec = {.type = tv->type, }; > > > + > > > + TEST(tv->func(timer, tst_its_get(&spec))); > > > + if (TST_RET == 0) { > > > + tst_res(TPASS, "timer_gettime() Passed"); > > > + } else { > > > + tst_res(TFAIL | TTERRNO, "timer_gettime() Failed"); > > > } > > > > Looking at manuals it seems that: > > > > * Newly created timer is disarmed > > > > * For disarmed timers the clock_gettime() should get itimer with zeroes > > > > So we check here that the returned itimer has zeroes for both oneshoot > > and interval timers here. > > I am not sure what oneshoot/interval timers you are talking about :) These are the two timerspec structures inside of the itimerspec, one of them is for oneshoot timer/alarm and the second one is for reocurring timer/alarm and called interval. > This is what I came to, is this sufficient ? > > diff --git a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c > index ea7cc6b59f7e..4a949486a920 100644 > --- a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c > +++ b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c > @@ -55,7 +55,19 @@ static void verify(void) > > TEST(tv->func(timer, tst_its_get(&spec))); > if (TST_RET == 0) { > - tst_res(TPASS, "timer_gettime() Passed"); > + if ((spec.type == TST_KERN_OLD_TIMESPEC && > + (spec.ts.kern_old_its.it_interval.tv_sec || > + spec.ts.kern_old_its.it_interval.tv_nsec || > + spec.ts.kern_old_its.it_value.tv_sec || > + spec.ts.kern_old_its.it_value.tv_nsec)) || > + (spec.type == TST_KERN_TIMESPEC && > + (spec.ts.kern_its.it_interval.tv_sec || > + spec.ts.kern_its.it_interval.tv_nsec || > + spec.ts.kern_its.it_value.tv_sec || > + spec.ts.kern_its.it_value.tv_nsec))) > + tst_res(TFAIL, "timespec should have been zeroed"); Indeed that's what I had in mind, can we please abstract this properly as well? I guess that we can add helpers such as tst_its_interval_sec() tst_its_inverval_nsec, etc. > + else > + tst_res(TPASS, "timer_gettime() Passed"); > } else { > tst_res(TFAIL | TTERRNO, "timer_gettime() Failed"); > } > > -- > viresh
On 25-06-20, 13:07, Cyril Hrubis wrote: > > TEST(tv->func(timer, tst_its_get(&spec))); > > if (TST_RET == 0) { > > - tst_res(TPASS, "timer_gettime() Passed"); > > + if ((spec.type == TST_KERN_OLD_TIMESPEC && > > + (spec.ts.kern_old_its.it_interval.tv_sec || > > + spec.ts.kern_old_its.it_interval.tv_nsec || > > + spec.ts.kern_old_its.it_value.tv_sec || > > + spec.ts.kern_old_its.it_value.tv_nsec)) || > > + (spec.type == TST_KERN_TIMESPEC && > > + (spec.ts.kern_its.it_interval.tv_sec || > > + spec.ts.kern_its.it_interval.tv_nsec || > > + spec.ts.kern_its.it_value.tv_sec || > > + spec.ts.kern_its.it_value.tv_nsec))) > > + tst_res(TFAIL, "timespec should have been zeroed"); > > Indeed that's what I had in mind, can we please abstract this properly > as well? I guess that we can add helpers such as tst_its_interval_sec() > tst_its_inverval_nsec, etc. I preferred it this way as no one else uses it and so maybe we can live without adding those helpers ?
Hi! > > Indeed that's what I had in mind, can we please abstract this properly > > as well? I guess that we can add helpers such as tst_its_interval_sec() > > tst_its_inverval_nsec, etc. > > I preferred it this way as no one else uses it and so maybe we can > live without adding those helpers ? I'm pretty sure we will need them for the timer_settime() tests anyway, i.e. test that would do timer_settime() and then timer_gettime() to check that the timer is read back correclty.
diff --git a/include/tst_timer.h b/include/tst_timer.h index 256e1d71e1bc..708a1e9e9a7a 100644 --- a/include/tst_timer.h +++ b/include/tst_timer.h @@ -15,6 +15,7 @@ #include <sys/time.h> #include <time.h> #include "tst_test.h" +#include "lapi/common_timers.h" #include "lapi/syscalls.h" /* @@ -112,6 +113,16 @@ struct __kernel_timespec { __kernel_time64_t tv_sec; /* seconds */ long long tv_nsec; /* nanoseconds */ }; + +struct __kernel_old_itimerspec { + struct __kernel_old_timespec it_interval; /* timer period */ + struct __kernel_old_timespec it_value; /* timer expiration */ +}; + +struct __kernel_itimerspec { + struct __kernel_timespec it_interval; /* timer period */ + struct __kernel_timespec it_value; /* timer expiration */ +}; #endif enum tst_ts_type { @@ -129,6 +140,14 @@ struct tst_ts { } ts; }; +struct tst_its { + enum tst_ts_type type; + union { + struct __kernel_old_itimerspec kern_old_its; + struct __kernel_itimerspec kern_its; + } ts; +}; + static inline void *tst_ts_get(struct tst_ts *t) { if (!t) @@ -147,6 +166,22 @@ static inline void *tst_ts_get(struct tst_ts *t) } } +static inline void *tst_its_get(struct tst_its *t) +{ + if (!t) + return NULL; + + switch (t->type) { + case TST_KERN_OLD_TIMESPEC: + return &t->ts.kern_old_its; + case TST_KERN_TIMESPEC: + return &t->ts.kern_its; + default: + tst_brk(TBROK, "Invalid type: %d", t->type); + return NULL; + } +} + static inline int libc_clock_getres(clockid_t clk_id, void *ts) { return clock_getres(clk_id, ts); @@ -212,6 +247,16 @@ static inline int sys_clock_nanosleep64(clockid_t clk_id, int flags, request, remain); } +static inline int sys_timer_gettime(timer_t timerid, void *its) +{ + return tst_syscall(__NR_timer_gettime, timerid, its); +} + +static inline int sys_timer_gettime64(timer_t timerid, void *its) +{ + return tst_syscall(__NR_timer_gettime64, timerid, its); +} + /* * Returns tst_ts seconds. */ diff --git a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c index 1c75f1cf0e45..d2b89eab4223 100644 --- a/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c +++ b/testcases/kernel/syscalls/timer_gettime/timer_gettime01.c @@ -1,24 +1,5 @@ -/****************************************************************************** - * Copyright (c) Crackerjack Project., 2007 * - * Porting from Crackerjack to LTP is done by: * - * Manas Kumar Nayak <maknayak@in.ibm.com> * - * Copyright (c) 2013 Cyril Hrubis <chrubis@suse.cz> * - * * - * This program is free software; you can redistribute it and/or modify * - * it under the terms of the GNU General Public License as published by * - * the Free Software Foundation; either version 2 of the License, or * - * (at your option) any later version. * - * * - * This program is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See * - * the GNU General Public License for more details. * - * * - * You should have received a copy of the GNU General Public License * - * along with this program; if not, write to the Free Software Foundation, * - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * - * * - ******************************************************************************/ +// SPDX-License-Identifier: GPL-2.0-or-later +/* Copyright (c) Crackerjack Project., 2007 */ #include <time.h> #include <signal.h> @@ -26,71 +7,70 @@ #include <stdio.h> #include <errno.h> -#include "test.h" -#include "lapi/syscalls.h" +#include "tst_timer.h" -char *TCID = "timer_gettime01"; -int TST_TOTAL = 3; +static struct test_variants { + int (*func)(timer_t timer, void *its); + enum tst_ts_type type; + char *desc; +} variants[] = { +#if (__NR_timer_gettime != __LTP__NR_INVALID_SYSCALL) + { .func = sys_timer_gettime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"}, +#endif -static void cleanup(void) -{ - tst_rmdir(); -} +#if (__NR_timer_gettime64 != __LTP__NR_INVALID_SYSCALL) + { .func = sys_timer_gettime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"}, +#endif +}; -static void setup(void) -{ - TEST_PAUSE; - tst_tmpdir(); -} +static timer_t timer; -int main(int ac, char **av) +static void setup(void) { - int lc; - struct sigevent ev; - struct itimerspec spec; - int timer; - tst_parse_opts(ac, av, NULL, NULL); - - setup(); + tst_res(TINFO, "Testing variant: %s", variants[tst_variant].desc); ev.sigev_value = (union sigval) 0; ev.sigev_signo = SIGALRM; ev.sigev_notify = SIGEV_SIGNAL; - TEST(ltp_syscall(__NR_timer_create, CLOCK_REALTIME, &ev, &timer)); - - if (TEST_RETURN != 0) - tst_brkm(TBROK | TERRNO, cleanup, "Failed to create timer"); - for (lc = 0; TEST_LOOPING(lc); ++lc) { - tst_count = 0; + TEST(tst_syscall(__NR_timer_create, CLOCK_REALTIME, &ev, &timer)); - TEST(ltp_syscall(__NR_timer_gettime, timer, &spec)); - if (TEST_RETURN == 0) { - tst_resm(TPASS, "timer_gettime(CLOCK_REALTIME) Passed"); - } else { - tst_resm(TFAIL | TERRNO, - "timer_gettime(CLOCK_REALTIME) Failed"); - } - - TEST(ltp_syscall(__NR_timer_gettime, -1, &spec)); - if (TEST_RETURN == -1 && TEST_ERRNO == EINVAL) { - tst_resm(TPASS, "timer_gettime(-1) Failed: EINVAL"); - } else { - tst_resm(TFAIL | TERRNO, - "timer_gettime(-1) = %li", TEST_RETURN); - } + if (TST_RET) { + tst_res(TFAIL | TTERRNO, "timer_create() failed"); + return; + } +} - TEST(ltp_syscall(__NR_timer_gettime, timer, NULL)); - if (TEST_RETURN == -1 && TEST_ERRNO == EFAULT) { - tst_resm(TPASS, "timer_gettime(NULL) Failed: EFAULT"); - } else { - tst_resm(TFAIL | TERRNO, - "timer_gettime(-1) = %li", TEST_RETURN); - } +static void verify(void) +{ + struct test_variants *tv = &variants[tst_variant]; + struct tst_its spec = {.type = tv->type, }; + + TEST(tv->func(timer, tst_its_get(&spec))); + if (TST_RET == 0) { + tst_res(TPASS, "timer_gettime() Passed"); + } else { + tst_res(TFAIL | TTERRNO, "timer_gettime() Failed"); } - cleanup(); - tst_exit(); + TEST(tv->func((timer_t)-1, tst_its_get(&spec))); + if (TST_RET == -1 && TST_ERR == EINVAL) + tst_res(TPASS, "timer_gettime(-1) Failed: EINVAL"); + else + tst_res(TFAIL | TTERRNO, "timer_gettime(-1) = %li", TST_RET); + + TEST(tv->func(timer, NULL)); + if (TST_RET == -1 && TST_ERR == EFAULT) + tst_res(TPASS, "timer_gettime(NULL) Failed: EFAULT"); + else + tst_res(TFAIL | TTERRNO, "timer_gettime(-1) = %li", TST_RET); } + +static struct tst_test test = { + .test_all = verify, + .test_variants = ARRAY_SIZE(variants), + .setup = setup, + .needs_tmpdir = 1, +};
This adds support for time64 tests to the existing timer_gettime() syscall tests. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- V6: Drop the binary files included by mistake. include/tst_timer.h | 45 +++++++ .../syscalls/timer_gettime/timer_gettime01.c | 124 ++++++++---------- 2 files changed, 97 insertions(+), 72 deletions(-)