Skip site navigation (1)Skip section navigation (2)
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:
>>> . . .
>>=20
>> . . .
>=20
> Of course I botched the formula, please try this instead:
>=20
> 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;
>=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 (fls(scale) + fls(delta) > 63) {
> +			x =3D (scale >> 32) * delta;
> +			scale &=3D UINT_MAX;

The following two lines confuse me overall:

> +			bt->sec +=3D 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 =3D _bt->frac;
        _bt->frac +=3D _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 +=3D (x >> 32) + 1;

Is that the intent?


> +		}
> +		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 (fls(scale) + fls(delta) > 63) {
> +			x =3D (scale >> 32) * delta;
> +			scale &=3D UINT_MAX;

The same for the below two lines:

> +			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);
> }
>=20
=3D=3D=3D
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>