Date: Thu, 28 Feb 2019 13:50:49 -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: <C42A56E5-63C8-41DC-B298-B43BFFB66064@yahoo.com> In-Reply-To: <0A345E1F-7675-4B4B-8A74-ACD59E90E72F@yahoo.com> References: <D3D7E9F4-9A5E-4320-B3C8-EC5CEF4A2764@yahoo.com> <20190228145542.GT2420@kib.kiev.ua> <20190228150811.GU2420@kib.kiev.ua> <0A345E1F-7675-4B4B-8A74-ACD59E90E72F@yahoo.com>
index | next in thread | previous in thread | raw e-mail
[I left implicit that I was summarizing for x!=0.]
On 2019-Feb-28, at 13:46, Mark Millard <marklmi at yahoo.com> wrote:
> 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:
I forgot to indicate the context: when x!-0
> 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)
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?C42A56E5-63C8-41DC-B298-B43BFFB66064>