<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div class="moz-cite-prefix">Hi Li,</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 11/15/22 05:00, Li Wang wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAEemH2fJJ8Fv0=OMjdh3d8QaEdr1ivLUuHs_8=D6qV6-YZhbwQ@mail.gmail.com">
<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" moz-do-not-send="true" class="moz-txt-link-freetext">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" moz-do-not-send="true" class="moz-txt-link-freetext">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" moz-do-not-send="true" class="moz-txt-link-freetext">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>
</div>
</div>
</blockquote>
The reason is that we want to trace input values in the setitimer
syscalls. By setting them to x5/7 we are able to debug setitimer
easily if bug appears.<br>
<blockquote type="cite" cite="mid:CAEemH2fJJ8Fv0=OMjdh3d8QaEdr1ivLUuHs_8=D6qV6-YZhbwQ@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<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>
</blockquote>
<p>Andrea<br>
</p>
</body>
</html>