lkml.org 
[lkml]   [2015]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] time, ntp: Do not update time_state in middle of leap second [v3]
On Tue, Feb 17, 2015 at 03:16:18PM -0800, John Stultz wrote:
> Ok, thanks for the more verbose explanation. Although this is more a
> history of what you've seen rather then the crux of the change.
>
> To distill this down just a bit, the point is the usual mode for NTP
> time_state machine looks like:
>
> TIME_OK -> TIME_INS -> TIME_OOP
> | |
> v v
> TIME_DEL ------------> TIME_WAIT -(back)-> TIME_OK
>
> (hopefully the ascii art survives here)
>
> Now, from any of these states, currently if adjtimex is called w/ the
> STA_PLL bit cleared (after STA_PLL was set), we reset back to TIME_OK,
> effectively cancelling any transitions. (You'll have to imagine a line
> from any of the states back to TIME_OK, since that's going to be too
> ugly to do in ascii)
>
> Your patch is trying to remove the line back from TIME_OOP back to
> TIME_OK. Basically stopping the ability to reset the ntp state during
> a leapsecond.
>
> I do get that the behavior seen was strange due to a bug in the test
> code which caused unexpected cancellation of state, but I'm not sure
> if we should change the behavior to enforce that cancellation not be
> possible. I could imagine some logic which really wants to reset the
> state, which just by chance lands during a leap second, and the
> application is confused since the state change didn't occur as
> expected.
>
> So I guess I'm not seeing that the state machine is actually "broken"
> in this case that you've outlined. If you can articulate better why
> the OOP -> OK transition is truly invalid, I'd be interested in
> hearing, but I'm not sure I want to risk a behavioral change unless
> there's wide agreement.

I think the only real problem occurs when the adjtimex is called in the
the TIME_OOP state with STA_PLL cleared _and_ STA_INS set.
In this case the state machine is reset to TIME_OK but goes back
to TIME_INS on the next second_overflow, potentially causing
another false leap second to be inserted on the following
midnight.

The state machine is meant to only go back to TIME_INS once STA_INS is
cleared and then set again - this is what the TIME_WAIT state is
for.

In fact, I don't see a reason why the STA_PLL -> !STA_PLL transition should
ever set the time_state to TIME_OK.
- When the STA_INS/STA_DEL flag is removed from the status, the state
machine will end up in TIME_OK from any state.
- When STA_INS/STA_DEL is set in
the status, the state mchine will transition from TIME_OK to
TIME_INS/TIME_DEL anyway.

I think the "time_status = TIME_OK" should be just dropped.

It has been added by eea83d896e318bda54be2d2770d2c5d6668d11db
(ntp: NTP4 user space bits update) and it's not clear why.
Roman?


--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ



\
 
 \ /
  Last update: 2015-02-18 18:41    [W:0.084 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site