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

[Mark Millard]

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