Re: [PATCH v2 3/3] process cputimer is moving faster than its corresponding clock
On Mon, 2013-04-29 at 02:25 -0400, KOSAKI Motohiro wrote:
> (4/27/13 12:41 AM), Olivier Langlois wrote:
> >
> >
> > Add thread group delta to cpu timer sample when computing a timer
> > expiration.
> >
> > This is mandatory to make sure that the posix cpu timer does not fire too
> > soon relative to the process cpu clock which do include the task group
> > delta.
> >
> > test case to validate the patch is glibc-2.17/rt/tst-cputimer1.c
>
> First, I could reproduce this issue. thanks. Second, actually, this issue is
> not
> cause by race. This just occur by timer initialization mistake. I'll show you
> the smallest fix.
>
>
Great!
>
> > @@ -697,7 +755,8 @@ static int posix_cpu_timer_set(struct k_itimer *timer,
> > int flags,
> > if (CPUCLOCK_PERTHREAD(timer->it_clock)) {
> > cpu_clock_sample(timer->it_clock, p, );
> > } else {
> > - cpu_timer_sample_group(timer->it_clock, p, );
> > + cpu_timer_sample_group(timer->it_clock, p, ,
> > + CPUTIMER_NEED_DELTA);
>
> POSIX says,
>
> http://pubs.opengroup.org/onlinepubs/7908799/xsh/timer_gettime.html
> > If the argument ovalue is not NULL, the function timer_settime() stores,
> > in the location referenced by ovalue, a value representing the previous
> > amount of time before the timer would have expired or zero if the timer
> > was disarmed, together with the previous timer reload value. The members
> > of ovalue are subject to the resolution of the timer, and they are the
> > same values that would be returned by a timer_gettime() call at that point
> > in time.
>
> ^^
>
>
> but your posix_cpu_timer_set() and posix_cpu_timer_get() are not consistent.
> I'm worry
> about this.
>
>
> > }
> >
> > if (old) {
> > @@ -845,7 +904,8 @@ static void posix_cpu_timer_get(struct k_itimer *timer,
> > struct itimerspec *itp)
> > read_unlock(_lock);
> > goto dead;
> > } else {
> > - cpu_timer_sample_group(timer->it_clock, p, );
> > + cpu_timer_sample_group(timer->it_clock, p, ,
> > + CPUTIMER_NO_DELTA);
> >
> > clear_dead = (unlikely(p->exit_state) &&
> > thread_group_empty(p));
> > }
>
> --
I have tried to minimize rq locks contention to strict minimum. If to
remain POSIX compliant, it is required to also use CPUTIMER_NEED_DELTA,
so be it. I have no objections.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] process cputimer is moving faster than its corresponding clock
(4/27/13 12:41 AM), Olivier Langlois wrote:
>
>
> Add thread group delta to cpu timer sample when computing a timer expiration.
>
> This is mandatory to make sure that the posix cpu timer does not fire too
> soon relative to the process cpu clock which do include the task group delta.
>
> test case to validate the patch is glibc-2.17/rt/tst-cputimer1.c
First, I could reproduce this issue. thanks. Second, actually, this issue is not
cause by race. This just occur by timer initialization mistake. I'll show you
the smallest fix.
> @@ -697,7 +755,8 @@ static int posix_cpu_timer_set(struct k_itimer *timer,
> int flags,
> if (CPUCLOCK_PERTHREAD(timer->it_clock)) {
> cpu_clock_sample(timer->it_clock, p, );
> } else {
> - cpu_timer_sample_group(timer->it_clock, p, );
> + cpu_timer_sample_group(timer->it_clock, p, ,
> + CPUTIMER_NEED_DELTA);
POSIX says,
http://pubs.opengroup.org/onlinepubs/7908799/xsh/timer_gettime.html
> If the argument ovalue is not NULL, the function timer_settime() stores,
> in the location referenced by ovalue, a value representing the previous
> amount of time before the timer would have expired or zero if the timer
> was disarmed, together with the previous timer reload value. The members
> of ovalue are subject to the resolution of the timer, and they are the
> same values that would be returned by a timer_gettime() call at that point in
> time.
^^
but your posix_cpu_timer_set() and posix_cpu_timer_get() are not consistent.
I'm worry
about this.
> }
>
> if (old) {
> @@ -845,7 +904,8 @@ static void posix_cpu_timer_get(struct k_itimer *timer,
> struct itimerspec *itp)
> read_unlock(_lock);
> goto dead;
> } else {
> - cpu_timer_sample_group(timer->it_clock, p, );
> + cpu_timer_sample_group(timer->it_clock, p, ,
> + CPUTIMER_NO_DELTA);
>
> clear_dead = (unlikely(p->exit_state) &&
> thread_group_empty(p));
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] process cputimer is moving faster than its corresponding clock
(4/27/13 12:41 AM), Olivier Langlois wrote:
Add thread group delta to cpu timer sample when computing a timer expiration.
This is mandatory to make sure that the posix cpu timer does not fire too
soon relative to the process cpu clock which do include the task group delta.
test case to validate the patch is glibc-2.17/rt/tst-cputimer1.c
First, I could reproduce this issue. thanks. Second, actually, this issue is not
cause by race. This just occur by timer initialization mistake. I'll show you
the smallest fix.
@@ -697,7 +755,8 @@ static int posix_cpu_timer_set(struct k_itimer *timer,
int flags,
if (CPUCLOCK_PERTHREAD(timer-it_clock)) {
cpu_clock_sample(timer-it_clock, p, val);
} else {
- cpu_timer_sample_group(timer-it_clock, p, val);
+ cpu_timer_sample_group(timer-it_clock, p, val,
+ CPUTIMER_NEED_DELTA);
POSIX says,
http://pubs.opengroup.org/onlinepubs/7908799/xsh/timer_gettime.html
If the argument ovalue is not NULL, the function timer_settime() stores,
in the location referenced by ovalue, a value representing the previous
amount of time before the timer would have expired or zero if the timer
was disarmed, together with the previous timer reload value. The members
of ovalue are subject to the resolution of the timer, and they are the
same values that would be returned by a timer_gettime() call at that point in
time.
^^
but your posix_cpu_timer_set() and posix_cpu_timer_get() are not consistent.
I'm worry
about this.
}
if (old) {
@@ -845,7 +904,8 @@ static void posix_cpu_timer_get(struct k_itimer *timer,
struct itimerspec *itp)
read_unlock(tasklist_lock);
goto dead;
} else {
- cpu_timer_sample_group(timer-it_clock, p, now);
+ cpu_timer_sample_group(timer-it_clock, p, now,
+ CPUTIMER_NO_DELTA);
clear_dead = (unlikely(p-exit_state)
thread_group_empty(p));
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] process cputimer is moving faster than its corresponding clock
On Mon, 2013-04-29 at 02:25 -0400, KOSAKI Motohiro wrote:
(4/27/13 12:41 AM), Olivier Langlois wrote:
Add thread group delta to cpu timer sample when computing a timer
expiration.
This is mandatory to make sure that the posix cpu timer does not fire too
soon relative to the process cpu clock which do include the task group
delta.
test case to validate the patch is glibc-2.17/rt/tst-cputimer1.c
First, I could reproduce this issue. thanks. Second, actually, this issue is
not
cause by race. This just occur by timer initialization mistake. I'll show you
the smallest fix.
Great!
@@ -697,7 +755,8 @@ static int posix_cpu_timer_set(struct k_itimer *timer,
int flags,
if (CPUCLOCK_PERTHREAD(timer-it_clock)) {
cpu_clock_sample(timer-it_clock, p, val);
} else {
- cpu_timer_sample_group(timer-it_clock, p, val);
+ cpu_timer_sample_group(timer-it_clock, p, val,
+ CPUTIMER_NEED_DELTA);
POSIX says,
http://pubs.opengroup.org/onlinepubs/7908799/xsh/timer_gettime.html
If the argument ovalue is not NULL, the function timer_settime() stores,
in the location referenced by ovalue, a value representing the previous
amount of time before the timer would have expired or zero if the timer
was disarmed, together with the previous timer reload value. The members
of ovalue are subject to the resolution of the timer, and they are the
same values that would be returned by a timer_gettime() call at that point
in time.
^^
but your posix_cpu_timer_set() and posix_cpu_timer_get() are not consistent.
I'm worry
about this.
}
if (old) {
@@ -845,7 +904,8 @@ static void posix_cpu_timer_get(struct k_itimer *timer,
struct itimerspec *itp)
read_unlock(tasklist_lock);
goto dead;
} else {
- cpu_timer_sample_group(timer-it_clock, p, now);
+ cpu_timer_sample_group(timer-it_clock, p, now,
+ CPUTIMER_NO_DELTA);
clear_dead = (unlikely(p-exit_state)
thread_group_empty(p));
}
--
I have tried to minimize rq locks contention to strict minimum. If to
remain POSIX compliant, it is required to also use CPUTIMER_NEED_DELTA,
so be it. I have no objections.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
