<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 14, 2022 at 11:52 PM Richard Palethorpe <<a href="mailto:rpalethorpe@suse.de" target="_blank">rpalethorpe@suse.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br>
<br>
Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>> writes:<br>
<br>
> Split checking the return ovalue from testing the signal is<br>
> delivered, so that we could use two time value for verifying.<br>
><br>
> Also, adding interval timer test by handling the signal at<br>
> least 10 times. After that recover the signal behavior to<br>
> default and do deliver-signal checking.<br>
><br>
> Signed-off-by: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
> ---<br>
> .../kernel/syscalls/setitimer/setitimer01.c | 63 ++++++++++++-------<br>
> 1 file changed, 39 insertions(+), 24 deletions(-)<br>
><br>
> diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c<br>
> index 1f631d457..260590b0e 100644<br>
> --- a/testcases/kernel/syscalls/setitimer/setitimer01.c<br>
> +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c<br>
> @@ -22,8 +22,10 @@<br>
> #include "tst_safe_clocks.h"<br>
> <br>
> static struct itimerval *value, *ovalue;<br>
> +static volatile unsigned long sigcnt;<br>
> static long time_step;<br>
> -static long time_count;<br>
> +static long time_sec;<br>
> +static long time_usec;<br>
> <br>
> static struct tcase {<br>
> int which;<br>
> @@ -40,54 +42,66 @@ static int sys_setitimer(int which, void *new_value, void *old_value)<br>
> return tst_syscall(__NR_setitimer, which, new_value, old_value);<br>
> }<br>
> <br>
> -static void set_setitimer_value(int usec, int o_usec)<br>
> +static void sig_routine(int signo LTP_ATTRIBUTE_UNUSED)<br>
> {<br>
> - value->it_value.tv_sec = 0;<br>
> - value->it_value.tv_usec = usec;<br>
> - value->it_interval.tv_sec = 0;<br>
> - value->it_interval.tv_usec = 0;<br>
> + sigcnt++;<br>
> +}<br>
> <br>
> - ovalue->it_value.tv_sec = o_usec;<br>
> - ovalue->it_value.tv_usec = o_usec;<br>
> - ovalue->it_interval.tv_sec = 0;<br>
> - ovalue->it_interval.tv_usec = 0;<br>
> +static void set_setitimer_value(int sec, int usec)<br>
> +{<br>
> + value->it_value.tv_sec = sec;<br>
> + value->it_value.tv_usec = usec;<br>
> + value->it_interval.tv_sec = sec;<br>
> + value->it_interval.tv_usec = usec;<br>
> }<br>
> <br>
> static void verify_setitimer(unsigned int i)<br>
> {<br>
> pid_t pid;<br>
> int status;<br>
> - long margin;<br>
> struct tcase *tc = &tcases[i];<br>
> <br>
> + tst_res(TINFO, "tc->which = %s", tc->des);<br>
> +<br>
> pid = SAFE_FORK();<br>
> <br>
> if (pid == 0) {<br>
> - tst_res(TINFO, "tc->which = %s", tc->des);<br>
> -<br>
> tst_no_corefile(0);<br>
> <br>
> - set_setitimer_value(time_count, 0);<br>
> + set_setitimer_value(time_sec, time_usec);<br>
> TST_EXP_PASS(sys_setitimer(tc->which, value, NULL));<br>
> <br>
> - set_setitimer_value(5 * time_step, 7 * time_step);<br>
> + set_setitimer_value(2 * time_sec, 2 * time_usec);<br>
<br>
IDK if there was some reason for choosing 5 and 7 in the first place?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">I don't see any necessary reasons for using prime numbers here.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">@Andrea, did I miss something?</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Andrea seemed to be going through the prime numbers.<br>
<br>
> TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue));<br>
> <br>
> - tst_res(TINFO, "tv_sec=%ld, tv_usec=%ld",<br>
> - ovalue->it_value.tv_sec,<br>
> - ovalue->it_value.tv_usec);<br>
> + TST_EXP_EQ_LI(ovalue->it_interval.tv_sec, time_sec);<br>
> + TST_EXP_EQ_LI(ovalue->it_interval.tv_usec, time_usec);<br>
> +<br>
> + tst_res(TINFO, "ovalue->it_value.tv_sec=%ld, ovalue->it_value.tv_usec=%ld",<br>
> + ovalue->it_value.tv_sec, ovalue->it_value.tv_usec);<br>
> <br>
> /*<br>
> * ITIMER_VIRTUAL and ITIMER_PROF timers always expire a<br>
> * time_step afterward the elapsed time to make sure that<br>
> * at least counters take effect.<br>
> */<br>
> - margin = tc->which == ITIMER_REAL ? 0 : time_step;<br>
> + long margin = (tc->which == ITIMER_REAL) ? 0 : time_step;<br>
<br>
Looks like an unecessary change?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yes, but the 'margin' is only used in children, and I moved</div><div class="gmail_default" style="font-size:small">the declaration here is just to highlight and cause attention</div><div class="gmail_default" style="font-size:small">in code reading.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> <br>
> - if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec > time_count + margin)<br>
> + if (ovalue->it_value.tv_sec > time_sec ||<br>
> + ovalue->it_value.tv_usec > time_usec + margin)<br>
<br>
Looking at the man page, technically speaking the valid range for<br>
ovalue->it_value.tv_sec is 0 to value->it_value.tv_sec when<br>
ovalue->it_value.tv_usec > 0.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Good point!! Seems we have to split the situation into two,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">1. no tv_sec elapse happen, just check</div><div class="gmail_default" style="font-size:small"> 'it_value.tv_usec' within [0, time_usec + margin]</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">2. tv_sec elapses happen, then check </div><div class="gmail_default" style="font-size:small"> 'it_value.tv_sec' within [0, time_sec]</div></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Something maybe like:</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">--- a/testcases/kernel/syscalls/setitimer/setitimer01.c<br>+++ b/testcases/kernel/syscalls/setitimer/setitimer01.c<br>@@ -87,9 +87,17 @@ static void verify_setitimer(unsigned int i)<br> */<br> long margin = (tc->which == ITIMER_REAL) ? 0 : time_step;<br> <br>- if (ovalue->it_value.tv_sec > time_sec ||<br>- ovalue->it_value.tv_usec > time_usec + margin)<br>- tst_res(TFAIL, "Ending counters are out of range");<br>+ if (ovalue->it_value.tv_sec == time_sec) {<br>+ if (ovalue->it_value.tv_usec < 0 ||<br>+ ovalue->it_value.tv_usec > time_usec + margin)<br>+ tst_res(TFAIL, "ovalue->it_value.tv_usec is out of range: %ld",<br>+ ovalue->it_value.tv_usec);<br>+ } else {<br>+ if (ovalue->it_value.tv_sec < 0 ||<br>+ ovalue->it_value.tv_sec > time_sec)<br>+ tst_res(TFAIL, "ovalue->it_value.tv_sec is out of range: %ld",<br>+ ovalue->it_value.tv_usec);<br>+ }<br> <br> SAFE_SIGNAL(tc->signo, sig_routine);<br></div><div class="gmail_default" style="font-size:small"><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Practically speaking we have to assume a large amount of time has passed<br>
when using ITIMER_REAL. It has to be *much* larger than a VM is likely<br>
to be paused for and then successfully resumed. Or the amount of time a<br>
clock may be corrected by (for e.g. with NTP).<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Hmm, not sure if I understand correctly above, will that</div><div class="gmail_default" style="font-size:small">timer value be out of the range but reasonable?</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Or is there any additional situation we should cover?</div><div class="gmail_default" style="font-size:small"><br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> tst_res(TFAIL, "Ending counters are out of range");<br>
<br>
While we are here; we should make our lives easier when the test fails<br>
by printing any relevant values.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">We already do that in the above print, but it's fine to have that here as well.</div></div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>