Linux kernel development

Richard Cochran | 18 May 2012 16:09

[PATCH RFC V2 0/6] Fix leap seconds and add tai clock

In honor of this summer's new leap second, this patch series aim to
fix the incorrect time values reported during a leap second. This
second version of the patch series does not change the core time
keeping code, but rather affects only the adjtimex interface.

Of course, the POSIX UTC system is broken by design, and the Linux
kernel cannot fix that. However, what we can do is correctly execute
leap seconds and report the time variables (UTC time, TAI offset, and
leap second status) with consistency.

Since commit 6b43ae8a, the leap second is applied on the first timer
tick after the actual deadline, resulting in incorrect time values
from gettimeofday and friends.  In contrast to V1, this series does
not address that issue but rather adds extra checks into the adjtimex
code path, with the effect of always providing correct UTC time
values, even during a leap second.

The first two patches merely clean up some cruft I noticed along the
way while working on this series.

John Stultz (1):
  time: Add CLOCK_TAI clockid

Richard Cochran (5):
  time: remove obsolete declaration
  ntp: remove useless parameter
  time: keep track of the pending utc/tai threshold
  time: introduce leap second functional interface
  time: move leap second management into time keeping core

(Continue reading)

headers

Richard Cochran | 18 May 2012 16:09

[PATCH RFC V2 4/6] time: introduce leap second functional interface

This patch adds a new private leap second interface for use by the NTP
code. In addition to methods for starting and ending leap seconds, the
interface provides a way to get the correct UTC time of day even during
a leap second.

Signed-off-by: Richard Cochran <richardcochran <at> gmail.com>
---
 kernel/time/leap-seconds.h |   21 +++++++
 kernel/time/timekeeping.c  |  129 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 0 deletions(-)
 create mode 100644 kernel/time/leap-seconds.h

diff --git a/kernel/time/leap-seconds.h b/kernel/time/leap-seconds.h
new file mode 100644
index 0000000..3dea7b0
--- /dev/null
+++ b/kernel/time/leap-seconds.h
 <at>  <at>  -0,0 +1,21  <at>  <at> 
+/*
+ * linux/kernel/time/leap-seconds.h
+ *
+ * Functional interface to the timekeeper code,
+ * for use by the NTP code.
+ *
+ */
+#ifndef __LINUX_KERNEL_TIME_LEAP_SECONDS_H
+#define __LINUX_KERNEL_TIME_LEAP_SECONDS_H
+
+#include <linux/time.h>
+

(Continue reading)

headers

John Stultz | 21 May 2012 20:01

Re: [PATCH RFC V2 4/6] time: introduce leap second functional interface

On 05/18/2012 07:09 AM, Richard Cochran wrote:
> This patch adds a new private leap second interface for use by the NTP
> code. In addition to methods for starting and ending leap seconds, the
> interface provides a way to get the correct UTC time of day even during
> a leap second.
>
> Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>
> ---
>   kernel/time/leap-seconds.h |   21 +++++++
>   kernel/time/timekeeping.c  |  129 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 150 insertions(+), 0 deletions(-)
>   create mode 100644 kernel/time/leap-seconds.h
>
> diff --git a/kernel/time/leap-seconds.h b/kernel/time/leap-seconds.h
> new file mode 100644
> index 0000000..3dea7b0
> --- /dev/null
> +++ b/kernel/time/leap-seconds.h
>  <at>  <at>  -0,0 +1,21  <at>  <at> 
> +/*
> + * linux/kernel/time/leap-seconds.h
> + *
> + * Functional interface to the timekeeper code,
> + * for use by the NTP code.
> + *
> + */
> +#ifndef __LINUX_KERNEL_TIME_LEAP_SECONDS_H
> +#define __LINUX_KERNEL_TIME_LEAP_SECONDS_H
> +
> +#include<linux/time.h>

(Continue reading)

headers

Richard Cochran | 21 May 2012 21:18

Re: [PATCH RFC V2 4/6] time: introduce leap second functional interface

On Mon, May 21, 2012 at 11:01:03AM -0700, John Stultz wrote:
> On 05/18/2012 07:09 AM, Richard Cochran wrote:
> >+
> >+int timekeeping_gettod_status(struct timespec *ts, time_t *offset);
> >+
> >+void timekeeping_delete_leap_second(void);
> >+
> >+void timekeeping_finish_leap_second(void);
> >+
> >+void timekeeping_insert_leap_second(void);
> >+
> >+#endif
> 
> Why not just add these to time.h?

This is a private interface only for ntp.c, not for the whole rest of
the kernel via time.h.

BTW this highlights the very icky incestuous relationship between
ntp.c and timekeeper.c. Probably there should be a comment documenting
the (unspoken) locking sequence for ntp_lock and timekeeper.lock.

Thanks,
Richard

headers

John Stultz | 21 May 2012 22:24

Re: [PATCH RFC V2 4/6] time: introduce leap second functional interface

