Re: [LTP] [PATCH 1/2] setitimer01: add interval timer test

[Richard Palethorpe <rpalethorpe@suse.de>]

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?

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?

>  
> -		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.

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).

>  			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.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp