[LTP] [PATCH v1] Correctly handle user time in setitimer01

[LTP] [PATCH v1] Correctly handle user time in setitimer01

[Andrea Cervesato via ltp]

Since ITIMER_VIRTUAL and ITIMER_PROF are counting down in user time, we
need to take in consideration CLOCK_MONOTONIC_COARSE resolution. This is
requested by the syscall, since it's considering context switch from
user to kernel mode by using a higher clock resolution.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 .../kernel/syscalls/setitimer/setitimer01.c   | 54 +++++++++++--------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c
index eb62f02c6..5fcae53e8 100644
--- a/testcases/kernel/syscalls/setitimer/setitimer01.c
+++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
@@ -8,9 +8,16 @@
 /*\
  * [Description]
  *
- * Spawn a child and verify that setitimer() syscall passes, and it ends up
+ * Spawn a child, verify that setitimer() syscall passes and it ends up
  * counting inside expected boundaries. Then verify from the parent that our
  * syscall sent the correct signal to the child.
+ *
+ * Boundaries are choosen accordingly with system clock. In particular, when
+ * timer counts down in real time, CLOCK_MONOTONIC resolution has taken into
+ * account as our time step. When timer counts down in user time,
+ * CLOCK_MONOTONIC_COARSE is used. The reason is that CLOCK_MONOTONIC_COARSE
+ * is our system resolution in user space, since it's taking in consideration
+ * context switches from user to kernel space.
  */
 
 #include <time.h>
@@ -22,7 +29,6 @@
 #include "tst_safe_clocks.h"
 
 static struct itimerval *value, *ovalue;
-static unsigned long time_step;
 
 static struct tcase {
 	int which;
@@ -55,9 +61,31 @@ static void set_setitimer_value(int usec, int o_usec)
 static void verify_setitimer(unsigned int i)
 {
 	pid_t pid;
-	int status;
-	int usec = 3 * time_step;
+	struct timespec res;
 	struct tcase *tc = &tcases[i];
+	int status, usec, time_step, error;
+
+	if (tc->which == ITIMER_REAL)
+		SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res);
+	else
+		SAFE_CLOCK_GETRES(CLOCK_MONOTONIC_COARSE, &res);
+
+	time_step = res.tv_nsec / 1000;
+	error = time_step;
+
+	if (time_step <= 0) {
+		time_step = 1000;
+		error = 0;
+	}
+
+	usec = 3 * time_step;
+
+	tst_res(TINFO, "clock resolution: %luns, "
+		"time step: %ius, "
+		"counter time: %ius",
+		res.tv_nsec,
+		time_step,
+		usec);
 
 	pid = SAFE_FORK();
 
@@ -76,7 +104,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 > usec + error)
 			tst_res(TFAIL, "Ending counters are out of range");
 
 		for (;;)
@@ -91,26 +119,10 @@ 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 handle user time in setitimer01

[Martin Doucha]

Hi,
suggestion below.

On 04. 11. 22 10:24, Andrea Cervesato via ltp wrote:
> Since ITIMER_VIRTUAL and ITIMER_PROF are counting down in user time, we
> need to take in consideration CLOCK_MONOTONIC_COARSE resolution. This is
> requested by the syscall, since it's considering context switch from
> user to kernel mode by using a higher clock resolution.
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>   .../kernel/syscalls/setitimer/setitimer01.c   | 54 +++++++++++--------
>   1 file changed, 33 insertions(+), 21 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c
> index eb62f02c6..5fcae53e8 100644
> --- a/testcases/kernel/syscalls/setitimer/setitimer01.c
> +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
> @@ -8,9 +8,16 @@
>   /*\
>    * [Description]
>    *
> - * Spawn a child and verify that setitimer() syscall passes, and it ends up
> + * Spawn a child, verify that setitimer() syscall passes and it ends up
>    * counting inside expected boundaries. Then verify from the parent that our
>    * syscall sent the correct signal to the child.
> + *
> + * Boundaries are choosen accordingly with system clock. In particular, when
> + * timer counts down in real time, CLOCK_MONOTONIC resolution has taken into
> + * account as our time step. When timer counts down in user time,
> + * CLOCK_MONOTONIC_COARSE is used. The reason is that CLOCK_MONOTONIC_COARSE
> + * is our system resolution in user space, since it's taking in consideration
> + * context switches from user to kernel space.
>    */
>   
>   #include <time.h>
> @@ -22,7 +29,6 @@
>   #include "tst_safe_clocks.h"
>   
>   static struct itimerval *value, *ovalue;
> -static unsigned long time_step;
>   
>   static struct tcase {
>   	int which;
> @@ -55,9 +61,31 @@ static void set_setitimer_value(int usec, int o_usec)
>   static void verify_setitimer(unsigned int i)
>   {
>   	pid_t pid;
> -	int status;
> -	int usec = 3 * time_step;
> +	struct timespec res;
>   	struct tcase *tc = &tcases[i];
> +	int status, usec, time_step, error;
> +
> +	if (tc->which == ITIMER_REAL)
> +		SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res);
> +	else
> +		SAFE_CLOCK_GETRES(CLOCK_MONOTONIC_COARSE, &res);
> +
> +	time_step = res.tv_nsec / 1000;
> +	error = time_step;
> +
> +	if (time_step <= 0) {
> +		time_step = 1000;
> +		error = 0;
> +	}

This approach looks like it'll lead to some bad edge cases when
0 < time_step < 1000. It'd be better to keep the original time_step 
detection and initialize "error" variable like this (and also rename it 
to "margin"):

int jiffy;

verify_setitimer()
{
	...
	margin = (tc->which == ITIMER_REAL) ? 0 : jiffy;
	...
}

setup()
{
	...
	SAFE_CLOCK_GETRES(CLOCK_MONOTONIC_COARSE, &res);
	jiffy = (res.tv_nsec + 999) / 1000;
	...
}

> +
> +	usec = 3 * time_step;
> +
> +	tst_res(TINFO, "clock resolution: %luns, "
> +		"time step: %ius, "
> +		"counter time: %ius",
> +		res.tv_nsec,
> +		time_step,
> +		usec);
>   
>   	pid = SAFE_FORK();
>   
> @@ -76,7 +104,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 > usec + error)
>   			tst_res(TFAIL, "Ending counters are out of range");
>   
>   		for (;;)
> @@ -91,26 +119,10 @@ 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 handle user time in setitimer01

[Cyril Hrubis]

Hi!
> Since ITIMER_VIRTUAL and ITIMER_PROF are counting down in user time, we
> need to take in consideration CLOCK_MONOTONIC_COARSE resolution. This is
> requested by the syscall, since it's considering context switch from
> user to kernel mode by using a higher clock resolution.

What exactly do you mean by this? I do not think that there is "user"
and "kernel" time, just different in-kernel timers/counters with
different precisions.

As ITIMER_VIRTUAL and ITIMER_PROF counts CPU time the precision would be
the same as the COARSE clocks which depends on jiffies and on the
compile time option CONFIG_HZ. That is because in the scheduller the CPU
time is quantized into chunks based on CONFIG_HZ.