On 05/21/2012 12:18 PM, Richard Cochran wrote:
> On Mon, May 21, 2012 at 11:01:03AM -0700, John Stultz wrote:
>> On 05/18/2012 07:09 AM, Richard Cochran wrote:
>>> +
>>> +int timekeeping_gettod_status(struct timespec *ts, time_t *offset);
>>> +
>>> +void timekeeping_delete_leap_second(void);
>>> +
>>> +void timekeeping_finish_leap_second(void);
>>> +
>>> +void timekeeping_insert_leap_second(void);
>>> +
>>> +#endif
>> Why not just add these to time.h?
> This is a private interface only for ntp.c, not for the whole rest of
> the kernel via time.h.
Hrm.  I prefer to keep things fairly flat (even having time.h and 
timex.h bugs me somewhat).  But having such a separation could be 
useful, but maybe at a slightly more coarse level. Something like 
timekeeping-internal.h and time.h, splitting all the general accessors 
away from the non-general.

I just don't want to have a ton of stray .h files, but maybe I'm 
prematurely worrying about it.

> BTW this highlights the very icky incestuous relationship between
> ntp.c and timekeeper.c. Probably there should be a comment documenting
> the (unspoken) locking sequence for ntp_lock and timekeeper.lock.
>
The locking order is pretty straight forward: timekeeper.lock ->

(Continue reading)

headers

Richard Cochran | 22 May 2012 06:25

Re: [PATCH RFC V2 4/6] time: introduce leap second functional interface

On Mon, May 21, 2012 at 01:24:57PM -0700, John Stultz wrote:
> On 05/21/2012 12:18 PM, Richard Cochran wrote:
> Hrm.  I prefer to keep things fairly flat (even having time.h and
> timex.h bugs me somewhat).  But having such a separation could be
> useful, but maybe at a slightly more coarse level. Something like
> timekeeping-internal.h and time.h, splitting all the general
> accessors away from the non-general.

Yes, time.h is full of stuff not really for public use. When compiling
on an atom netbook as I do, it gets really noticeable and annoying
when you tweak some private prototype, and then the whole darn kernel
rebuilds.

> The locking order is pretty straight forward: timekeeper.lock ->
> ntp_lock.   This only gets messy when you require timekeeping data
> from the ntp context, but usually we provide the required data via
> the caller.  But better documentation is always welcome.

The icky part is the fact that ntp would need access to timekeeper
state while holding ntp_lock.

Richard

headers

John Stultz | 22 May 2012 17:10

Re: [PATCH RFC V2 4/6] time: introduce leap second functional interface

On 05/21/2012 09:25 PM, Richard Cochran wrote:
> On Mon, May 21, 2012 at 01:24:57PM -0700, John Stultz wrote:
>> The locking order is pretty straight forward: timekeeper.lock ->
>> ntp_lock.   This only gets messy when you require timekeeping data
>> from the ntp context, but usually we provide the required data via
>> the caller.  But better documentation is always welcome.
> The icky part is the fact that ntp would need access to timekeeper
> state while holding ntp_lock.
Well, that needs to be reworked so it doesn't.  :)   Again, passing the 
required time state to NTP functions from the timekeeping context should 
handle these issues, and for those few NTP paths that aren't triggered 
from the timekeeping core (do_adjtimex basically), we can grab the 
required time state before taking the ntp lock as we have been doing.

thanks
-john

headers

Richard Cochran | 18 May 2012 16:09

[PATCH RFC V2 6/6] time: Add CLOCK_TAI clockid

From: John Stultz <john.stultz <at> linaro.org>

This adds a CLOCK_TAI clockid and the needed accessors.

Signed-off-by: John Stultz <john.stultz <at> linaro.org>
Signed-off-by: Richard Cochran <richardcochran <at> gmail.com>
---
 include/linux/time.h      |    7 +++----
 kernel/posix-timers.c     |   10 ++++++++++
 kernel/time/timekeeping.c |   31 +++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index b6034b0..9be8205 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
 <at>  <at>  -169,6 +169,7  <at>  <at>  extern int timekeeping_valid_for_hres(void);
 extern u64 timekeeping_max_deferment(void);
 extern int timekeeping_inject_offset(struct timespec *ts);
 extern void timekeeping_set_tai_offset(time_t tai_offset);
