* [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 @ 2022-11-02 14:59 Andrea Cervesato via ltp 2022-11-02 15:39 ` Martin Doucha 2022-11-04 6:48 ` Li Wang 0 siblings, 2 replies; 7+ messages in thread From: Andrea Cervesato via ltp @ 2022-11-02 14:59 UTC (permalink / raw) To: ltp Last test rewrite didn't consider the right expected boundaries when setitimer syscall was tested. We also introduced counter times as multiple of clock resolution, to avoid kernel rounding during setitimer counter increase. Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> --- .../kernel/syscalls/setitimer/setitimer01.c | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c index f04cb5a69..3fb9250e2 100644 --- a/testcases/kernel/syscalls/setitimer/setitimer01.c +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c @@ -8,20 +8,21 @@ /*\ * [Description] * - * Check that a setitimer() call pass with timer seting. - * Check if signal is generated correctly when timer expiration. + * Spaw a child and verify that setitimer() syscall passes and it ends up + * counting inside expected boundaries. Then verify from parent that our syscall + * sent the correct signal to the child. */ +#include <time.h> #include <errno.h> #include <sys/time.h> #include <stdlib.h> #include "tst_test.h" #include "lapi/syscalls.h" - -#define USEC1 10000 -#define USEC2 20000 +#include "tst_safe_clocks.h" static struct itimerval *value, *ovalue; +static unsigned long time_step; static struct tcase { int which; @@ -55,6 +56,7 @@ static void verify_setitimer(unsigned int i) { pid_t pid; int status; + int usec = 3 * time_step; struct tcase *tc = &tcases[i]; pid = SAFE_FORK(); @@ -64,14 +66,18 @@ static void verify_setitimer(unsigned int i) tst_no_corefile(0); - set_setitimer_value(USEC1, 0); - TST_EXP_PASS(sys_setitimer(tc->which, value, NULL)); + set_setitimer_value(usec, 0); + TST_EXP_PASS(sys_setitimer(tc->which, value, 0)); - set_setitimer_value(USEC2, USEC2); + set_setitimer_value(5 * time_step, 7 * time_step); TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue)); - if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec >= USEC2) - tst_brk(TFAIL, "old timer value is not within the expected range"); + tst_res(TINFO, "tv_sec=%ld, tv_usec=%ld", + ovalue->it_value.tv_sec, + ovalue->it_value.tv_usec); + + if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec > usec) + tst_res(TFAIL, "Ending counters are out of range"); for (;;) ; @@ -85,10 +91,26 @@ static void verify_setitimer(unsigned int i) tst_res(TFAIL, "Child: %s", tst_strstatus(status)); } +static void setup(void) +{ + struct timespec res; + + SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res); + + time_step = res.tv_nsec / 1000; + if (time_step < 10000) + time_step = 10000; + + tst_res(TINFO, "clock resolution: %luns, time step: %luus", + res.tv_nsec, + time_step); +} + static struct tst_test test = { .tcnt = ARRAY_SIZE(tcases), .forks_child = 1, .test = verify_setitimer, + .setup = setup, .bufs = (struct tst_buffers[]) { {&value, .size = sizeof(struct itimerval)}, {&ovalue, .size = sizeof(struct itimerval)}, -- 2.35.3 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 2022-11-02 14:59 [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 Andrea Cervesato via ltp @ 2022-11-02 15:39 ` Martin Doucha 2022-11-03 13:35 ` Andrea Cervesato via ltp 2022-11-04 6:48 ` Li Wang 1 sibling, 1 reply; 7+ messages in thread From: Martin Doucha @ 2022-11-02 15:39 UTC (permalink / raw) To: Andrea Cervesato, ltp Hi, one small nit below, otherwise: Reviewed-by: Martin Doucha <mdoucha@suse.cz> On 02. 11. 22 15:59, Andrea Cervesato via ltp wrote: > Last test rewrite didn't consider the right expected boundaries when > setitimer syscall was tested. We also introduced counter times as > multiple of clock resolution, to avoid kernel rounding during setitimer > counter increase. > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > .../kernel/syscalls/setitimer/setitimer01.c | 42 ++++++++++++++----- > 1 file changed, 32 insertions(+), 10 deletions(-) > > diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c > index f04cb5a69..3fb9250e2 100644 > --- a/testcases/kernel/syscalls/setitimer/setitimer01.c > +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c > @@ -8,20 +8,21 @@ > /*\ > * [Description] > * > - * Check that a setitimer() call pass with timer seting. > - * Check if signal is generated correctly when timer expiration. > + * Spaw a child and verify that setitimer() syscall passes and it ends up > + * counting inside expected boundaries. Then verify from parent that our syscall > + * sent the correct signal to the child. > */ > > +#include <time.h> > #include <errno.h> > #include <sys/time.h> > #include <stdlib.h> > #include "tst_test.h" > #include "lapi/syscalls.h" > - > -#define USEC1 10000 > -#define USEC2 20000 > +#include "tst_safe_clocks.h" > > static struct itimerval *value, *ovalue; > +static unsigned long time_step; > > static struct tcase { > int which; > @@ -55,6 +56,7 @@ static void verify_setitimer(unsigned int i) > { > pid_t pid; > int status; > + int usec = 3 * time_step; > struct tcase *tc = &tcases[i]; > > pid = SAFE_FORK(); > @@ -64,14 +66,18 @@ static void verify_setitimer(unsigned int i) > > tst_no_corefile(0); > > - set_setitimer_value(USEC1, 0); > - TST_EXP_PASS(sys_setitimer(tc->which, value, NULL)); > + set_setitimer_value(usec, 0); > + TST_EXP_PASS(sys_setitimer(tc->which, value, 0)); Why change the third argument from NULL to 0? > > - set_setitimer_value(USEC2, USEC2); > + set_setitimer_value(5 * time_step, 7 * time_step); > TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue)); > > - if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec >= USEC2) > - tst_brk(TFAIL, "old timer value is not within the expected range"); > + tst_res(TINFO, "tv_sec=%ld, tv_usec=%ld", > + ovalue->it_value.tv_sec, > + ovalue->it_value.tv_usec); > + > + if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec > usec) > + tst_res(TFAIL, "Ending counters are out of range"); > > for (;;) > ; > @@ -85,10 +91,26 @@ static void verify_setitimer(unsigned int i) > tst_res(TFAIL, "Child: %s", tst_strstatus(status)); > } > > +static void setup(void) > +{ > + struct timespec res; > + > + SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res); > + > + time_step = res.tv_nsec / 1000; > + if (time_step < 10000) > + time_step = 10000; > + > + tst_res(TINFO, "clock resolution: %luns, time step: %luus", > + res.tv_nsec, > + time_step); > +} > + > static struct tst_test test = { > .tcnt = ARRAY_SIZE(tcases), > .forks_child = 1, > .test = verify_setitimer, > + .setup = setup, > .bufs = (struct tst_buffers[]) { > {&value, .size = sizeof(struct itimerval)}, > {&ovalue, .size = sizeof(struct itimerval)}, -- Martin Doucha mdoucha@suse.cz QA Engineer for Software Maintenance SUSE LINUX, s.r.o. CORSO IIa Krizikova 148/34 186 00 Prague 8 Czech Republic -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 2022-11-02 15:39 ` Martin Doucha @ 2022-11-03 13:35 ` Andrea Cervesato via ltp 2022-11-03 14:05 ` Petr Vorel 0 siblings, 1 reply; 7+ messages in thread From: Andrea Cervesato via ltp @ 2022-11-03 13:35 UTC (permalink / raw) To: Martin Doucha, ltp Hi, On 11/2/22 16:39, Martin Doucha wrote: > Hi, > one small nit below, otherwise: > > Reviewed-by: Martin Doucha <mdoucha@suse.cz> > > On 02. 11. 22 15:59, Andrea Cervesato via ltp wrote: >> Last test rewrite didn't consider the right expected boundaries when >> setitimer syscall was tested. We also introduced counter times as >> multiple of clock resolution, to avoid kernel rounding during setitimer >> counter increase. >> >> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >> --- >> .../kernel/syscalls/setitimer/setitimer01.c | 42 ++++++++++++++----- >> 1 file changed, 32 insertions(+), 10 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c >> b/testcases/kernel/syscalls/setitimer/setitimer01.c >> index f04cb5a69..3fb9250e2 100644 >> --- a/testcases/kernel/syscalls/setitimer/setitimer01.c >> +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c >> @@ -8,20 +8,21 @@ >> /*\ >> * [Description] >> * >> - * Check that a setitimer() call pass with timer seting. >> - * Check if signal is generated correctly when timer expiration. >> + * Spaw a child and verify that setitimer() syscall passes and it >> ends up >> + * counting inside expected boundaries. Then verify from parent that >> our syscall >> + * sent the correct signal to the child. >> */ >> +#include <time.h> >> #include <errno.h> >> #include <sys/time.h> >> #include <stdlib.h> >> #include "tst_test.h" >> #include "lapi/syscalls.h" >> - >> -#define USEC1 10000 >> -#define USEC2 20000 >> +#include "tst_safe_clocks.h" >> static struct itimerval *value, *ovalue; >> +static unsigned long time_step; >> static struct tcase { >> int which; >> @@ -55,6 +56,7 @@ static void verify_setitimer(unsigned int i) >> { >> pid_t pid; >> int status; >> + int usec = 3 * time_step; >> struct tcase *tc = &tcases[i]; >> pid = SAFE_FORK(); >> @@ -64,14 +66,18 @@ static void verify_setitimer(unsigned int i) >> tst_no_corefile(0); >> - set_setitimer_value(USEC1, 0); >> - TST_EXP_PASS(sys_setitimer(tc->which, value, NULL)); >> + set_setitimer_value(usec, 0); >> + TST_EXP_PASS(sys_setitimer(tc->which, value, 0)); > > Why change the third argument from NULL to 0? > It's a mistake. I sent patch with 0 because I was tweaking with values during tests. >> - set_setitimer_value(USEC2, USEC2); >> + set_setitimer_value(5 * time_step, 7 * time_step); >> TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue)); >> - if (ovalue->it_value.tv_sec != 0 || >> ovalue->it_value.tv_usec >= USEC2) >> - tst_brk(TFAIL, "old timer value is not within the >> expected range"); >> + tst_res(TINFO, "tv_sec=%ld, tv_usec=%ld", >> + ovalue->it_value.tv_sec, >> + ovalue->it_value.tv_usec); >> + >> + if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec >> > usec) >> + tst_res(TFAIL, "Ending counters are out of range"); >> for (;;) >> ; >> @@ -85,10 +91,26 @@ static void verify_setitimer(unsigned int i) >> tst_res(TFAIL, "Child: %s", tst_strstatus(status)); >> } >> +static void setup(void) >> +{ >> + struct timespec res; >> + >> + SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res); >> + >> + time_step = res.tv_nsec / 1000; >> + if (time_step < 10000) >> + time_step = 10000; >> + >> + tst_res(TINFO, "clock resolution: %luns, time step: %luus", >> + res.tv_nsec, >> + time_step); >> +} >> + >> static struct tst_test test = { >> .tcnt = ARRAY_SIZE(tcases), >> .forks_child = 1, >> .test = verify_setitimer, >> + .setup = setup, >> .bufs = (struct tst_buffers[]) { >> {&value, .size = sizeof(struct itimerval)}, >> {&ovalue, .size = sizeof(struct itimerval)}, > Andrea -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 2022-11-03 13:35 ` Andrea Cervesato via ltp @ 2022-11-03 14:05 ` Petr Vorel 0 siblings, 0 replies; 7+ messages in thread From: Petr Vorel @ 2022-11-03 14:05 UTC (permalink / raw) To: Andrea Cervesato; +Cc: ltp Hi all, > > > @@ -64,14 +66,18 @@ static void verify_setitimer(unsigned int i) > > > tst_no_corefile(0); > > > - set_setitimer_value(USEC1, 0); > > > - TST_EXP_PASS(sys_setitimer(tc->which, value, NULL)); > > > + set_setitimer_value(usec, 0); > > > + TST_EXP_PASS(sys_setitimer(tc->which, value, 0)); > > Why change the third argument from NULL to 0? > It's a mistake. I sent patch with 0 because I was tweaking with values > during tests. Good. I put it back and merged with updated copyright and adjusted the commit message and description. Thanks! Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 2022-11-02 14:59 [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 Andrea Cervesato via ltp 2022-11-02 15:39 ` Martin Doucha @ 2022-11-04 6:48 ` Li Wang 2022-11-04 8:18 ` Li Wang 2022-11-04 8:18 ` Andrea Cervesato via ltp 1 sibling, 2 replies; 7+ messages in thread From: Li Wang @ 2022-11-04 6:48 UTC (permalink / raw) To: Andrea Cervesato; +Cc: ltp [-- Attachment #1.1: Type: text/plain, Size: 4314 bytes --] Hi Andrea, Andrea Cervesato via ltp <ltp@lists.linux.it> wrote: Last test rewrite didn't consider the right expected boundaries when > setitimer syscall was tested. We also introduced counter times as > multiple of clock resolution, to avoid kernel rounding during setitimer > counter increase. > Good catch, but I'm afraid this can not solve the problem thoroughly. According failure log on "ITIMER_VIRTUAL/PROF" with running this patch.: setitimer01.c:64: TINFO: tc->which = ITIMER_VIRTUAL setitimer01.c:69: TPASS: sys_setitimer(tc->which, value, NULL) passed setitimer01.c:72: TPASS: sys_setitimer(tc->which, value, ovalue) passed setitimer01.c:76: TINFO: tv_sec=0, tv_usec=31000 setitimer01.c:79: TFAIL: Ending counters are out of range setitimer01.c:88: TPASS: Child received signal: SIGVTALRM setitimer01.c:64: TINFO: tc->which = ITIMER_PROF setitimer01.c:69: TPASS: sys_setitimer(tc->which, value, NULL) passed setitimer01.c:72: TPASS: sys_setitimer(tc->which, value, ovalue) passed setitimer01.c:76: TINFO: tv_sec=0, tv_usec=31000 setitimer01.c:79: TFAIL: Ending counters are out of range setitimer01.c:88: TPASS: Child received signal: SIGPROF It seems the tv_usec always increase 1000us, I highly suspect this increase comes from kernel function set_cpu_itimer() that explicitly add TICK_NSEC when 'nval' is larger than zero: $ cat kernel/time/itimer.c -n ... 168 static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id, 169 const struct itimerspec64 *const value, 170 struct itimerspec64 *const ovalue) 171 { ... 182 if (oval || nval) { 183 if (nval > 0) 184 nval += TICK_NSEC; 185 set_process_cpu_timer(tsk, clock_id, &nval, &oval); 186 } 187 ... 198 } To verify my guess, I do a modification based on your patch and then easily get the result in pass. --- a/testcases/kernel/syscalls/setitimer/setitimer01.c +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c @@ -76,7 +76,7 @@ static void verify_setitimer(unsigned int i) ovalue->it_value.tv_sec, ovalue->it_value.tv_usec); - if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec > usec) + if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec - time_step > usec) tst_res(TFAIL, "Ending counters are out of range"); for (;;) @@ -98,8 +98,8 @@ static void setup(void) SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res); time_step = res.tv_nsec / 1000; - if (time_step < 10000) - time_step = 10000; + if (time_step < 1000) + time_step = 1000; tst_res(TINFO, "clock resolution: %luns, time step: %luus", res.tv_nsec, But after trying this with my RasberryPi4, it fails again with an increase 4000us each time. So it might related to the system use different time resolutions. When I shift to use `CLOCK_MONOTONIC_COARSE` then test gets passed on all my platforms. Any comments? ----------CLOCK_MONOTONIC------------- setitimer01.c:65: TINFO: tc->which = ITIMER_VIRTUAL setitimer01.c:70: TPASS: sys_setitimer(tc->which, value, NULL) passed setitimer01.c:73: TPASS: sys_setitimer(tc->which, value, ovalue) passed setitimer01.c:77: TINFO: tv_sec=0, tv_usec=7000 setitimer01.c:80: TFAIL: Ending counters are out of range -----------CLOCK_MONOTONIC_COARSE------- setitimer01.c:65: TINFO: tc->which = ITIMER_VIRTUAL setitimer01.c:70: TPASS: sys_setitimer(tc->which, value, NULL) passed setitimer01.c:73: TPASS: sys_setitimer(tc->which, value, ovalue) passed setitimer01.c:77: TINFO: tv_sec=0, tv_usec=16000 setitimer01.c:89: TPASS: Child received signal: SIGVTALRM # lscpu Architecture: aarch64 Byte Order: Little Endian CPU(s): 4 On-line CPU(s) list: 0-3 Thread(s) per core: 1 Core(s) per cluster: 4 Socket(s): - Cluster(s): 1 Vendor ID: ARM Model: 3 Model name: Cortex-A72 Stepping: r0p3 CPU max MHz: 1500.0000 CPU min MHz: 600.0000 BogoMIPS: 108.00 Flags: fp asimd evtstrm crc32 cpuid -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 7440 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 2022-11-04 6:48 ` Li Wang @ 2022-11-04 8:18 ` Li Wang 2022-11-04 8:18 ` Andrea Cervesato via ltp 1 sibling, 0 replies; 7+ messages in thread From: Li Wang @ 2022-11-04 8:18 UTC (permalink / raw) To: Andrea Cervesato; +Cc: ltp [-- Attachment #1.1: Type: text/plain, Size: 5705 bytes --] Btw, these are the final changes I propose to fix the problem, but I haven't figured out why the resolution from CLOCK_*_COARSE worked. --- a/testcases/kernel/syscalls/setitimer/setitimer01.c +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c @@ -76,7 +76,7 @@ static void verify_setitimer(unsigned int i) ovalue->it_value.tv_sec, ovalue->it_value.tv_usec); - if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec > usec) + if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec - time_step > usec) tst_res(TFAIL, "Ending counters are out of range"); for (;;) @@ -95,11 +95,9 @@ static void setup(void) { struct timespec res; - SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res); + SAFE_CLOCK_GETRES(CLOCK_MONOTONIC_COARSE, &res); time_step = res.tv_nsec / 1000; - if (time_step < 10000) - time_step = 10000; tst_res(TINFO, "clock resolution: %luns, time step: %luus", res.tv_nsec, On Fri, Nov 4, 2022 at 2:48 PM Li Wang <liwang@redhat.com> wrote: > Hi Andrea, > > Andrea Cervesato via ltp <ltp@lists.linux.it> wrote: > > Last test rewrite didn't consider the right expected boundaries when >> setitimer syscall was tested. We also introduced counter times as >> multiple of clock resolution, to avoid kernel rounding during setitimer >> counter increase. >> > > Good catch, but I'm afraid this can not solve the problem thoroughly. > According failure log on "ITIMER_VIRTUAL/PROF" with running this patch.: > > setitimer01.c:64: TINFO: tc->which = ITIMER_VIRTUAL > setitimer01.c:69: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:72: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:76: TINFO: tv_sec=0, tv_usec=31000 > setitimer01.c:79: TFAIL: Ending counters are out of range > setitimer01.c:88: TPASS: Child received signal: SIGVTALRM > > setitimer01.c:64: TINFO: tc->which = ITIMER_PROF > setitimer01.c:69: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:72: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:76: TINFO: tv_sec=0, tv_usec=31000 > setitimer01.c:79: TFAIL: Ending counters are out of range > setitimer01.c:88: TPASS: Child received signal: SIGPROF > > It seems the tv_usec always increase 1000us, I highly suspect > this increase comes from kernel function set_cpu_itimer() that > explicitly add TICK_NSEC when 'nval' is larger than zero: > > $ cat kernel/time/itimer.c -n > ... > 168 static void set_cpu_itimer(struct task_struct *tsk, unsigned int > clock_id, > 169 const struct itimerspec64 *const value, > 170 struct itimerspec64 *const ovalue) > 171 { > ... > 182 if (oval || nval) { > 183 if (nval > 0) > 184 nval += TICK_NSEC; > 185 set_process_cpu_timer(tsk, clock_id, &nval, &oval); > 186 } > 187 ... > 198 } > > To verify my guess, I do a modification based on your patch and > then easily get the result in pass. > > --- a/testcases/kernel/syscalls/setitimer/setitimer01.c > +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c > @@ -76,7 +76,7 @@ static void verify_setitimer(unsigned int i) > ovalue->it_value.tv_sec, > ovalue->it_value.tv_usec); > > - if (ovalue->it_value.tv_sec != 0 || > ovalue->it_value.tv_usec > usec) > + if (ovalue->it_value.tv_sec != 0 || > ovalue->it_value.tv_usec - time_step > usec) > tst_res(TFAIL, "Ending counters are out of range"); > > for (;;) > @@ -98,8 +98,8 @@ static void setup(void) > SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res); > > time_step = res.tv_nsec / 1000; > - if (time_step < 10000) > - time_step = 10000; > + if (time_step < 1000) > + time_step = 1000; > > tst_res(TINFO, "clock resolution: %luns, time step: %luus", > res.tv_nsec, > > > But after trying this with my RasberryPi4, it fails again with an increase > 4000us each time. So it might related to the system use different time > resolutions. When I shift to use `CLOCK_MONOTONIC_COARSE` > then test gets passed on all my platforms. > > Any comments? > > > ----------CLOCK_MONOTONIC------------- > setitimer01.c:65: TINFO: tc->which = ITIMER_VIRTUAL > setitimer01.c:70: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:73: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:77: TINFO: tv_sec=0, tv_usec=7000 > setitimer01.c:80: TFAIL: Ending counters are out of range > > -----------CLOCK_MONOTONIC_COARSE------- > setitimer01.c:65: TINFO: tc->which = ITIMER_VIRTUAL > setitimer01.c:70: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:73: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:77: TINFO: tv_sec=0, tv_usec=16000 > setitimer01.c:89: TPASS: Child received signal: SIGVTALRM > > # lscpu > Architecture: aarch64 > Byte Order: Little Endian > CPU(s): 4 > On-line CPU(s) list: 0-3 > Thread(s) per core: 1 > Core(s) per cluster: 4 > Socket(s): - > Cluster(s): 1 > Vendor ID: ARM > Model: 3 > Model name: Cortex-A72 > Stepping: r0p3 > CPU max MHz: 1500.0000 > CPU min MHz: 600.0000 > BogoMIPS: 108.00 > Flags: fp asimd evtstrm crc32 cpuid > > -- > Regards, > Li Wang > -- Regards, Li Wang [-- Attachment #1.2: Type: text/html, Size: 8819 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 2022-11-04 6:48 ` Li Wang 2022-11-04 8:18 ` Li Wang @ 2022-11-04 8:18 ` Andrea Cervesato via ltp 1 sibling, 0 replies; 7+ messages in thread From: Andrea Cervesato via ltp @ 2022-11-04 8:18 UTC (permalink / raw) To: Li Wang; +Cc: ltp [-- Attachment #1.1: Type: text/plain, Size: 5151 bytes --] Hi! On 11/4/22 07:48, Li Wang wrote: > Hi Andrea, > > Andrea Cervesato via ltp <ltp@lists.linux.it> wrote: > > Last test rewrite didn't consider the right expected boundaries when > setitimer syscall was tested. We also introduced counter times as > multiple of clock resolution, to avoid kernel rounding during > setitimer > counter increase. > > > Good catch, but I'm afraid this can not solve the problem thoroughly. > According failure log on "ITIMER_VIRTUAL/PROF" with running this patch.: > > setitimer01.c:64: TINFO: tc->which = ITIMER_VIRTUAL > setitimer01.c:69: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:72: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:76: TINFO: tv_sec=0, tv_usec=31000 > setitimer01.c:79: TFAIL: Ending counters are out of range > setitimer01.c:88: TPASS: Child received signal: SIGVTALRM > > setitimer01.c:64: TINFO: tc->which = ITIMER_PROF > setitimer01.c:69: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:72: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:76: TINFO: tv_sec=0, tv_usec=31000 > setitimer01.c:79: TFAIL: Ending counters are out of range > setitimer01.c:88: TPASS: Child received signal: SIGPROF > > It seems the tv_usec always increase 1000us, I highly suspect > this increase comes from kernel function set_cpu_itimer() that > explicitly add TICK_NSEC when 'nval' is larger than zero: > > $ cat kernel/time/itimer.c -n > ... > 168 static void set_cpu_itimer(struct task_struct *tsk, unsigned > int clock_id, > 169 const struct itimerspec64 *const value, > 170 struct itimerspec64 *const ovalue) > 171 { > ... > 182 if (oval || nval) { > 183 if (nval > 0) > 184 nval += TICK_NSEC; > 185 set_process_cpu_timer(tsk, clock_id, &nval, > &oval); > 186 } > 187 ... > 198 } > Yes, you caught the reason why VIRTUAL/PROF tests are not passing. CLOCK_MONOTONIC_COARSE is probably used because setitimer takes in consideration context switch from kernel to user space while counting. To use CLOCK_MONOTONIC_COARSE for VIRTUAL/PROF is probably the way to go. > To verify my guess, I do a modification based on your patch and > then easily get the result in pass. > > --- a/testcases/kernel/syscalls/setitimer/setitimer01.c > +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c > @@ -76,7 +76,7 @@ static void verify_setitimer(unsigned int i) > ovalue->it_value.tv_sec, > ovalue->it_value.tv_usec); > > - if (ovalue->it_value.tv_sec != 0 || > ovalue->it_value.tv_usec > usec) > + if (ovalue->it_value.tv_sec != 0 || > ovalue->it_value.tv_usec - time_step > usec) > tst_res(TFAIL, "Ending counters are out of > range"); > > for (;;) > @@ -98,8 +98,8 @@ static void setup(void) > SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res); > > time_step = res.tv_nsec / 1000; > - if (time_step < 10000) > - time_step = 10000; > + if (time_step < 1000) > + time_step = 1000; > > tst_res(TINFO, "clock resolution: %luns, time step: %luus", > res.tv_nsec, > > But after trying this with my RasberryPi4, it fails again with an increase > 4000us each time. So it might related to the system use different time > resolutions. When I shift to use `CLOCK_MONOTONIC_COARSE` > then test gets passed on all my platforms. > > Any comments? > > > ----------CLOCK_MONOTONIC------------- > setitimer01.c:65: TINFO: tc->which = ITIMER_VIRTUAL > setitimer01.c:70: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:73: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:77: TINFO: tv_sec=0, tv_usec=7000 > setitimer01.c:80: TFAIL: Ending counters are out of range > > -----------CLOCK_MONOTONIC_COARSE------- > setitimer01.c:65: TINFO: tc->which = ITIMER_VIRTUAL > setitimer01.c:70: TPASS: sys_setitimer(tc->which, value, NULL) passed > setitimer01.c:73: TPASS: sys_setitimer(tc->which, value, ovalue) passed > setitimer01.c:77: TINFO: tv_sec=0, tv_usec=16000 > setitimer01.c:89: TPASS: Child received signal: SIGVTALRM > > # lscpu > Architecture: aarch64 > Byte Order: Little Endian > CPU(s): 4 > On-line CPU(s) list: 0-3 > Thread(s) per core: 1 > Core(s) per cluster: 4 > Socket(s): - > Cluster(s): 1 > Vendor ID: ARM > Model: 3 > Model name: Cortex-A72 > Stepping: r0p3 > CPU max MHz: 1500.0000 > CPU min MHz: 600.0000 > BogoMIPS: 108.00 > Flags: fp asimd evtstrm crc32 cpuid > > -- > Regards, > Li Wang Andrea [-- Attachment #1.2: Type: text/html, Size: 11963 bytes --] [-- Attachment #2: Type: text/plain, Size: 60 bytes --] -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-04 12:11 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-02 14:59 [LTP] [PATCH v1] Correctly check setitimer params in setitimer01 Andrea Cervesato via ltp 2022-11-02 15:39 ` Martin Doucha 2022-11-03 13:35 ` Andrea Cervesato via ltp 2022-11-03 14:05 ` Petr Vorel 2022-11-04 6:48 ` Li Wang 2022-11-04 8:18 ` Li Wang 2022-11-04 8:18 ` Andrea Cervesato via ltp
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.