I have been tinkering with clock subsystem for more or less tickless based approach. I am not sure whether calculation in "lib/libc/sys/__vdso_gettimeofday.c" for "binuptime" function is correct. Currently the code looks like this: scale = th->th_scale; #ifdef _LP64 scale_bits = ffsl(scale); #else scale_bits = ffsll(scale); #endif if (__predict_false(scale_bits + fls(delta) > 63)) { x = (scale >> 32) * delta; scale &= 0xffffffff; bt->sec += x >> 32; bintime_addx(bt, x << 32); } Example outputs from two time points (time is measured by ARM Generic Timer, but that's just 64bit counter masked to 32bits): th->th_boottime = 1640852968.ff886104742783f9 timecounter delta = 29015463 th->th_scale = 295147905178 th->th_offset_count = 539967626 bintime_addx(bt, scale * delta = 0x76D8EB0A9A877676) => 9.4522dbb32c111955 th->th_boottime = 1640852968.ff886104742783f9 timecounter delta = 64100295 th->th_scale = 295147905178 th->th_offset_count = 539967626 bintime_addx(bt, scale * delta) => 8.d4d7f89392515095 Multiplication of scale and delta will overflow 64 bits. I am not sure whether the scale_bits should rather be calculated by flsl/flsll to detect the overflow.
I do not understand what are you saying. If scale * delta overflows, which we detect by calculating ffsl(scale) + fls(delta), we do more complicated calculation that moves overflow to bt->sec, and clamp scale to 32bit.
(In reply to Konstantin Belousov from comment #1) scale = 295147905178 ffsl(295147905178) = 2 scale_bits = 2 delta = 64100295 fls(delta) = 26 2 + 26 < 63 => but overflow occurs.
I see, do you mean the following: diff --git a/lib/libc/sys/__vdso_gettimeofday.c b/lib/libc/sys/__vdso_gettimeofday.c index c1457a54d37e..cf1400cdf291 100644 --- a/lib/libc/sys/__vdso_gettimeofday.c +++ b/lib/libc/sys/__vdso_gettimeofday.c @@ -83,9 +83,9 @@ binuptime(struct bintime *bt, struct vdso_timekeep *tk, bool abs) return (error); scale = th->th_scale; #ifdef _LP64 - scale_bits = ffsl(scale); + scale_bits = flsl(scale); #else - scale_bits = ffsll(scale); + scale_bits = flsll(scale); #endif if (__predict_false(scale_bits + fls(delta) > 63)) { x = (scale >> 32) * delta;
(In reply to Konstantin Belousov from comment #3) Yes, it seems to do the trick :) I tested such fix locally for 8 hours and it looks like it works fine.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a1f9326607dea5ab6979935e3ca2d7402dcc7cc1 commit a1f9326607dea5ab6979935e3ca2d7402dcc7cc1 Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2022-02-08 19:13:40 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2022-02-08 19:44:23 +0000 libc binuptime(): use the right function to get the most significant bit index Reported and tested by: Jaroslaw Pelczar <jarek@jpelczar.com> PR: 261781 Sponsored by: The FreeBSD Foundation MFC after: 1 week lib/libc/sys/__vdso_gettimeofday.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d0199f27c06caac4ccfedc61e8b2d13b658ea589 commit d0199f27c06caac4ccfedc61e8b2d13b658ea589 Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2022-02-08 19:13:40 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2022-02-15 00:36:51 +0000 libc binuptime(): use the right function to get the most significant bit index PR: 261781 (cherry picked from commit a1f9326607dea5ab6979935e3ca2d7402dcc7cc1) lib/libc/sys/__vdso_gettimeofday.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)