And the ITIMER_REAL runs on hrtimers which would be the same precision
as CLOCK_REALTIME or CLOCK_MONOTONIC.

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  .../kernel/syscalls/setitimer/setitimer01.c   | 54 +++++++++++--------
>  1 file changed, 33 insertions(+), 21 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c b/testcases/kernel/syscalls/setitimer/setitimer01.c
> index eb62f02c6..5fcae53e8 100644
> --- a/testcases/kernel/syscalls/setitimer/setitimer01.c
> +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
> @@ -8,9 +8,16 @@
>  /*\
>   * [Description]
>   *
> - * Spawn a child and verify that setitimer() syscall passes, and it ends up
> + * Spawn a child, verify that setitimer() syscall passes and it ends up
>   * counting inside expected boundaries. Then verify from the parent that our
>   * syscall sent the correct signal to the child.
> + *
> + * Boundaries are choosen accordingly with system clock. In particular, when
> + * timer counts down in real time, CLOCK_MONOTONIC resolution has taken into
> + * account as our time step. When timer counts down in user time,
> + * CLOCK_MONOTONIC_COARSE is used. The reason is that CLOCK_MONOTONIC_COARSE
> + * is our system resolution in user space, since it's taking in consideration
> + * context switches from user to kernel space.
>   */
>  
>  #include <time.h>
> @@ -22,7 +29,6 @@
>  #include "tst_safe_clocks.h"
>  
>  static struct itimerval *value, *ovalue;
> -static unsigned long time_step;
>  
>  static struct tcase {
>  	int which;
> @@ -55,9 +61,31 @@ static void set_setitimer_value(int usec, int o_usec)
>  static void verify_setitimer(unsigned int i)
>  {
>  	pid_t pid;
> -	int status;
> -	int usec = 3 * time_step;
> +	struct timespec res;
>  	struct tcase *tc = &tcases[i];
> +	int status, usec, time_step, error;
> +
> +	if (tc->which == ITIMER_REAL)
> +		SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res);
> +	else
> +		SAFE_CLOCK_GETRES(CLOCK_MONOTONIC_COARSE, &res);

This is not going to change, so we may as well get the fine/coarse
resolution once in the test setup and initialize fine/coarse time step
once as well.

> +	time_step = res.tv_nsec / 1000;
> +	error = time_step;
> +
> +	if (time_step <= 0) {
> +		time_step = 1000;
> +		error = 0;
> +	}
> +
> +	usec = 3 * time_step;
> +
> +	tst_res(TINFO, "clock resolution: %luns, "
> +		"time step: %ius, "
> +		"counter time: %ius",
> +		res.tv_nsec,
> +		time_step,
> +		usec);
>  
>  	pid = SAFE_FORK();
>  
> @@ -76,7 +104,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 > usec + error)
>  			tst_res(TFAIL, "Ending counters are out of range");
>  
>  		for (;;)
> @@ -91,26 +119,10 @@ 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

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v1] Correctly handle user time in setitimer01

[Andrea Cervesato via ltp]

Hi!

