From 0aec63e67c69545ca757a73a66f5dcf05fa484bf Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Fri, 6 Jan 2006 15:36:48 -0800 Subject: [PATCH] Fix posix-cpu-timers sched_time accumulation I've spent the past 3 days digging into a glibc testsuite failure in current CVS, specifically libc/rt/tst-cputimer1.c The thr1 and thr2 timers fire too early in the second pass of this test. The second pass is noteworthy because it makes use of intervals, whereas the first pass does not. All throughout the posix-cpu-timers.c code, the calculation of the process sched_time sum is implemented roughly as: unsigned long long sum; sum = tsk->signal->sched_time; t = tsk; do { sum += t->sched_time; t = next_thread(t); } while (t != tsk); In fact this is the exact scheme used by check_process_timers(). In the case of check_process_timers(), current->sched_time has just been updated (via scheduler_tick(), which is invoked by update_process_times(), which subsequently invokes run_posix_cpu_timers()) So there is no special processing necessary wrt. that. In other contexts, we have to allot for the fact that tsk->sched_time might be a bit out of date if we are current. And the posix-cpu-timers.c code uses current_sched_time() to deal with that. Unfortunately it does so in an erroneous and inconsistent manner in one spot which is what results in the early timer firing. In cpu_clock_sample_group_locked(), it does this: cpu->sched = p->signal->sched_time; /* Add in each other live thread. */ while ((t = next_thread(t)) != p) { cpu->sched += t->sched_time; } if (p->tgid == current->tgid) { /* * We're sampling ourselves, so include the * cycles not yet banked. We still omit * other threads running on other CPUs, * so the total can always be behind as * much as max(nthreads-1,ncpus) * (NSEC_PER_SEC/HZ). */ cpu->sched += current_sched_time(current); } else { cpu->sched += p->sched_time; } The problem is the "p->tgid == current->tgid" test. If "p" is not current, and the tgids are the same, we will add the process t->sched_time twice into cpu->sched and omit "p"'s sched_time which is very very very wrong. posix-cpu-timers.c has a helper function, sched_ns(p) which takes care of this, so my fix is to use that here instead of this special tgid test. The fact that current can be one of the sub-threads of "p" points out that we could make things a little bit more accurate, perhaps by using sched_ns() on every thread we process in these loops. It also points out that we don't use the most accurate value for threads in the group actively running other cpus (and this is mentioned in the comment). But that is a future enhancement, and this fix here definitely makes sense. Signed-off-by: David S. Miller Signed-off-by: Linus Torvalds --- kernel/posix-cpu-timers.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) (limited to 'kernel/posix-cpu-timers.c') diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index cae4f5728997..4c68edff900b 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c @@ -238,18 +238,7 @@ static int cpu_clock_sample_group_locked(unsigned int clock_idx, while ((t = next_thread(t)) != p) { cpu->sched += t->sched_time; } - if (p->tgid == current->tgid) { - /* - * We're sampling ourselves, so include the - * cycles not yet banked. We still omit - * other threads running on other CPUs, - * so the total can always be behind as - * much as max(nthreads-1,ncpus) * (NSEC_PER_SEC/HZ). - */ - cpu->sched += current_sched_time(current); - } else { - cpu->sched += p->sched_time; - } + cpu->sched += sched_ns(p); break; } return 0; -- cgit v1.2.2 From a924b04ddea9788e09f387fe19ccbede5f09ddd8 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Mon, 9 Jan 2006 20:52:27 -0800 Subject: [PATCH] hrtimer: make clockid_t arguments const add const arguments to the posix-timers.h API functions Signed-off-by: Thomas Gleixner Signed-off-by: Ingo Molnar Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/posix-cpu-timers.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) (limited to 'kernel/posix-cpu-timers.c') diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index 4c68edff900b..abf6990c6eb5 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c @@ -7,7 +7,7 @@ #include #include -static int check_clock(clockid_t which_clock) +static int check_clock(const clockid_t which_clock) { int error = 0; struct task_struct *p; @@ -31,7 +31,7 @@ static int check_clock(clockid_t which_clock) } static inline union cpu_time_count -timespec_to_sample(clockid_t which_clock, const struct timespec *tp) +timespec_to_sample(const clockid_t which_clock, const struct timespec *tp) { union cpu_time_count ret; ret.sched = 0; /* high half always zero when .cpu used */ @@ -43,7 +43,7 @@ timespec_to_sample(clockid_t which_clock, const struct timespec *tp) return ret; } -static void sample_to_timespec(clockid_t which_clock, +static void sample_to_timespec(const clockid_t which_clock, union cpu_time_count cpu, struct timespec *tp) { @@ -55,7 +55,7 @@ static void sample_to_timespec(clockid_t which_clock, } } -static inline int cpu_time_before(clockid_t which_clock, +static inline int cpu_time_before(const clockid_t which_clock, union cpu_time_count now, union cpu_time_count then) { @@ -65,7 +65,7 @@ static inline int cpu_time_before(clockid_t which_clock, return cputime_lt(now.cpu, then.cpu); } } -static inline void cpu_time_add(clockid_t which_clock, +static inline void cpu_time_add(const clockid_t which_clock, union cpu_time_count *acc, union cpu_time_count val) { @@ -75,7 +75,7 @@ static inline void cpu_time_add(clockid_t which_clock, acc->cpu = cputime_add(acc->cpu, val.cpu); } } -static inline union cpu_time_count cpu_time_sub(clockid_t which_clock, +static inline union cpu_time_count cpu_time_sub(const clockid_t which_clock, union cpu_time_count a, union cpu_time_count b) { @@ -151,7 +151,7 @@ static inline unsigned long long sched_ns(struct task_struct *p) return (p == current) ? current_sched_time(p) : p->sched_time; } -int posix_cpu_clock_getres(clockid_t which_clock, struct timespec *tp) +int posix_cpu_clock_getres(const clockid_t which_clock, struct timespec *tp) { int error = check_clock(which_clock); if (!error) { @@ -169,7 +169,7 @@ int posix_cpu_clock_getres(clockid_t which_clock, struct timespec *tp) return error; } -int posix_cpu_clock_set(clockid_t which_clock, const struct timespec *tp) +int posix_cpu_clock_set(const clockid_t which_clock, const struct timespec *tp) { /* * You can never reset a CPU clock, but we check for other errors @@ -186,7 +186,7 @@ int posix_cpu_clock_set(clockid_t which_clock, const struct timespec *tp) /* * Sample a per-thread clock for the given task. */ -static int cpu_clock_sample(clockid_t which_clock, struct task_struct *p, +static int cpu_clock_sample(const clockid_t which_clock, struct task_struct *p, union cpu_time_count *cpu) { switch (CPUCLOCK_WHICH(which_clock)) { @@ -248,7 +248,7 @@ static int cpu_clock_sample_group_locked(unsigned int clock_idx, * Sample a process (thread group) clock for the given group_leader task. * Must be called with tasklist_lock held for reading. */ -static int cpu_clock_sample_group(clockid_t which_clock, +static int cpu_clock_sample_group(const clockid_t which_clock, struct task_struct *p, union cpu_time_count *cpu) { @@ -262,7 +262,7 @@ static int cpu_clock_sample_group(clockid_t which_clock, } -int posix_cpu_clock_get(clockid_t which_clock, struct timespec *tp) +int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *tp) { const pid_t pid = CPUCLOCK_PID(which_clock); int error = -EINVAL; @@ -1399,7 +1399,7 @@ void set_process_cpu_timer(struct task_struct *tsk, unsigned int clock_idx, static long posix_cpu_clock_nanosleep_restart(struct restart_block *); -int posix_cpu_nsleep(clockid_t which_clock, int flags, +int posix_cpu_nsleep(const clockid_t which_clock, int flags, struct timespec *rqtp) { struct restart_block *restart_block = @@ -1503,11 +1503,13 @@ posix_cpu_clock_nanosleep_restart(struct restart_block *restart_block) #define PROCESS_CLOCK MAKE_PROCESS_CPUCLOCK(0, CPUCLOCK_SCHED) #define THREAD_CLOCK MAKE_THREAD_CPUCLOCK(0, CPUCLOCK_SCHED) -static int process_cpu_clock_getres(clockid_t which_clock, struct timespec *tp) +static int process_cpu_clock_getres(const clockid_t which_clock, + struct timespec *tp) { return posix_cpu_clock_getres(PROCESS_CLOCK, tp); } -static int process_cpu_clock_get(clockid_t which_clock, struct timespec *tp) +static int process_cpu_clock_get(const clockid_t which_clock, + struct timespec *tp) { return posix_cpu_clock_get(PROCESS_CLOCK, tp); } @@ -1516,16 +1518,18 @@ static int process_cpu_timer_create(struct k_itimer *timer) timer->it_clock = PROCESS_CLOCK; return posix_cpu_timer_create(timer); } -static int process_cpu_nsleep(clockid_t which_clock, int flags, +static int process_cpu_nsleep(const clockid_t which_clock, int flags, struct timespec *rqtp) { return posix_cpu_nsleep(PROCESS_CLOCK, flags, rqtp); } -static int thread_cpu_clock_getres(clockid_t which_clock, struct timespec *tp) +static int thread_cpu_clock_getres(const clockid_t which_clock, + struct timespec *tp) { return posix_cpu_clock_getres(THREAD_CLOCK, tp); } -static int thread_cpu_clock_get(clockid_t which_clock, struct timespec *tp) +static int thread_cpu_clock_get(const clockid_t which_clock, + struct timespec *tp) { return posix_cpu_clock_get(THREAD_CLOCK, tp); } @@ -1534,7 +1538,7 @@ static int thread_cpu_timer_create(struct k_itimer *timer) timer->it_clock = THREAD_CLOCK; return posix_cpu_timer_create(timer); } -static int thread_cpu_nsleep(clockid_t which_clock, int flags, +static int thread_cpu_nsleep(const clockid_t which_clock, int flags, struct timespec *rqtp) { return -EINVAL; -- cgit v1.2.2 From 97735f25d2ba898ec5e13746451525580631c834 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Mon, 9 Jan 2006 20:52:37 -0800 Subject: [PATCH] hrtimer: switch clock_nanosleep to hrtimer nanosleep API Switch clock_nanosleep to use the new nanosleep functions in hrtimer.c Signed-off-by: Thomas Gleixner Signed-off-by: Ingo Molnar Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/posix-cpu-timers.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'kernel/posix-cpu-timers.c') diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index abf6990c6eb5..520f6c59948d 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c @@ -1400,7 +1400,7 @@ void set_process_cpu_timer(struct task_struct *tsk, unsigned int clock_idx, static long posix_cpu_clock_nanosleep_restart(struct restart_block *); int posix_cpu_nsleep(const clockid_t which_clock, int flags, - struct timespec *rqtp) + struct timespec *rqtp, struct timespec __user *rmtp) { struct restart_block *restart_block = ¤t_thread_info()->restart_block; @@ -1425,7 +1425,6 @@ int posix_cpu_nsleep(const clockid_t which_clock, int flags, error = posix_cpu_timer_create(&timer); timer.it_process = current; if (!error) { - struct timespec __user *rmtp; static struct itimerspec zero_it; struct itimerspec it = { .it_value = *rqtp, .it_interval = {} }; @@ -1472,7 +1471,6 @@ int posix_cpu_nsleep(const clockid_t which_clock, int flags, /* * Report back to the user the time still remaining. */ - rmtp = (struct timespec __user *) restart_block->arg1; if (rmtp != NULL && !(flags & TIMER_ABSTIME) && copy_to_user(rmtp, &it.it_value, sizeof *rmtp)) return -EFAULT; @@ -1480,6 +1478,7 @@ int posix_cpu_nsleep(const clockid_t which_clock, int flags, restart_block->fn = posix_cpu_clock_nanosleep_restart; /* Caller already set restart_block->arg1 */ restart_block->arg0 = which_clock; + restart_block->arg1 = (unsigned long) rmtp; restart_block->arg2 = rqtp->tv_sec; restart_block->arg3 = rqtp->tv_nsec; @@ -1493,10 +1492,15 @@ static long posix_cpu_clock_nanosleep_restart(struct restart_block *restart_block) { clockid_t which_clock = restart_block->arg0; - struct timespec t = { .tv_sec = restart_block->arg2, - .tv_nsec = restart_block->arg3 }; + struct timespec __user *rmtp; + struct timespec t; + + rmtp = (struct timespec __user *) restart_block->arg1; + t.tv_sec = restart_block->arg2; + t.tv_nsec = restart_block->arg3; + restart_block->fn = do_no_restart_syscall; - return posix_cpu_nsleep(which_clock, TIMER_ABSTIME, &t); + return posix_cpu_nsleep(which_clock, TIMER_ABSTIME, &t, rmtp); } @@ -1519,9 +1523,10 @@ static int process_cpu_timer_create(struct k_itimer *timer) return posix_cpu_timer_create(timer); } static int process_cpu_nsleep(const clockid_t which_clock, int flags, - struct timespec *rqtp) + struct timespec *rqtp, + struct timespec __user *rmtp) { - return posix_cpu_nsleep(PROCESS_CLOCK, flags, rqtp); + return posix_cpu_nsleep(PROCESS_CLOCK, flags, rqtp, rmtp); } static int thread_cpu_clock_getres(const clockid_t which_clock, struct timespec *tp) @@ -1539,7 +1544,7 @@ static int thread_cpu_timer_create(struct k_itimer *timer) return posix_cpu_timer_create(timer); } static int thread_cpu_nsleep(const clockid_t which_clock, int flags, - struct timespec *rqtp) + struct timespec *rqtp, struct timespec __user *rmtp) { return -EINVAL; } -- cgit v1.2.2