git: bb53dd56c30c - main - kern_tc.c/cputick2usec() (which is used to calculate cputime from cpu ticks) has some imprecision and, worse, huge timestep (about 20 minutes on 4GHz CPU) near 53.4 days of elapsed time.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 21 Mar 2022 13:34:09 UTC
The branch main has been updated by gnn: URL: https://cgit.FreeBSD.org/src/commit/?id=bb53dd56c30c6360fc82be762ed98b0af6b9f69f commit bb53dd56c30c6360fc82be762ed98b0af6b9f69f Author: firk <firk@cantconnect.ru> AuthorDate: 2022-03-21 13:33:11 +0000 Commit: George V. Neville-Neil <gnn@FreeBSD.org> CommitDate: 2022-03-21 13:33:46 +0000 kern_tc.c/cputick2usec() (which is used to calculate cputime from cpu ticks) has some imprecision and, worse, huge timestep (about 20 minutes on 4GHz CPU) near 53.4 days of elapsed time. kern_time.c/cputick2timespec() (it is used for clock_gettime() for querying process or thread consumed cpu time) Uses cputick2usec() and then needlessly converting usec to nsec, obviously losing precision even with fixed cputick2usec(). kern_time.c/kern_clock_getres() uses some weird (anyway wrong) formula for getting cputick resolution. PR: 262215 Reviewed by: gnn Differential Revision: https://reviews.freebsd.org/D34558 --- lib/libkvm/kvm_proc.c | 9 ++------- sys/kern/kern_tc.c | 12 +++--------- sys/kern/kern_time.c | 12 +++++------- 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/lib/libkvm/kvm_proc.c b/lib/libkvm/kvm_proc.c index 5058e86a645b..1af4ce38615f 100644 --- a/lib/libkvm/kvm_proc.c +++ b/lib/libkvm/kvm_proc.c @@ -94,15 +94,10 @@ static uint64_t cpu_tick_frequency; static uint64_t cputick2usec(uint64_t tick) { - if (cpu_tick_frequency == 0) return (0); - if (tick > 18446744073709551) /* floor(2^64 / 1000) */ - return (tick / (cpu_tick_frequency / 1000000)); - else if (tick > 18446744073709) /* floor(2^64 / 1000000) */ - return ((tick * 1000) / (cpu_tick_frequency / 1000)); - else - return ((tick * 1000000) / cpu_tick_frequency); + return ((tick / cpu_tick_frequency) * 1000000ULL) + + ((tick % cpu_tick_frequency) * 1000000ULL) / cpu_tick_frequency; } /* diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c index 01b4e8656090..fcdec7a58200 100644 --- a/sys/kern/kern_tc.c +++ b/sys/kern/kern_tc.c @@ -2155,20 +2155,14 @@ cpu_tickrate(void) * years) and in 64 bits at 4 GHz (146 years), but if we do a multiply * before divide conversion (to retain precision) we find that the * margin shrinks to 1.5 hours (one millionth of 146y). - * With a three prong approach we never lose significant bits, no - * matter what the cputick rate and length of timeinterval is. */ uint64_t cputick2usec(uint64_t tick) { - - if (tick > 18446744073709551LL) /* floor(2^64 / 1000) */ - return (tick / (cpu_tickrate() / 1000000LL)); - else if (tick > 18446744073709LL) /* floor(2^64 / 1000000) */ - return ((tick * 1000LL) / (cpu_tickrate() / 1000LL)); - else - return ((tick * 1000000LL) / cpu_tickrate()); + uint64_t tr; + tr = cpu_tickrate(); + return ((tick / tr) * 1000000ULL) + ((tick % tr) * 1000000ULL) / tr; } cpu_tick_f *cpu_ticks = tc_cpu_ticks; diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c index 0bab05c65ffc..194a23fdc9e8 100644 --- a/sys/kern/kern_time.c +++ b/sys/kern/kern_time.c @@ -245,9 +245,10 @@ sys_clock_gettime(struct thread *td, struct clock_gettime_args *uap) static inline void cputick2timespec(uint64_t runtime, struct timespec *ats) { - runtime = cputick2usec(runtime); - ats->tv_sec = runtime / 1000000; - ats->tv_nsec = runtime % 1000000 * 1000; + uint64_t tr; + tr = cpu_tickrate(); + ats->tv_sec = runtime / tr; + ats->tv_nsec = ((runtime % tr) * 1000000000ULL) / tr; } void @@ -477,10 +478,7 @@ kern_clock_getres(struct thread *td, clockid_t clock_id, struct timespec *ts) case CLOCK_THREAD_CPUTIME_ID: case CLOCK_PROCESS_CPUTIME_ID: cputime: - /* sync with cputick2usec */ - ts->tv_nsec = 1000000 / cpu_tickrate(); - if (ts->tv_nsec == 0) - ts->tv_nsec = 1000; + ts->tv_nsec = 1000000000 / cpu_tickrate() + 1; break; default: if ((int)clock_id < 0)