Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 11 May 2013 02:56:14 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 2/7] posix-cpu-timers: fix acounting delta_exec twice |
| |
On Fri, May 03, 2013 at 12:47:43AM -0400, kosaki.motohiro@gmail.com wrote: > From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > > Currently glibc rt/tst-cpuclock2 test(*) sporadically fails because > scheduler delta can be accounted twice from thread_group_cputimer() > and account_group_exec_runtime(). > > Finally, clock_nanosleep() wakes up before an argument. This is posix > violation. This issue was introduced by commit d670ec1317 (posix-cpu-timers: > Cure SMP wobbles). > > (*) http://sourceware.org/git/?p=glibc.git;a=blob;f=rt/tst-cpuclock2.c;h=6752721717f959e89c0d692b3f1ee082d507eec2;hb=HEAD > > Cc: Olivier Langlois <olivier@trillion01.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Frederic Weisbecker <fweisbec@gmail.com> > Cc: Ingo Molnar <mingo@kernel.org> > Signed-off-by: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> [...] > diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c > index 8fd709c..e56be4c 100644 > --- a/kernel/posix-cpu-timers.c > +++ b/kernel/posix-cpu-timers.c > @@ -220,7 +220,7 @@ static int cpu_clock_sample(const clockid_t which_clock, struct task_struct *p, > cpu->cpu = virt_ticks(p); > break; > case CPUCLOCK_SCHED: > - cpu->sched = task_sched_runtime(p); > + cpu->sched = task_sched_runtime(p, true); > break; > } > return 0; > @@ -250,8 +250,13 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times) > * values through the TIMER_ABSTIME flag, therefore we have > * to synchronize the timer to the clock every time we start > * it. > + * > + * Do not add the current delta, because > + * account_group_exec_runtime() will also this delta and we > + * wouldn't want to double account time and get ahead of > + * ourselves. > */ > - thread_group_cputime(tsk, &sum); > + thread_group_cputime(tsk, false, &sum); > raw_spin_lock_irqsave(&cputimer->lock, flags);
I wonder if we should move thread_group_cputime() inside this lock. Otherwise we can miss some updates in-between.
| ![\](/images/icornerr.gif) |