Re: powerpc64 head -r344018 stuck sleeping problems: th->th_scale * tc_delta(th) overflows unsigned 64 bits sometimes

[I was distracted and made a stupid mistake. Sorry for
the noise.]

On 2019-Feb-28, at 13:50, Mark Millard <[email protected]> wrote:

> [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):

How I got _u > _bt->frac for that call in general
when the call's (x<<32) != 0 I do not know.

So ignore my stupid, mistaken questions.

>> 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)

_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-ppc
To unsubscribe, send any mail to "[email protected]"