Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Oct 2017 14:03:24 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Sebastian Huber <sebastian.huber@embedded-brains.de>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: [PATCH] Simplify th_bintime update in tc_windup()
Message-ID:  <20171011110324.GY95911@kib.kiev.ua>
In-Reply-To: <94f238ec-4cac-7c27-87ac-bc84d13d2eda@embedded-brains.de>
References:  <20171011064816.20809-1-sebastian.huber@embedded-brains.de> <20171011093101.GX95911@kib.kiev.ua> <94f238ec-4cac-7c27-87ac-bc84d13d2eda@embedded-brains.de>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 11, 2017 at 11:49:01AM +0200, Sebastian Huber wrote:
> On 11/10/17 11:31, Konstantin Belousov wrote:
> 
> > On Wed, Oct 11, 2017 at 08:48:16AM +0200, Sebastian Huber wrote:
> >> The th_bintime, th_microtime and th_nanotime members of the timehand all
> >> cache the last system time (uptime + boottime).  Only the format
> >> differs.  Do not re-calculate the bintime and simply use the value used
> >> to calculate the microtime and nanotime.
> >>
> >> Group all the updates under the relevant comment.
> > th->th_bintime is recalculated after possible adjustments made by
> > the ntp loop to the th_boottime.
> 
> The loop is:
> 
>  ššš bt = th->th_offset;
>  ššš bintime_add(&bt, &th->th_boottime);
> 
> <--- here the bt is our new system time
> 
>  ššš i = bt.sec - tho->th_microtime.tv_sec;
>  ššš if (i > LARGE_STEP)
>  ššš ššš i = 2;
>  ššš for (; i > 0; i--) {
>  ššš ššš t = bt.sec;
>  ššš ššš ntp_update_second(&th->th_adjustment, &bt.sec);
>  ššš ššš if (bt.sec != t)
Can you fix your mail client ?  The mail body is filled with the \x9a
symbols which makes the text unreadable and requiring decyphering.

> 
> <-- here the bt.sec changed
> 
>  ššš ššš ššš th->th_boottime.sec += bt.sec - t;
> 
> <-- here we update the boottime seconds, so now bt == uptime + boottime
>  ššš }
> 
> So, I think the patch is correct.
Thank you for the explanation.  I committed this as r324528.

> 
> >
> > But your patch makes me consider the two lines after the XXX comment.
> > Shouldn't we use the updated th_bintime instead of the pre-adjustment
> > bt value ?
> >
> 
> All values should reflect the same time.
s/should// due to synchronity of the updates to bt and th_bootime, as
you noted above.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171011110324.GY95911>