From owner-freebsd-hackers@freebsd.org Fri Mar 1 20:45:43 2019 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A839E15002A0 for ; Fri, 1 Mar 2019 20:45:43 +0000 (UTC) (envelope-from marklmi@yahoo.com) Received: from sonic308-11.consmr.mail.ne1.yahoo.com (sonic308-11.consmr.mail.ne1.yahoo.com [66.163.187.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 01DA590DDE for ; Fri, 1 Mar 2019 20:45:42 +0000 (UTC) (envelope-from marklmi@yahoo.com) X-YMail-OSG: w_ImTU8VM1lSO2WiVOJxikblHhLZ1O7DwLaxSqVU13MJYKUDrIbQmuK6SgQuOJb y1Qqeia.EbJnu1I26_s4LgMg_u5YKctBdKaODuRe08gAm3SgQ8j_PxV2qwiDYJ6Q0HDWcdiwUhlB VH25xHo7QuzPhqrWmwIgjzDUoDtJn.FXYLmf2qVhpnsfBdd.suGY5jB_4mozV3upiPu4OxksVttk N4xlCfE7oRUA5WJik8c7OVEMoMuPFLHgefgXStVFBAG2gabVQUpLdIffvPbG32SCrKDqGkuPcm1a B0W3lePvGRsxbxigZfhbPDJhnDEqZtSfLqsc5WNzxak0UGVr8wpA5vwXGNoBajBHQRWEHw9Gyrg. UwcaIlob4UyNcnSh_pE_iGjgkupRCHsUYnqSzxyvxSDuEzNvT2bNoL10PB4Vgbp9FsPWhDDTEBdg qluK_o6.gCO5xHpLbvJ7djPWwPnwRx1HbdJXCJ1KL7W6_QgpLVhkJfkbz6LntrfXHYliE2e_mzis Jwow2su9BtZeknPyXNamnonyE3sNAV1o5CLThIlxe1DXLsoyP.6d8N1JNr0edv9lQCt8Sn86BuuV PS2fIdECaFnyAJWebiPjRLQcFiI.8KBe5xtaPNSobOJplxo8b5Z7SbqmaetClsCjmMFeS5U69xIQ zl1bAQd_o0tTYICTjSLEk34AOM378RxeqXfS6.dLu3JUco6IC1oPESMsXiT4g7s63dbKfbovbNht G7E4uY4sKIndk_2ISbNBvRL3DwbdEJHI_6LcxDeLTHiO8PzQT2kB4BtKIuVXZ4YGUxlilYM9XbR9 MARAoqHsN4Lib7gziezivWZAaVp641gkkYFhoxGO4QX2e3oYUF6mBwpr6KXTYofk0sSQjzPJYQNv 3E5SAIFFJmzOksAgBw9Mj5LRGNvfCW60oSjk7QMN.fnVaop3mBNAsQ.28UB46M99Ncm.hvtNEy8I F8A_fcoQqUyTa5zeebvArQoNf.SMft6l2JtVF3EKQBmd9o71_hHJ46p81n.aFfZ0rrEdcXGavAhE TwjZoEk1Hm8HdUtA_VVirIY965jtVL7X0V3nm5YqsRVf8KXf2wpN.okFVi3CFZXmkOb7CEb2QLtu W Received: from sonic.gate.mail.ne1.yahoo.com by sonic308.consmr.mail.ne1.yahoo.com with HTTP; Fri, 1 Mar 2019 20:45:34 +0000 Received: from c-67-170-167-181.hsd1.or.comcast.net (EHLO [192.168.1.113]) ([67.170.167.181]) by smtp404.mail.ne1.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID 25490b72501497ce03a54ef8a5fe872c; Fri, 01 Mar 2019 20:45:31 +0000 (UTC) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: Re: powerpc64 head -r344018 stuck sleeping problems: th->th_scale * tc_delta(th) overflows unsigned 64 bits sometimes [patched failed] From: Mark Millard In-Reply-To: <20190301112717.GW2420@kib.kiev.ua> Date: Fri, 1 Mar 2019 12:45:29 -0800 Cc: bde@freebsd.org, freebsd-hackers Hackers , FreeBSD PowerPC ML Content-Transfer-Encoding: quoted-printable Message-Id: <679402FF-907C-43AF-B18C-8C9CC857D7A6@yahoo.com> References: <20190228145542.GT2420@kib.kiev.ua> <20190228150811.GU2420@kib.kiev.ua> <962D78C3-65BE-40C1-BB50-A0088223C17B@yahoo.com> <28C2BB0A-3DAA-4D18-A317-49A8DD52778F@yahoo.com> <20190301112717.GW2420@kib.kiev.ua> To: Konstantin Belousov X-Mailer: Apple Mail (2.3445.102.3) X-Rspamd-Queue-Id: 01DA590DDE X-Spamd-Bar: ------ X-Spamd-Result: default: False [-6.98 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.98)[-0.982,0] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Mar 2019 20:45:43 -0000 On 2019-Mar-1, at 03:27, Konstantin Belousov wrote: > On Thu, Feb 28, 2019 at 11:39:06PM -0800, Mark Millard wrote: >> [The new, trial code also has truncation occurring.] > In fact no, I do not think it is. >=20 >> An example observation of diff_scaled having an overflowed >> value was: >>=20 >> scale_factor =3D=3D 0x80da2067ac >> scale_factor*freq overflows unsigned, 64 bit representation. >> tim_offset =3D=3D 0x3da0eaeb >> tim_cnt =3D=3D 0x42dea3c4 >> tim_diff =3D=3D 0x53db8d9 >> For reference: 0x1fc9d43 =3D=3D = 0xffffffffffffffffull/scale_factor >> scaled_diff =3D=3D 0xA353A5BF3FF780CC (truncated to 64 bits) >>=20 >> But for the new, trail code: >>=20 >> 0x80da2067ac is 40 bits >> 0x53db8d9 is 27 bits >> So 67 bits, more than 63. Then: >>=20 >> x >> =3D=3D (0x80da2067ac>>32) * 0x53db8d9 >> =3D=3D 0x80 * 0x53db8d9 >> =3D=3D 0x29EDC6C80 >>=20 >> x>>32 >> =3D=3D 0x2 >>=20 >> x<<32 >> =3D=3D 0x9EDC6C8000000000 (limited to 64 bits) >> Note the truncation of: 0x29EDC6C8000000000. > Right, this is how the patch is supposed to work. Note that the = overflow > bits 'lost' due to overflow of the left shift are the same bits that = as > used to increment bt->sec: > bt->sec +=3D x >> 32; > So the 2 seconds are accounted for. Good to know. >>=20 >> Thus the "bintime_addx(bt, x << 32)" is still >> based on a truncated value. >>=20 >=20 > I must admit that 2 seconds of interval where the timehands where > not updated is too much. This might be the real cause of all ppc > troubles. I tried to see if the overflow case is possible on amd64, > and did not get a single case of the '> 63' branch executed during the > /usr/tests/lib/libc run. >=20 > Actually, the same overflow-prone code exists in libc, so below is the > updated patch: > - I added __predict_false() > - libc multiplication is also done separately for high-order bits. > (fftclock counterpart is still pending). >=20 > diff --git a/lib/libc/sys/__vdso_gettimeofday.c = b/lib/libc/sys/__vdso_gettimeofday.c > index 3749e0473af..a14576988ff 100644 > --- a/lib/libc/sys/__vdso_gettimeofday.c > +++ b/lib/libc/sys/__vdso_gettimeofday.c > @@ -32,6 +32,8 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > +#include > #include > #include > #include "libc_private.h" > @@ -62,6 +64,7 @@ binuptime(struct bintime *bt, struct vdso_timekeep = *tk, int abs) > { > struct vdso_timehands *th; > uint32_t curr, gen; > + uint64_t scale, x; > u_int delta; > int error; >=20 > @@ -78,7 +81,14 @@ binuptime(struct bintime *bt, struct vdso_timekeep = *tk, int abs) > continue; > if (error !=3D 0) > return (error); > - bintime_addx(bt, th->th_scale * delta); > + scale =3D th->th_scale; > + if (__predict_false(fls(scale) + fls(delta) > 63)) { > + x =3D (scale >> 32) * delta; > + scale &=3D UINT_MAX; > + bt->sec +=3D x >> 32; > + bintime_addx(bt, x << 32); > + } > + bintime_addx(bt, scale * delta); > if (abs) > bintime_add(bt, &th->th_boottime); >=20 > diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c > index 2656fb4d22f..be75781e000 100644 > --- a/sys/kern/kern_tc.c > +++ b/sys/kern/kern_tc.c > @@ -355,13 +355,22 @@ void > binuptime(struct bintime *bt) > { > struct timehands *th; > - u_int gen; > + uint64_t scale, x; > + u_int delta, gen; >=20 > do { > th =3D timehands; > gen =3D atomic_load_acq_int(&th->th_generation); > *bt =3D th->th_offset; > - bintime_addx(bt, th->th_scale * tc_delta(th)); > + scale =3D th->th_scale; > + delta =3D tc_delta(th); > + if (__predict_false(fls(scale) + fls(delta) > 63)) { > + x =3D (scale >> 32) * delta; > + scale &=3D UINT_MAX; > + bt->sec +=3D x >> 32; > + bintime_addx(bt, x << 32); > + } > + bintime_addx(bt, scale * delta); > atomic_thread_fence_acq(); > } while (gen =3D=3D 0 || gen !=3D th->th_generation); > } > @@ -388,13 +397,22 @@ void > bintime(struct bintime *bt) > { > struct timehands *th; > - u_int gen; > + uint64_t scale, x; > + u_int delta, gen; >=20 > do { > th =3D timehands; > gen =3D atomic_load_acq_int(&th->th_generation); > *bt =3D th->th_bintime; > - bintime_addx(bt, th->th_scale * tc_delta(th)); > + scale =3D th->th_scale; > + delta =3D tc_delta(th); > + if (__predict_false(fls(scale) + fls(delta) > 63)) { > + x =3D (scale >> 32) * delta; > + scale &=3D UINT_MAX; > + bt->sec +=3D x >> 32; > + bintime_addx(bt, x << 32); > + } > + bintime_addx(bt, scale * delta); > atomic_thread_fence_acq(); > } while (gen =3D=3D 0 || gen !=3D th->th_generation); > } Thanks. I'll note that powerpc64 seems to use: int fls(int mask) { int bit; if (mask =3D=3D 0) return (0); for (bit =3D 1; mask !=3D 1; bit++) mask =3D (unsigned int)mask >> 1; return (bit); } from /usr/src/sys/libkern/fls.c (unless I missed finding an alternate machine-dependent substitution somewhere). So lots of looping. I've no clue if this contributes to why, with your prior patch, the powerpcl64 spent most of its time being non-responsive and I was unable to do anything effective but escape into and use ddb. (I had to power off to shutdown. I've had to boot from other media to repair things after such.) I expect I need to figure out how to investigate the operation of substructure that contributes to binuptime in order to figure out what contributes to the large deltas. One point from my general understanding is that keeping the tbr's reasonably tracking across the 2 sockets (2 cores per socket) in my context is problematical/racy. (Not that I know the details at this point.) FYI: # sysctl kern.timecounter kern.timecounter.tc.timebase.quality: 0 kern.timecounter.tc.timebase.frequency: 33333333 kern.timecounter.tc.timebase.counter: 1831468476 kern.timecounter.tc.timebase.mask: 4294967295 kern.timecounter.stepwarnings: 0 kern.timecounter.alloweddeviation: 5 kern.timecounter.hardware: timebase kern.timecounter.choice: timebase(0) dummy(-1000000) kern.timecounter.tick: 1 kern.timecounter.fast_gettime: 1 At around 33.3 MHz 2^32 counts would be about 128 seconds to wrap. I do know that the tbr is 64 bit on powerpc64 --and is truncated to 32 bits for its use: static unsigned decr_get_timecount(struct timecounter *tc) { return (mftb()); } in /usr/src/sys/powerpc/powerpc/clock.c . I'll keep trying to get evidence. =3D=3D=3D Mark Millard marklmi at yahoo.com ( dsl-only.net went away in early 2018-Mar)