Date: Thu, 28 Feb 2019 23:39:06 -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 [patched failed] Message-ID: <28C2BB0A-3DAA-4D18-A317-49A8DD52778F@yahoo.com> In-Reply-To: <962D78C3-65BE-40C1-BB50-A0088223C17B@yahoo.com> References: <D3D7E9F4-9A5E-4320-B3C8-EC5CEF4A2764@yahoo.com> <20190228145542.GT2420@kib.kiev.ua> <20190228150811.GU2420@kib.kiev.ua> <962D78C3-65BE-40C1-BB50-A0088223C17B@yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[The new, trial code also has truncation occurring.]
On 2019-Feb-28, at 17:55, Mark Millard <marklmi at yahoo.com> wrote:
> [The PowerMac becomes non-responsive for significant periods of time.]
>=20
> On 2019-Feb-28, at 07:08, Konstantin Belousov <kib at freebsd.org> =
wrote:
>=20
>> 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;
>> + 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 (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);
>> }
>=20
> The PowerPC G5 ends up not responsive for long periods and
> responsive for only very short periods, such as being able
> to type in a few letters and have them show up at the time.
>=20
> I've only barely started investigating what is going on
> and I'll be rechecking my instrumented variant for stupid
> mistakes and such. I"ll try your un-instrumented binuptime
> as well.
>=20
> As stands I'll be updating the kernel via booting a 2nd
> disk that is not being experimented with.
>=20
> My information gathering may not be very timely.
>=20
Live experimenting and inspection has proved problematical.
Below I experiment with the prior scale_factor and tim_diff
figures from my oroginal code that recorded such, but
showing some of what your new code does for them. (In part
the below is text from the original list submittal to have
a context.)
Observed consistently for tc->tc_frequency:
tc->tc_frequency =3D=3D 0x1fca055 (i.e., 33333333)
( tc->tc_counter_mask is 0xfffffffful as well. )
An example observation of diff_scaled having an overflowed
value was:
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)
But for the new, trail code:
0x80da2067ac is 40 bits
0x53db8d9 is 27 bits
So 67 bits, more than 63. Then:
x
=3D=3D (0x80da2067ac>>32) * 0x53db8d9
=3D=3D 0x80 * 0x53db8d9
=3D=3D 0x29EDC6C80
x>>32
=3D=3D 0x2
x<<32
=3D=3D 0x9EDC6C8000000000 (limited to 64 bits)
Note the truncation of: 0x29EDC6C8000000000.
Thus the "bintime_addx(bt, x << 32)" is still
based on a truncated value.
I'll not bother with the other two examples unless you
want such.
=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?28C2BB0A-3DAA-4D18-A317-49A8DD52778F>