On 11/4/22 13:46, Martin Doucha wrote:
> Hi,
> suggestion below.
>
> On 04. 11. 22 10:24, Andrea Cervesato via ltp wrote:
>> Since ITIMER_VIRTUAL and ITIMER_PROF are counting down in user time, we
>> need to take in consideration CLOCK_MONOTONIC_COARSE resolution. This is
>> requested by the syscall, since it's considering context switch from
>> user to kernel mode by using a higher clock resolution.
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   .../kernel/syscalls/setitimer/setitimer01.c   | 54 +++++++++++--------
>>   1 file changed, 33 insertions(+), 21 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/setitimer/setitimer01.c 
>> b/testcases/kernel/syscalls/setitimer/setitimer01.c
>> index eb62f02c6..5fcae53e8 100644
>> --- a/testcases/kernel/syscalls/setitimer/setitimer01.c
>> +++ b/testcases/kernel/syscalls/setitimer/setitimer01.c
>> @@ -8,9 +8,16 @@
>>   /*\
>>    * [Description]
>>    *
>> - * Spawn a child and verify that setitimer() syscall passes, and it 
>> ends up
>> + * Spawn a child, verify that setitimer() syscall passes and it ends up
>>    * counting inside expected boundaries. Then verify from the parent 
>> that our
>>    * syscall sent the correct signal to the child.
>> + *
>> + * Boundaries are choosen accordingly with system clock. In 
>> particular, when
>> + * timer counts down in real time, CLOCK_MONOTONIC resolution has 
>> taken into
>> + * account as our time step. When timer counts down in user time,
>> + * CLOCK_MONOTONIC_COARSE is used. The reason is that 
>> CLOCK_MONOTONIC_COARSE
>> + * is our system resolution in user space, since it's taking in 
>> consideration
>> + * context switches from user to kernel space.
>>    */
>>     #include <time.h>
>> @@ -22,7 +29,6 @@
>>   #include "tst_safe_clocks.h"
>>     static struct itimerval *value, *ovalue;
>> -static unsigned long time_step;
>>     static struct tcase {
>>       int which;
>> @@ -55,9 +61,31 @@ static void set_setitimer_value(int usec, int o_usec)
>>   static void verify_setitimer(unsigned int i)
>>   {
>>       pid_t pid;
>> -    int status;
>> -    int usec = 3 * time_step;
>> +    struct timespec res;
>>       struct tcase *tc = &tcases[i];
>> +    int status, usec, time_step, error;
>> +
>> +    if (tc->which == ITIMER_REAL)
>> +        SAFE_CLOCK_GETRES(CLOCK_MONOTONIC, &res);
>> +    else
>> +        SAFE_CLOCK_GETRES(CLOCK_MONOTONIC_COARSE, &res);
>> +
>> +    time_step = res.tv_nsec / 1000;
>> +    error = time_step;
>> +
>> +    if (time_step <= 0) {
>> +        time_step = 1000;
>> +        error = 0;
>> +    }
>
> This approach looks like it'll lead to some bad edge cases when
> 0 < time_step < 1000. It'd be better to keep the original time_step 
> detection and initialize "error" variable like this (and also rename 
> it to "margin"):
>
> int jiffy;
>
> verify_setitimer()
> {
>     ...
>     margin = (tc->which == ITIMER_REAL) ? 0 : jiffy;
Here we can't take in consideration CLOCK_MONOTONIC_COARSE resolution by 
default, because on ITIMER_REAL we are having a clock resolution given 
by CLOCK_MONOTONIC. And unfortunately we are not sure it's under the 
millisecond resolution all the times, which means margin > 0. For this 
reason, in the patch we are fetching clock resolution in a different 
way, according with the counter timer. We can fetch different 
resolutions from setup tho and using inside the test code.
>     ...
> }
>
> setup()
> {
>     ...
>     SAFE_CLOCK_GETRES(CLOCK_MONOTONIC_COARSE, &res);
>     jiffy = (res.tv_nsec + 999) / 1000;
>     ...
> }
>
>> +
>> +    usec = 3 * time_step;
>> +
>> +    tst_res(TINFO, "clock resolution: %luns, "
>> +        "time step: %ius, "
>> +        "counter time: %ius",
>> +        res.tv_nsec,
>> +        time_step,
>> +        usec);
>>         pid = SAFE_FORK();
>>   @@ -76,7 +104,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 
>> > usec + error)
>>               tst_res(TFAIL, "Ending counters are out of range");
>>             for (;;)
>> @@ -91,26 +119,10 @@ 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 handle user time in setitimer01

[Martin Doucha]

On 04. 11. 22 15:13, Cyril Hrubis wrote:
> Hi!
>> Since ITIMER_VIRTUAL and ITIMER_PROF are counting down in user time, we
>> need to take in consideration CLOCK_MONOTONIC_COARSE resolution. This is
>> requested by the syscall, since it's considering context switch from
>> user to kernel mode by using a higher clock resolution.
> 
> What exactly do you mean by this? I do not think that there is "user"
> and "kernel" time, just different in-kernel timers/counters with
> different precisions.
> 
> As ITIMER_VIRTUAL and ITIMER_PROF counts CPU time the precision would be
> the same as the COARSE clocks which depends on jiffies and on the
> compile time option CONFIG_HZ. That is because in the scheduller the CPU
> time is quantized into chunks based on CONFIG_HZ.
> 
> And the ITIMER_REAL runs on hrtimers which would be the same precision
> as CLOCK_REALTIME or CLOCK_MONOTONIC.

The kernel mangles the new timer values for ITIMER_PROF and 
ITIMER_VIRTUAL. It'll always add one extra jiffy to the actual timer 
value you've passed. For ITIMER_REAL, the timer value gets set as is.

 From set_cpu_itimer() in kernel/time/itimer.c:

if (oval || nval) {
	if (nval > 0)
		nval += TICK_NSEC;
	set_process_cpu_timer(tsk, clock_id, &nval, &oval);
}

nval = new timer value passed to setitimer() converted to nanoseconds
TICK_NSEC = 1 jiffy in nanoseconds

-- 
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 handle user time in setitimer01

[Martin Doucha]

On 04. 11. 22 15:20, Andrea Cervesato wrote:
> Hi!
> 
> On 11/4/22 13:46, Martin Doucha wrote:
>> This approach looks like it'll lead to some bad edge cases when
>> 0 < time_step < 1000. It'd be better to keep the original time_step 
>> detection and initialize "error" variable like this (and also rename 
>> it to "margin"):
>>
>> int jiffy;
>>
>> verify_setitimer()
>> {
>>     ...
>>     margin = (tc->which == ITIMER_REAL) ? 0 : jiffy;
> Here we can't take in consideration CLOCK_MONOTONIC_COARSE resolution by 
> default, because on ITIMER_REAL we are having a clock resolution given 
> by CLOCK_MONOTONIC. And unfortunately we are not sure it's under the 
> millisecond resolution all the times, which means margin > 0. For this 
> reason, in the patch we are fetching clock resolution in a different 
> way, according with the counter timer. We can fetch different 
> resolutions from setup tho and using inside the test code.

There is nothing preventing you from using CLOCK_MONOTONIC_COARSE 
resolution as time_step even for ITIMER_REAL. The only constraints are 
that the timer value must be:
1) higher than CLOCK_MONOTONIC resolution (preferably a multiple of it)
2) large enough so that you can call setitimer() again before the timer 
expires

