Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757587AbbBENUR (ORCPT ); Thu, 5 Feb 2015 08:20:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41250 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752989AbbBENUQ (ORCPT ); Thu, 5 Feb 2015 08:20:16 -0500 Message-ID: <54D36E08.1060400@redhat.com> Date: Thu, 05 Feb 2015 08:20:08 -0500 From: Prarit Bhargava User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131028 Thunderbird/17.0.10 MIME-Version: 1.0 To: Miroslav Lichvar CC: John Stultz , Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH] time, ntp: Do not update time_state in middle of leap References: <20150204163005.GF26460@localhost> In-Reply-To: <20150204163005.GF26460@localhost> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3310 Lines: 85 On 02/04/2015 11:30 AM, Miroslav Lichvar wrote: > Prarit Bhargava wrote: >> While this is highly unlikely to ever happen in the real world it is >> still something we should protect against, as breaking the state machine >> is obviously bad. > > I'm not sure what exactly breaks here. If the PLL is disabled before > time_state is set to TIME_OOP, the insertion/deletion will be aborted. Yes, that is correct. > If after that, adjtimex() will return with TIME_ERROR as expected, or > not? It is possible that an adjtimex() will set the time_state here back to TIME_OK and return TIME_OK to userspace. Again, and I want to stress this, this is extremely unlikely to happen. I only hit this due to a bug in a test program. But at the end of the day, it is possible that this happens and we should protect against it. [ 942.952833] time_state [1] change from TIME_OK to TIME_INS Fri Feb 13 18:59:51 2015 + 318126 us TIME_INS Fri Feb 13 18:59:51 2015 + 818167 us TIME_INS Fri Feb 13 18:59:52 2015 + 318208 us TIME_INS Fri Feb 13 18:59:52 2015 + 818248 us TIME_INS Fri Feb 13 18:59:53 2015 + 318290 us TIME_INS Fri Feb 13 18:59:53 2015 + 818331 us TIME_INS Fri Feb 13 18:59:54 2015 + 318372 us TIME_INS Fri Feb 13 18:59:54 2015 + 818413 us TIME_INS Fri Feb 13 18:59:55 2015 + 318454 us TIME_INS Fri Feb 13 18:59:55 2015 + 818495 us TIME_INS Fri Feb 13 18:59:56 2015 + 318534 us TIME_INS Fri Feb 13 18:59:56 2015 + 818575 us TIME_INS Fri Feb 13 18:59:57 2015 + 318617 us TIME_INS Fri Feb 13 18:59:57 2015 + 818660 us TIME_INS Fri Feb 13 18:59:58 2015 + 318702 us TIME_INS Fri Feb 13 18:59:58 2015 + 818744 us TIME_INS Fri Feb 13 18:59:59 2015 + 318785 us TIME_INS Fri Feb 13 18:59:59 2015 + 818837 us TIME_INS [ 952.953143] time_state [4] change from TIME_INS to TIME_OOP [ 952.953150] Clock: inserting leap second 23:59:60 UTC [ 953.299905] process_adj_status: insert_leap_sec[1223] setting time_state back to TIME_OK [1, 1] <<< adjtimex() call [ 953.299913] time_state [9] change from TIME_OOP to TIME_OK Fri Feb 13 18:59:59 2015 + 318878 us TIME_OK Fri Feb 13 18:59:59 2015 + 818931 us TIME_OK [ 954.064237] time_state [1] change from TIME_OK to TIME_INS Fri Feb 13 19:00:00 2015 + 318972 us TIME_INS Fri Feb 13 19:00:00 2015 + 819012 us TIME_INS Fri Feb 13 19:00:01 2015 + 319051 us TIME_INS Fri Feb 13 19:00:01 2015 + 819089 us TIME_INS Fri Feb 13 19:00:02 2015 + 319128 us TIME_INS P. > >> static inline void process_adj_status(struct timex *txc, struct timespec64 *ts) >> { >> - if ((time_status & STA_PLL) && !(txc->status & STA_PLL)) { >> + if ((time_status & STA_PLL) && !(txc->status & STA_PLL) && >> + (time_state != TIME_OOP)) { >> time_state = TIME_OK; >> time_status = STA_UNSYNC; >> /* restart PPS frequency calibration */ > > Shouldn't be time_status reset and the PPS calibration restarted even > when state is TIME_OOP? No, this should only happen after the leap second is done IMO (which should be no more than 2 seconds later). > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/