Date: Thu, 28 Feb 2019 13:46:11 -0800 From: Mark Millard <marklmi@yahoo.com> To: Konstantin Belousov <kib@freebsd.org> Cc: freebsd-hackers Hackers <freebsd-hackers@freebsd.org>, FreeBSD PowerPC ML <freebsd-ppc@freebsd.org> Subject: Re: powerpc64 head -r344018 stuck sleeping problems: th->th_scale * tc_delta(th) overflows unsigned 64 bits sometimes Message-ID: <0A345E1F-7675-4B4B-8A74-ACD59E90E72F@yahoo.com> In-Reply-To: <20190228150811.GU2420@kib.kiev.ua> References: <D3D7E9F4-9A5E-4320-B3C8-EC5CEF4A2764@yahoo.com> <20190228145542.GT2420@kib.kiev.ua> <20190228150811.GU2420@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2019-Feb-28, at 07:08, Konstantin Belousov <kib at freebsd.org> wrote:
> On Thu, Feb 28, 2019 at 04:55:42PM +0200, Konstantin Belousov wrote:
>> On Thu, Feb 28, 2019 at 05:06:23AM -0800, Mark Millard via freebsd-ppc wrote:
>>> . . .
>>
>> . . .
>
> Of course I botched the formula, please try this instead:
>
> diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c
> index 2656fb4d22f..fdd4f4f6a52 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;
>
> do {
> th = timehands;
> gen = atomic_load_acq_int(&th->th_generation);
> *bt = th->th_offset;
> - bintime_addx(bt, th->th_scale * tc_delta(th));
> + scale = th->th_scale;
> + delta = tc_delta(th);
> + if (fls(scale) + fls(delta) > 63) {
> + x = (scale >> 32) * delta;
> + scale &= UINT_MAX;
The following two lines confuse me overall:
> + bt->sec += x >> 32;
> + bintime_addx(bt, x << 32);
bintime_addx does:
static __inline void
bintime_addx(struct bintime *_bt, uint64_t _x)
{
uint64_t _u;
_u = _bt->frac;
_bt->frac += _x;
if (_u > _bt->frac)
_bt->sec++;
}
So I'd expect:
bintime_addx(bt, x << 32)
to find _u > _bt->frac and to also do _bt->sec++ .
So overall (as a means of summarizing for
bt->sec):
bt->sec += (x >> 32) + 1;
Is that the intent?
> + }
> + bintime_addx(bt, scale * delta);
> atomic_thread_fence_acq();
> } while (gen == 0 || gen != 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;
>
> do {
> th = timehands;
> gen = atomic_load_acq_int(&th->th_generation);
> *bt = th->th_bintime;
> - bintime_addx(bt, th->th_scale * tc_delta(th));
> + scale = th->th_scale;
> + delta = tc_delta(th);
> + if (fls(scale) + fls(delta) > 63) {
> + x = (scale >> 32) * delta;
> + scale &= UINT_MAX;
The same for the below two lines:
> + bt->sec += x >> 32;
> + bintime_addx(bt, x << 32);
> + }
> + bintime_addx(bt, scale * delta);
> atomic_thread_fence_acq();
> } while (gen == 0 || gen != th->th_generation);
> }
>
===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0A345E1F-7675-4B4B-8A74-ACD59E90E72F>