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