From owner-svn-src-head@freebsd.org Fri Feb 14 23:27:45 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id A01B0246608; Fri, 14 Feb 2020 23:27:45 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48K8gP3ch3z4RDw; Fri, 14 Feb 2020 23:27:45 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 775AA392F; Fri, 14 Feb 2020 23:27:45 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 01ENRj2P073511; Fri, 14 Feb 2020 23:27:45 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 01ENRjb4073510; Fri, 14 Feb 2020 23:27:45 GMT (envelope-from kib@FreeBSD.org) Message-Id: <202002142327.01ENRjb4073510@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Fri, 14 Feb 2020 23:27:45 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357948 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: kib X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 357948 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Feb 2020 23:27:45 -0000 Author: kib Date: Fri Feb 14 23:27:45 2020 New Revision: 357948 URL: https://svnweb.freebsd.org/changeset/base/357948 Log: Consolidate read code for timecounters and fix possible overflow in bintime()/binuptime(). The algorithm to read the consistent snapshot of current timehand is repeated in each accessor, including the details proper rollup detection and synchronization with the writer. In fact there are only two different kind of readers: one for bintime()/binuptime() which has to do the in-place calculation, and another kind which fetches some member from struct timehand. Extract the logic into type-checked macros, GETTHBINTIME() for bintime calculation, and GETTHMEMBER() for safe read of a structure' member. This way, the synchronization is only written in bintime_off() and getthmember(). In bintime_off(), use overflow-safe calculation of th_scale * delta(timecounter). In tc_windup, pre-calculate the min delta value which overflows and require slow algorithm, into the new timehands th_large_delta member. This part with overflow fix was written by Bruce Evans. Reported by: Mark Millard (the overflow issue) Tested by: pho Discussed with: emaste Sponsored by: The FreeBSD Foundation (kib) MFC after: 3 weeks Modified: head/sys/kern/kern_tc.c Modified: head/sys/kern/kern_tc.c ============================================================================== --- head/sys/kern/kern_tc.c Fri Feb 14 23:18:32 2020 (r357947) +++ head/sys/kern/kern_tc.c Fri Feb 14 23:27:45 2020 (r357948) @@ -72,6 +72,7 @@ struct timehands { struct timecounter *th_counter; int64_t th_adjustment; uint64_t th_scale; + u_int th_large_delta; u_int th_offset_count; struct bintime th_offset; struct bintime th_bintime; @@ -87,6 +88,7 @@ static struct timehands ths[16] = { [0] = { .th_counter = &dummy_timecounter, .th_scale = (uint64_t)-1 / 1000000, + .th_large_delta = 1000000, .th_offset = { .sec = 1 }, .th_generation = 1, }, @@ -202,23 +204,75 @@ tc_delta(struct timehands *th) * the comment in for a description of these 12 functions. */ -#ifdef FFCLOCK -void -fbclock_binuptime(struct bintime *bt) +static __inline void +bintime_off(struct bintime *bt, u_int off) { struct timehands *th; - unsigned int gen; + struct bintime *btp; + uint64_t scale, x; + u_int delta, gen, large_delta; do { th = timehands; gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_offset; - bintime_addx(bt, th->th_scale * tc_delta(th)); + btp = (struct bintime *)((vm_offset_t)th + off); + *bt = *btp; + scale = th->th_scale; + delta = tc_delta(th); + large_delta = th->th_large_delta; atomic_thread_fence_acq(); } while (gen == 0 || gen != th->th_generation); + + if (__predict_false(delta >= large_delta)) { + /* Avoid overflow for scale * delta. */ + x = (scale >> 32) * delta; + bt->sec += x >> 32; + bintime_addx(bt, x << 32); + bintime_addx(bt, (scale & 0xffffffff) * delta); + } else { + bintime_addx(bt, scale * delta); + } } +#define GETTHBINTIME(dst, member) \ +do { \ + _Static_assert(_Generic(((struct timehands *)NULL)->member, \ + struct bintime: 1, default: 0) == 1, \ + "struct timehands member is not of struct bintime type"); \ + bintime_off(dst, __offsetof(struct timehands, member)); \ +} while (0) +static __inline void +getthmember(void *out, size_t out_size, u_int off) +{ + struct timehands *th; + u_int gen; + + do { + th = timehands; + gen = atomic_load_acq_int(&th->th_generation); + memcpy(out, (char *)th + off, out_size); + atomic_thread_fence_acq(); + } while (gen == 0 || gen != th->th_generation); +} +#define GETTHMEMBER(dst, member) \ +do { \ + _Static_assert(_Generic(*dst, \ + __typeof(((struct timehands *)NULL)->member): 1, \ + default: 0) == 1, \ + "*dst and struct timehands member have different types"); \ + getthmember(dst, sizeof(*dst), __offsetof(struct timehands, \ + member)); \ +} while (0) + +#ifdef FFCLOCK void +fbclock_binuptime(struct bintime *bt) +{ + + GETTHBINTIME(bt, th_offset); +} + +void fbclock_nanouptime(struct timespec *tsp) { struct bintime bt; @@ -239,16 +293,8 @@ fbclock_microuptime(struct timeval *tvp) void fbclock_bintime(struct bintime *bt) { - struct timehands *th; - unsigned int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_bintime; - bintime_addx(bt, th->th_scale * tc_delta(th)); - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHBINTIME(bt, th_bintime); } void @@ -272,100 +318,55 @@ fbclock_microtime(struct timeval *tvp) void fbclock_getbinuptime(struct bintime *bt) { - struct timehands *th; - unsigned int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_offset; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(bt, th_offset); } void fbclock_getnanouptime(struct timespec *tsp) { - struct timehands *th; - unsigned int gen; + struct bintime bt; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - bintime2timespec(&th->th_offset, tsp); - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(&bt, th_offset); + bintime2timespec(&bt, tsp); } void fbclock_getmicrouptime(struct timeval *tvp) { - struct timehands *th; - unsigned int gen; + struct bintime bt; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - bintime2timeval(&th->th_offset, tvp); - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(&bt, th_offset); + bintime2timeval(&bt, tvp); } void fbclock_getbintime(struct bintime *bt) { - struct timehands *th; - unsigned int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_bintime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(bt, th_bintime); } void fbclock_getnanotime(struct timespec *tsp) { - struct timehands *th; - unsigned int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *tsp = th->th_nanotime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(tsp, th_nanotime); } void fbclock_getmicrotime(struct timeval *tvp) { - struct timehands *th; - unsigned int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *tvp = th->th_microtime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(tvp, th_microtime); } #else /* !FFCLOCK */ + void binuptime(struct bintime *bt) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_offset; - bintime_addx(bt, th->th_scale * tc_delta(th)); - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHBINTIME(bt, th_offset); } void @@ -389,16 +390,8 @@ microuptime(struct timeval *tvp) void bintime(struct bintime *bt) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_bintime; - bintime_addx(bt, th->th_scale * tc_delta(th)); - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHBINTIME(bt, th_bintime); } void @@ -422,85 +415,47 @@ microtime(struct timeval *tvp) void getbinuptime(struct bintime *bt) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_offset; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(bt, th_offset); } void getnanouptime(struct timespec *tsp) { - struct timehands *th; - u_int gen; + struct bintime bt; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - bintime2timespec(&th->th_offset, tsp); - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(&bt, th_offset); + bintime2timespec(&bt, tsp); } void getmicrouptime(struct timeval *tvp) { - struct timehands *th; - u_int gen; + struct bintime bt; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - bintime2timeval(&th->th_offset, tvp); - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(&bt, th_offset); + bintime2timeval(&bt, tvp); } void getbintime(struct bintime *bt) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *bt = th->th_bintime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(bt, th_bintime); } void getnanotime(struct timespec *tsp) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *tsp = th->th_nanotime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(tsp, th_nanotime); } void getmicrotime(struct timeval *tvp) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *tvp = th->th_microtime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(tvp, th_microtime); } #endif /* FFCLOCK */ @@ -516,15 +471,8 @@ getboottime(struct timeval *boottime) void getboottimebin(struct bintime *boottimebin) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *boottimebin = th->th_boottime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(boottimebin, th_boottime); } #ifdef FFCLOCK @@ -1040,15 +988,8 @@ getmicrotime(struct timeval *tvp) void dtrace_getnanotime(struct timespec *tsp) { - struct timehands *th; - u_int gen; - do { - th = timehands; - gen = atomic_load_acq_int(&th->th_generation); - *tsp = th->th_nanotime; - atomic_thread_fence_acq(); - } while (gen == 0 || gen != th->th_generation); + GETTHMEMBER(tsp, th_nanotime); } /* @@ -1466,6 +1407,7 @@ tc_windup(struct bintime *new_boottimebin) scale += (th->th_adjustment / 1024) * 2199; scale /= th->th_counter->tc_frequency; th->th_scale = scale * 2; + th->th_large_delta = MIN(((uint64_t)1 << 63) / scale, UINT_MAX); /* * Now that the struct timehands is again consistent, set the new