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

[Mark Millard]

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:

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)