[LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Andrea Cervesato via 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

Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Martin Doucha]

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

Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Andrea Cervesato via 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

Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Petr Vorel]

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

Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Li Wang]

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

Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Li Wang]

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

Re: [LTP] [PATCH v1] Correctly check setitimer params in setitimer01

[Andrea Cervesato via 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