CLOCK_MONOTONIC_COARSE resolution should satisfy the first constraint by 
definition.

-- 
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 handle user time in setitimer01

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1563 bytes --]

Martin Doucha <mdoucha@suse.cz> wrote:

On 04. 11. 22 15:13, Cyril Hrubis wrote:

> As ITIMER_VIRTUAL and ITIMER_PROF counts CPU time the precision would be
> > the same as the COARSE clocks which depends on jiffies and on the
> > compile time option CONFIG_HZ. That is because in the scheduller the CPU
> > time is quantized into chunks based on CONFIG_HZ.
> >
> > And the ITIMER_REAL runs on hrtimers which would be the same precision
> > as CLOCK_REALTIME or CLOCK_MONOTONIC.
>

This description of the rationale is what I was trying to confirm.
Thanks for sharing.



>
> The kernel mangles the new timer values for ITIMER_PROF and
> ITIMER_VIRTUAL. It'll always add one extra jiffy to the actual timer
> value you've passed. For ITIMER_REAL, the timer value gets set as is.
>


Right, but I don't think it could be said as "mangles" cause that is
on purpose, in case people give a very tiny 'tv_usec' to setiitimer(),
so plus one jiffy to guarantee at least the timer takes effect.



>
>  From set_cpu_itimer() in kernel/time/itimer.c:
>
> if (oval || nval) {
>         if (nval > 0)
>                 nval += TICK_NSEC;
>         set_process_cpu_timer(tsk, clock_id, &nval, &oval);
> }
>
> nval = new timer value passed to setitimer() converted to nanoseconds
> TICK_NSEC = 1 jiffy in nanoseconds
>
> --
> 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
>
>

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 3242 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v1] Correctly handle user time in setitimer01

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1697 bytes --]

On Fri, Nov 4, 2022 at 11:44 PM Martin Doucha <mdoucha@suse.cz> wrote:

> On 04. 11. 22 15:20, Andrea Cervesato wrote:
> > Hi!
> >
> > On 11/4/22 13:46, Martin Doucha wrote:
> >> This approach looks like it'll lead to some bad edge cases when
> >> 0 < time_step < 1000. It'd be better to keep the original time_step
> >> detection and initialize "error" variable like this (and also rename
> >> it to "margin"):
> >>
> >> int jiffy;
> >>
> >> verify_setitimer()
> >> {
> >>     ...
> >>     margin = (tc->which == ITIMER_REAL) ? 0 : jiffy;
> > Here we can't take in consideration CLOCK_MONOTONIC_COARSE resolution by
> > default, because on ITIMER_REAL we are having a clock resolution given
> > by CLOCK_MONOTONIC. And unfortunately we are not sure it's under the
> > millisecond resolution all the times, which means margin > 0. For this
> > reason, in the patch we are fetching clock resolution in a different
> > way, according with the counter timer. We can fetch different
> > resolutions from setup tho and using inside the test code.
>
> There is nothing preventing you from using CLOCK_MONOTONIC_COARSE
> resolution as time_step even for ITIMER_REAL. The only constraints are
> that the timer value must be:
> 1) higher than CLOCK_MONOTONIC resolution (preferably a multiple of it)



> 2) large enough so that you can call setitimer() again before the timer
> expires

Just finding a proper size should be fine, because next I plan to
split ovalue check from the signal test. That means we can give both
SEC and USEC for setitimer() verification.

Also, the interval timer should be tested as well, it will be rebased
(on this problem fixing) in a separate patch.


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2976 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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