+extern void timekeeping_clocktai(struct timespec *ts);

 struct tms;
 extern void do_sys_times(struct tms *);
 <at>  <at>  -297,11 +298,9  <at>  <at>  struct itimerval {
 #define CLOCK_BOOTTIME			7
 #define CLOCK_REALTIME_ALARM		8
 #define CLOCK_BOOTTIME_ALARM		9
+#define CLOCK_SGI_CYCLE			10	/* Hardware specific */
+#define CLOCK_TAI			11

(Continue reading)

headers

Richard Cochran | 18 May 2012 16:09

[PATCH RFC V2 5/6] time: move leap second management into time keeping core

This patch refactors the timekeeping and ntp code in order to
improve leap second handling and to provide a TAI based clock.
The change has a number of aspects.

* remove the NTP time_status variable

Instead, compute this value on demand in adjtimex.

* move TAI managment into timekeeping core from ntp

Currently NTP manages the TAI offset. Since there's plans for a
CLOCK_TAI clockid, push the TAI management into the timekeeping
core.

[ Original idea from: John Stultz <john.stultz <at> linaro.org> ]
[ Changed by RC: ]

  - replace u32 with time_t
  - fixup second call site of second_overflow()

* replace modulus with integer test and schedule leap second

On the day of a leap second, the NTP code performs a division on every
tick in order to know when to leap. This patch replaces the division
with an integer comparison, making the code faster and easier to
understand.

Signed-off-by: Richard Cochran <richardcochran <at> gmail.com>
---
 include/linux/time.h      |    1 +

(Continue reading)

headers

John Stultz | 21 May 2012 20:18

Re: [PATCH RFC V2 5/6] time: move leap second management into time keeping core

On 05/18/2012 07:09 AM, Richard Cochran wrote:
> This patch refactors the timekeeping and ntp code in order to
> improve leap second handling and to provide a TAI based clock.
> The change has a number of aspects.
>
> * remove the NTP time_status variable
>
> Instead, compute this value on demand in adjtimex.
>
> * move TAI managment into timekeeping core from ntp
>
> Currently NTP manages the TAI offset. Since there's plans for a
> CLOCK_TAI clockid, push the TAI management into the timekeeping
> core.
>
> [ Original idea from: John Stultz<john.stultz <at> linaro.org>  ]
> [ Changed by RC: ]
>
>    - replace u32 with time_t
>    - fixup second call site of second_overflow()
>
> * replace modulus with integer test and schedule leap second
>
> On the day of a leap second, the NTP code performs a division on every
> tick in order to know when to leap. This patch replaces the division
> with an integer comparison, making the code faster and easier to
> understand.
>
> Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>
> ---

(Continue reading)

headers

Richard Cochran | 21 May 2012 21:24

Re: [PATCH RFC V2 5/6] time: move leap second management into time keeping core

On Mon, May 21, 2012 at 11:18:12AM -0700, John Stultz wrote:
> On 05/18/2012 07:09 AM, Richard Cochran wrote:
> >This patch refactors the timekeeping and ntp code in order to
> >improve leap second handling and to provide a TAI based clock.
> >The change has a number of aspects.
> >
> >* remove the NTP time_status variable
> >
> >Instead, compute this value on demand in adjtimex.
> >
> >* move TAI managment into timekeeping core from ntp
> >
> >Currently NTP manages the TAI offset. Since there's plans for a
> >CLOCK_TAI clockid, push the TAI management into the timekeeping
> >core.
> >
> >[ Original idea from: John Stultz<john.stultz <at> linaro.org>  ]
> >[ Changed by RC: ]
> >
> >   - replace u32 with time_t
> >   - fixup second call site of second_overflow()
> >
> >* replace modulus with integer test and schedule leap second
> >
> >On the day of a leap second, the NTP code performs a division on every
> >tick in order to know when to leap. This patch replaces the division
> >with an integer comparison, making the code faster and easier to
> >understand.
> >
> >Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>

(Continue reading)

headers

Richard Cochran | 18 May 2012 16:09

[PATCH RFC V2 1/6] time: remove obsolete declaration

The function, timekeeping_leap_insert, was removed in commit
6b43ae8a619d17c4935c3320d2ef9e92bdeed05d

Signed-off-by: Richard Cochran <richardcochran <at> gmail.com>
---
 include/linux/time.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index 33a92ea..179f4d6 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
 <at>  <at>  -167,7 +167,6  <at>  <at>  extern void get_monotonic_boottime(struct timespec *ts);
 extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
 extern int timekeeping_valid_for_hres(void);
 extern u64 timekeeping_max_deferment(void);
-extern void timekeeping_leap_insert(int leapsecond);
 extern int timekeeping_inject_offset(struct timespec *ts);

 struct tms;
--

-- 
1.7.2.5

headers

John Stultz | 22 May 2012 01:57

Re: [PATCH RFC V2 1/6] time: remove obsolete declaration

On 05/18/2012 07:09 AM, Richard Cochran wrote:
> The function, timekeeping_leap_insert, was removed in commit
> 6b43ae8a619d17c4935c3320d2ef9e92bdeed05d
>
> Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>
>

I just queued this one and sent it to tglx.

thanks
-john

headers

Richard Cochran | 18 May 2012 16:09

[PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

This patch introduces time keeping variables to track the next
mini-epoch between the UTC and TAI timescales. A leap second occurs
one second before a mini-epoch. When no leap second is pending, then
the epoch is set to the far future, LONG_MAX.

This code will become useful later on for providing correct time
surrounding a leap second.

Signed-off-by: Richard Cochran <richardcochran <at> gmail.com>
---
 kernel/time/timekeeping.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d66b213..ac04de4 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
 <at>  <at>  -70,6 +70,19  <at>  <at>  struct timekeeper {
 	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
 	struct timespec raw_time;

+	/* The current TAI - UTC offset */
+	time_t tai_offset;
+	/* The UTC time of the next leap second epoch */
+	time_t utc_epoch;
+	/* Tracks where we stand with regard to leap the second epoch. */
+	enum {
+		LEAP_IDLE,
+		LEAP_INS_PENDING,
+		LEAP_INS_DONE,

(Continue reading)

headers

John Stultz | 21 May 2012 20:09

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/18/2012 07:09 AM, Richard Cochran wrote:
> This patch introduces time keeping variables to track the next
> mini-epoch between the UTC and TAI timescales. A leap second occurs
> one second before a mini-epoch. When no leap second is pending, then
> the epoch is set to the far future, LONG_MAX.
>
> This code will become useful later on for providing correct time
> surrounding a leap second.
>
> Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>
> ---
>   kernel/time/timekeeping.c |   14 ++++++++++++++
>   1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index d66b213..ac04de4 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
>  <at>  <at>  -70,6 +70,19  <at>  <at>  struct timekeeper {
>   	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
>   	struct timespec raw_time;
>
> +	/* The current TAI - UTC offset */
> +	time_t tai_offset;
> +	/* The UTC time of the next leap second epoch */
> +	time_t utc_epoch;

How about leap_utc_epoch just to be more clear?

> +	/* Tracks where we stand with regard to leap the second epoch. */

(Continue reading)

headers

Richard Cochran | 21 May 2012 21:08

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Mon, May 21, 2012 at 11:09:51AM -0700, John Stultz wrote:
> On 05/18/2012 07:09 AM, Richard Cochran wrote:
> >This patch introduces time keeping variables to track the next
> >mini-epoch between the UTC and TAI timescales. A leap second occurs
> >one second before a mini-epoch. When no leap second is pending, then
> >the epoch is set to the far future, LONG_MAX.
> >
> >This code will become useful later on for providing correct time
> >surrounding a leap second.
> >
> >Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>
> >---
> >  kernel/time/timekeeping.c |   14 ++++++++++++++
> >  1 files changed, 14 insertions(+), 0 deletions(-)
> >
> >diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> >index d66b213..ac04de4 100644
> >--- a/kernel/time/timekeeping.c
> >+++ b/kernel/time/timekeeping.c
> > <at>  <at>  -70,6 +70,19  <at>  <at>  struct timekeeper {
> >  	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
> >  	struct timespec raw_time;
> >
> >+	/* The current TAI - UTC offset */
> >+	time_t tai_offset;
> >+	/* The UTC time of the next leap second epoch */
> >+	time_t utc_epoch;
> 
> How about leap_utc_epoch just to be more clear?

(Continue reading)

headers

Richard Cochran | 22 May 2012 19:39

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Mon, May 21, 2012 at 09:08:15PM +0200, Richard Cochran wrote:
> On Mon, May 21, 2012 at 11:09:51AM -0700, John Stultz wrote:
> > On 05/18/2012 07:09 AM, Richard Cochran wrote:
> > >+	/* Tracks where we stand with regard to leap the second epoch. */
> > >+	enum {
> > >+		LEAP_IDLE,
> > >+		LEAP_INS_PENDING,
> > >+		LEAP_INS_DONE,
> > >+		LEAP_DEL_PENDING,
> > >+		LEAP_DEL_DONE,
> > >+	} leap_state;

...

> I don't think I am explaining this very well. I will try again to make
> it clear using a table or something later on.

The following table illustrates what happens around a (fictitious)
leap second. Imagine a new epoch will occur at UTC time value 10 and
the new TAI - UTC offset will be 2 seconds. The columns of the table
show the values of the relevant time variables.

U:     UTC time
CODE:  NTP time code
T:     TAI - UTC offset
P:     pending (explained below)

   U   CODE    T   P 
 --------------------
   1   INS     1   1  leap second sheduled

(Continue reading)

headers

John Stultz | 22 May 2012 20:06

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/22/2012 10:39 AM, Richard Cochran wrote:
> On Mon, May 21, 2012 at 09:08:15PM +0200, Richard Cochran wrote:
>> On Mon, May 21, 2012 at 11:09:51AM -0700, John Stultz wrote:
>>> On 05/18/2012 07:09 AM, Richard Cochran wrote:
>>>> +	/* Tracks where we stand with regard to leap the second epoch. */
>>>> +	enum {
>>>> +		LEAP_IDLE,
>>>> +		LEAP_INS_PENDING,
>>>> +		LEAP_INS_DONE,
>>>> +		LEAP_DEL_PENDING,
>>>> +		LEAP_DEL_DONE,
>>>> +	} leap_state;
> ...
>
>> I don't think I am explaining this very well. I will try again to make
>> it clear using a table or something later on.
> The following table illustrates what happens around a (fictitious)
> leap second. Imagine a new epoch will occur at UTC time value 10 and
> the new TAI - UTC offset will be 2 seconds. The columns of the table
> show the values of the relevant time variables.
>
> U:     UTC time
> CODE:  NTP time code
> T:     TAI - UTC offset
> P:     pending (explained below)
>
>     U   CODE    T   P
>   --------------------
>     1   INS     1   1  leap second sheduled
>   --------------------

(Continue reading)

headers

Richard Cochran | 23 May 2012 10:29

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Tue, May 22, 2012 at 11:06:09AM -0700, John Stultz wrote:
> On 05/22/2012 10:39 AM, Richard Cochran wrote:
> >On Mon, May 21, 2012 at 09:08:15PM +0200, Richard Cochran wrote:
> >>On Mon, May 21, 2012 at 11:09:51AM -0700, John Stultz wrote:
> >>>On 05/18/2012 07:09 AM, Richard Cochran wrote:
> >>>>+	/* Tracks where we stand with regard to leap the second epoch. */
> >>>>+	enum {
> >>>>+		LEAP_IDLE,
> >>>>+		LEAP_INS_PENDING,
> >>>>+		LEAP_INS_DONE,
> >>>>+		LEAP_DEL_PENDING,
> >>>>+		LEAP_DEL_DONE,
> >>>>+	} leap_state;
> >...
> >
> >>I don't think I am explaining this very well. I will try again to make
> >>it clear using a table or something later on.
> >The following table illustrates what happens around a (fictitious)
> >leap second. Imagine a new epoch will occur at UTC time value 10 and
> >the new TAI - UTC offset will be 2 seconds. The columns of the table
> >show the values of the relevant time variables.
> >
> >U:     UTC time
> >CODE:  NTP time code
> >T:     TAI - UTC offset
> >P:     pending (explained below)
> >
> >    U   CODE    T   P
> >  --------------------
> >    1   INS     1   1  leap second sheduled

(Continue reading)

headers

John Stultz | 23 May 2012 18:50

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/23/2012 01:29 AM, Richard Cochran wrote:
> On Tue, May 22, 2012 at 11:06:09AM -0700, John Stultz wrote:
>> It seems currently we have:
>>
>>     U   CODE    T
>>   ----------------
>>     9   INS     1
>>   ----------------
>>    10   INS     1     pre tick, post leap second edge (this is the technically incorrect interval)
>>   ~~~~~~~~~~~~~~~~
>>     9   OOP     2     post tick, post leap second edge
>>   ----------------
>>    10   WAIT    2     new epoch
>>
>>
>> If you're trying to correct the pre-tick, post leap second edge, the above provides all you need.
>>
>> In the adjtimex code, all you have to do is:
>>
>>
>> if (unlikely(CODE == INS&&   U == 10))
>>
>> 	/*note, we're not modifying state here, just returning corrected local values*/
>>
>> 	return (U-1, OOP, T+1);
>>
>> return (U,CODE, T);
> Okay, if you want it that way, then you will have to add the other
> cases. For example:
>

(Continue reading)

headers

Richard Cochran | 23 May 2012 21:17

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Wed, May 23, 2012 at 09:50:13AM -0700, John Stultz wrote:
> On 05/23/2012 01:29 AM, Richard Cochran wrote:
> >Okay, if you want it that way, then you will have to add the other
> >cases. For example:
> >
> >	switch (code) {
> >	case INS:
> >		if (U == epoch) {
> >			U--;
> >			T++;
> >			code = OOP;
> >		}
> >		break;
> >	case OOP:
> >		if (U == epoch) {
> epoch + 1 here, right?

No, I really mean epoch (not the leap second value, but the value when
the new TAI offset comes into effect).

> >			code = WAIT;
> >		}
> >		break;
> >	case DEL:
> >		if (U == epoch - 1) {
> >			U++;
> >			T--;
> >			code = WAIT;
> >		}
> >		break;

(Continue reading)

headers

John Stultz | 23 May 2012 22:18

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/23/2012 12:17 PM, Richard Cochran wrote:
> On Wed, May 23, 2012 at 09:50:13AM -0700, John Stultz wrote:
>> On 05/23/2012 01:29 AM, Richard Cochran wrote:
>>> Okay, if you want it that way, then you will have to add the other
>>> cases. For example:
>>>
>>> 	switch (code) {
>>> 	case INS:
>>> 		if (U == epoch) {
>>> 			U--;
>>> 			T++;
>>> 			code = OOP;
>>> 		}
>>> 		break;
>>> 	case OOP:
>>> 		if (U == epoch) {
>> epoch + 1 here, right?
> No, I really mean epoch (not the leap second value, but the value when
> the new TAI offset comes into effect).
>
>>> 			code = WAIT;
>>> 		}
>>> 		break;
>>> 	case DEL:
>>> 		if (U == epoch - 1) {
>>> 			U++;
>>> 			T--;
>>> 			code = WAIT;
>>> 		}
>>> 		break;

(Continue reading)

headers

Richard Cochran | 24 May 2012 08:43

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Wed, May 23, 2012 at 01:18:27PM -0700, John Stultz wrote:
> So I've avoided the term epoch just to try not to confuse things
> with the unix epoch, that's why I've used next_leap, etc.
> Even so, I'm not sure you've made clear the subtlety of the difference.

For me, when working with the whole UTC leap second mess, it is
important to distinguish between the leap seconds and the epochs. I
sometime have written mini-epoch to make clear that it is not about
*the* epoch from 1970.

The NIST leap second table give a list of epochs and TAI offsets. The
leap second is always the second just before the epoch. For example:

 30 June 1972 23:59:59 (NTP 2287785599,  first time)  <-- normal second
 30 June 1972 23:59:60 (NTP 2287785599, second time)  <-- leap second
 1  July 1972 00:00:00 (NTP 2287785600)               <-- epoch

We should write the thresholding code so that it is clear whether we
are testing against the epoch or the leap second.

> I believe the internal time_state (along with the next leap second)
> already provides this.
> 
> From the reader's perspective:
> 
> Not applied:		(INS&&  U<  leap):		return (INS, U)
> Applied:		(INS&&   U == leap):		return (OOP, U-1)
> Finished applied:	((INS||OOP)&&  U>= (leap+1)):	return (WAIT,U-1)
> Delete:			(DEL&&  U>= (leap-1)):		return (WAIT,U+1)

(Continue reading)

headers

Richard Cochran | 24 May 2012 08:57

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Thu, May 24, 2012 at 08:43:40AM +0200, Richard Cochran wrote:
> BTW you can use the program I have been using to test this at
> 
>    git://github.com/richardcochran/leap.git

That program exposes another leap second bug, too, I think. It reads
the time via adjtimex in a tight loop, optionally sleeping using

	clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL);

The program does not wake from this call during a leap second. It is
my expectation that CLOCK_MONOTONIC should always work. Why doesn't
it?

Thanks,
Richard

headers

Richard Cochran | 26 May 2012 17:07

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Thu, May 24, 2012 at 08:57:28AM +0200, Richard Cochran wrote:
> On Thu, May 24, 2012 at 08:43:40AM +0200, Richard Cochran wrote:
> > BTW you can use the program I have been using to test this at
> > 
> >    git://github.com/richardcochran/leap.git
> 
> That program exposes another leap second bug, too, I think. It reads
> the time via adjtimex in a tight loop, optionally sleeping using
> 
> 	clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL);
> 
> The program does not wake from this call during a leap second. It is
> my expectation that CLOCK_MONOTONIC should always work. Why doesn't
> it?

FWIW, the V1 of this patch series fixes this problem, but V2 doesn't.

Maybe that fact promotes my original idea?

Thanks,
Richard

headers

John Stultz | 30 May 2012 03:46

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/23/2012 11:57 PM, Richard Cochran wrote:
> On Thu, May 24, 2012 at 08:43:40AM +0200, Richard Cochran wrote:
>> BTW you can use the program I have been using to test this at
>>
>>     git://github.com/richardcochran/leap.git
> That program exposes another leap second bug, too, I think. It reads
> the time via adjtimex in a tight loop, optionally sleeping using
>
> 	clock_nanosleep(CLOCK_MONOTONIC, 0,&ts, NULL);
>
> The program does not wake from this call during a leap second. It is
> my expectation that CLOCK_MONOTONIC should always work. Why doesn't
> it?

Sorry for being slow here, just got a chance to look at this.

Does the following patch solve this issue for you?

thanks
-john

Make sure we update wall_to_monotonic when adding a leapsecond. This 
could otherwise cause discontinuities in CLOCK_MONOTONIC.

Reported-by: Richard Cochran <richardcochran <at> gmail.com>
Signed-off-by: John Stultz <john.stultz <at> linaro.org>

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6e46cac..fb95654 100644
--- a/kernel/time/timekeeping.c

(Continue reading)

headers

John Stultz | 30 May 2012 03:49

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/29/2012 06:46 PM, John Stultz wrote:
> On 05/23/2012 11:57 PM, Richard Cochran wrote:
>> On Thu, May 24, 2012 at 08:43:40AM +0200, Richard Cochran wrote:
>>> BTW you can use the program I have been using to test this at
>>>
>>>     git://github.com/richardcochran/leap.git
>> That program exposes another leap second bug, too, I think. It reads
>> the time via adjtimex in a tight loop, optionally sleeping using
>>
>>     clock_nanosleep(CLOCK_MONOTONIC, 0,&ts, NULL);
>>
>> The program does not wake from this call during a leap second. It is
>> my expectation that CLOCK_MONOTONIC should always work. Why doesn't
>> it?
>
> Sorry for being slow here, just got a chance to look at this.
>
> Does the following patch solve this issue for you?

Sorry, attached the wrong patch (that one doesn't build!).

Try this one.

thanks
-john

Make sure we update wall_to_monotonic when adding a leapsecond.
This could otherwise cause discontinuities in CLOCK_MONOTONIC.

Reported-by: Richard Cochran <richardcochran <at> gmail.com>

(Continue reading)

headers

Richard Cochran | 30 May 2012 07:11

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Tue, May 29, 2012 at 06:49:30PM -0700, John Stultz wrote:
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 6e46cac..81c76a9 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
>  <at>  <at>  -962,6 +962,7  <at>  <at>  static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
>  		timekeeper.xtime.tv_sec++;
>  		leap = second_overflow(timekeeper.xtime.tv_sec);
>  		timekeeper.xtime.tv_sec += leap;
> +		timekeeper.wall_to_monotonic.tv_sec -= leap;

Don't you need this in update_wall_time() too?

BTW I suggest refactoring this code (two almost identical if{} bodies)
into a shared helper function.

Thanks,
Richard

headers

John Stultz | 30 May 2012 07:56

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/29/2012 10:11 PM, Richard Cochran wrote:
> On Tue, May 29, 2012 at 06:49:30PM -0700, John Stultz wrote:
>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>> index 6e46cac..81c76a9 100644
>> --- a/kernel/time/timekeeping.c
>> +++ b/kernel/time/timekeeping.c
>>  <at>  <at>  -962,6 +962,7  <at>  <at>  static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
>>   		timekeeper.xtime.tv_sec++;
>>   		leap = second_overflow(timekeeper.xtime.tv_sec);
>>   		timekeeper.xtime.tv_sec += leap;
>> +		timekeeper.wall_to_monotonic.tv_sec -= leap;
> Don't you need this in update_wall_time() too?
Yep. Good point.

> BTW I suggest refactoring this code (two almost identical if{} bodies)
> into a shared helper function.
>
Sounds good, although since the logic is ever so slightly different I 
might split up the pure fix and do the refactoring later so the fix is 
easier to apply to -stable branches.

thanks
-john

headers

Richard Cochran | 30 May 2012 08:19

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Tue, May 29, 2012 at 10:56:18PM -0700, John Stultz wrote:
> On 05/29/2012 10:11 PM, Richard Cochran wrote:
> >On Tue, May 29, 2012 at 06:49:30PM -0700, John Stultz wrote:
> >>diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> >>index 6e46cac..81c76a9 100644
> >>--- a/kernel/time/timekeeping.c
> >>+++ b/kernel/time/timekeeping.c
> >> <at>  <at>  -962,6 +962,7  <at>  <at>  static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
> >>  		timekeeper.xtime.tv_sec++;
> >>  		leap = second_overflow(timekeeper.xtime.tv_sec);
> >>  		timekeeper.xtime.tv_sec += leap;
> >>+		timekeeper.wall_to_monotonic.tv_sec -= leap;
> >Don't you need this in update_wall_time() too?
> Yep. Good point.

Okay, so I can confirm that this fixes the CLOCK_MONOTONIC timer issue
during a leap second.

Thanks,
Richard

headers

John Stultz | 30 May 2012 08:23

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/29/2012 11:19 PM, Richard Cochran wrote:
> On Tue, May 29, 2012 at 10:56:18PM -0700, John Stultz wrote:
>> On 05/29/2012 10:11 PM, Richard Cochran wrote:
>>> On Tue, May 29, 2012 at 06:49:30PM -0700, John Stultz wrote:
>>>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>>>> index 6e46cac..81c76a9 100644
>>>> --- a/kernel/time/timekeeping.c
>>>> +++ b/kernel/time/timekeeping.c
>>>>  <at>  <at>  -962,6 +962,7  <at>  <at>  static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
>>>>   		timekeeper.xtime.tv_sec++;
>>>>   		leap = second_overflow(timekeeper.xtime.tv_sec);
>>>>   		timekeeper.xtime.tv_sec += leap;
>>>> +		timekeeper.wall_to_monotonic.tv_sec -= leap;
>>> Don't you need this in update_wall_time() too?
>> Yep. Good point.
> Okay, so I can confirm that this fixes the CLOCK_MONOTONIC timer issue
> during a leap second.
Thanks! Is it ok if I add your Tested-by:  to the patch?

thanks
-john

headers

Richard Cochran | 30 May 2012 09:27

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Tue, May 29, 2012 at 11:23:27PM -0700, John Stultz wrote:
> On 05/29/2012 11:19 PM, Richard Cochran wrote:
> >On Tue, May 29, 2012 at 10:56:18PM -0700, John Stultz wrote:
> >>On 05/29/2012 10:11 PM, Richard Cochran wrote:
> >>>On Tue, May 29, 2012 at 06:49:30PM -0700, John Stultz wrote:
> >>>>diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> >>>>index 6e46cac..81c76a9 100644
> >>>>--- a/kernel/time/timekeeping.c
> >>>>+++ b/kernel/time/timekeeping.c
> >>>> <at>  <at>  -962,6 +962,7  <at>  <at>  static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
> >>>>  		timekeeper.xtime.tv_sec++;
> >>>>  		leap = second_overflow(timekeeper.xtime.tv_sec);
> >>>>  		timekeeper.xtime.tv_sec += leap;
> >>>>+		timekeeper.wall_to_monotonic.tv_sec -= leap;
> >>>Don't you need this in update_wall_time() too?
> >>Yep. Good point.
> >Okay, so I can confirm that this fixes the CLOCK_MONOTONIC timer issue
> >during a leap second.
> Thanks! Is it ok if I add your Tested-by:  to the patch?

Yes, by all means, thanks.

Richard

headers

Richard Cochran | 23 May 2012 21:42

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Wed, May 23, 2012 at 09:50:13AM -0700, John Stultz wrote:
> 
> Thus the only additional state you might need over what we already
> have is the next_leap value.

And that is BTW exactly what my patches do.

The added enumerated state variable 'leap_state' in timekeeper.c is
balanced by the removal of 'time_state' in ntp.c.

Thanks,
Richard

headers

John Stultz | 21 May 2012 20:21

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On 05/18/2012 07:09 AM, Richard Cochran wrote:
> This patch introduces time keeping variables to track the next
> mini-epoch between the UTC and TAI timescales. A leap second occurs
> one second before a mini-epoch. When no leap second is pending, then
> the epoch is set to the far future, LONG_MAX.
>
> This code will become useful later on for providing correct time
> surrounding a leap second.
>
> Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>
> ---
>   kernel/time/timekeeping.c |   14 ++++++++++++++
>   1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index d66b213..ac04de4 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
>  <at>  <at>  -70,6 +70,19  <at>  <at>  struct timekeeper {
>   	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
>   	struct timespec raw_time;
>
> +	/* The current TAI - UTC offset */
> +	time_t tai_offset;
> +	/* The UTC time of the next leap second epoch */
> +	time_t utc_epoch;
> +	/* Tracks where we stand with regard to leap the second epoch. */
> +	enum {
> +		LEAP_IDLE,
> +		LEAP_INS_PENDING,

(Continue reading)

headers

Richard Cochran | 21 May 2012 21:13

Re: [PATCH RFC V2 3/6] time: keep track of the pending utc/tai threshold

On Mon, May 21, 2012 at 11:21:18AM -0700, John Stultz wrote:
> On 05/18/2012 07:09 AM, Richard Cochran wrote:
> >This patch introduces time keeping variables to track the next
> >mini-epoch between the UTC and TAI timescales. A leap second occurs
> >one second before a mini-epoch. When no leap second is pending, then
> >the epoch is set to the far future, LONG_MAX.
> >
> >This code will become useful later on for providing correct time
> >surrounding a leap second.
> >
> >Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>
> >---
> >  kernel/time/timekeeping.c |   14 ++++++++++++++
> >  1 files changed, 14 insertions(+), 0 deletions(-)
> >
> >diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> >index d66b213..ac04de4 100644
> >--- a/kernel/time/timekeeping.c
> >+++ b/kernel/time/timekeeping.c
> > <at>  <at>  -70,6 +70,19  <at>  <at>  struct timekeeper {
> >  	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
> >  	struct timespec raw_time;
> >
> >+	/* The current TAI - UTC offset */
> >+	time_t tai_offset;
> >+	/* The UTC time of the next leap second epoch */
> >+	time_t utc_epoch;
> >+	/* Tracks where we stand with regard to leap the second epoch. */
> >+	enum {
> >+		LEAP_IDLE,

(Continue reading)

headers

Richard Cochran | 18 May 2012 16:09

[PATCH RFC V2 2/6] ntp: remove useless parameter

This patch removes some leftover cruft in the NTP code.

Signed-off-by: Richard Cochran <richardcochran <at> gmail.com>
---
 kernel/time/ntp.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index e8c8671..d4d48b0 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
 <at>  <at>  -540,7 +540,7  <at>  <at>  static inline void notify_cmos_timer(void) { }
 /*
  * Propagate a new txc->status value into the NTP state:
  */
-static inline void process_adj_status(struct timex *txc, struct timespec *ts)
+static inline void process_adj_status(struct timex *txc)
 {
 	if ((time_status & STA_PLL) && !(txc->status & STA_PLL)) {
 		time_state = TIME_OK;
 <at>  <at>  -565,10 +565,10  <at>  <at>  static inline void process_adj_status(struct timex *txc, struct timespec *ts)
  * Called with the xtime lock held, so we can access and modify
  * all the global NTP state:
  */
-static inline void process_adjtimex_modes(struct timex *txc, struct timespec *ts)
+static inline void process_adjtimex_modes(struct timex *txc)
 {
 	if (txc->modes & ADJ_STATUS)
-		process_adj_status(txc, ts);
+		process_adj_status(txc);

(Continue reading)

headers

John Stultz | 22 May 2012 01:58

Re: [PATCH RFC V2 2/6] ntp: remove useless parameter

On 05/18/2012 07:09 AM, Richard Cochran wrote:
> This patch removes some leftover cruft in the NTP code.
>
> Signed-off-by: Richard Cochran<richardcochran <at> gmail.com>

FYI: This one didn't apply when I tried to add it to the other cleanups 
you've sent me along with tip/timers/core.
Please rework it against the tree I sent Thomas and I'll queue it up.

thanks
-john