| Message ID | AANLkTini2WdT-1v4k9V3JOZYDkA59P+SyscTe8-fK2Wk@mail.gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, Jan 02, 2011 at 05:38:19AM +0900, Kuwahara,T. wrote: > As you know, it conflicts with MOD_PPSMAX. And also, it is logically > the same as ADJ_OFFSET, unless the kernel PLL is enabled explicitly. I choose another bit, for the next version of the patch series. > > So here's my simple solution: > I have read the API documentation in the nanokernel source archive. It explains the (very complex looking) timex structure quite clearly and nicely. Now I am more convinced than ever that adding a new mode bit is the best way to go, as opposed to ADJ_OFFSET/!STA_PLL, because: 1. The mode bits update kernel variables. That is what we want. 2. Clearing STA_PLL means disable adjustments. 3. The range of the timex.offset is way too small. I expand on each point, below. BTW, the API document is also available for online reading here: http://www.slac.stanford.edu/comp/unix/package/rtems/src/ssrlApps/ntpNanoclock/api.htm 1. Looking at the API, the documentation for the bits of the 'modes' field states: These bits control which field of the timex structure are used to update the corresponding kernel variable. The bits may be set in any combination. See the description below for which bits control which variable. With the ADJ_SETOFFSET mode, we are telling the kernel to update the instantaneous value of the 'current time' variable. That usage agrees with the sematics of the other mode bits. 2. The documentation for STA_PLL states: Master enable switch for the PLL/FLL loop. The algorithm is responsive to time and/or frequency updates if set; otherwise, no change in the current time or frequency will be made other than to complete a pending phase adjustment. This bit does not affect the PPS loop. So when we clear this bit, the kernel promises that it will make "no change in the current time." The proposed ADJ_OFFSET/!STA_PLL solution would break this pattern. 3. The timex.offset field is of type "long" and represents either nanoseconds or microseconds. On 32 bit architectures, the maximum possible adjustment would be 2^31 * 10^-6 = 2147.5 seconds which is less than one hour. For the first adjustment of a clock, we want to be able to jump the clock arbitrarily. Not every computer has an RTC, and so some boot up believing that it is still the year 1970. Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 9, 2011 at 2:50 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> we want to be able to jump the clock arbitrarily.
Another problem remains: How do you deal with leap seconds? I mean,
given that 1 minute is not always 60 seconds, then what time was it
XXXXX seconds ago? Maybe some kind of lookup table is necessary, but
in such case, isn't it a better choice just to use the
clock_settime/settimeofday syscall?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 10, 2011 at 06:07:26AM +0900, Kuwahara,T. wrote: > On Sun, Jan 9, 2011 at 2:50 AM, Richard Cochran > <richardcochran@gmail.com> wrote: > > we want to be able to jump the clock arbitrarily. > > Another problem remains: How do you deal with leap seconds? I mean, > given that 1 minute is not always 60 seconds, then what time was it > XXXXX seconds ago? Maybe some kind of lookup table is necessary, but > in such case, isn't it a better choice just to use the > clock_settime/settimeofday syscall? Well, first of all, the PTP Hardware Clocks for which this whole patch set was created in the first place will keep their time as TAI. Adding seconds to such time values is unambiguous. Secondly, the question you ask applies equally to the existing interfaces, so it is a mute point with regard to the patch series. Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 10, 2011 at 06:07:26AM +0900, Kuwahara,T. wrote: > > Another problem remains: How do you deal with leap seconds? I mean, Since you have changed the topic, does that mean that you now agree with adding a new mode bit? ;) Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-01-10 at 06:07 +0900, Kuwahara,T. wrote: > On Sun, Jan 9, 2011 at 2:50 AM, Richard Cochran > <richardcochran@gmail.com> wrote: > > we want to be able to jump the clock arbitrarily. > > Another problem remains: How do you deal with leap seconds? I mean, > given that 1 minute is not always 60 seconds, then what time was it > XXXXX seconds ago? Maybe some kind of lookup table is necessary, but > in such case, isn't it a better choice just to use the > clock_settime/settimeofday syscall? Leapsecond processing is done via an absolute hrtimer. Thus when the time offset is set, the hrtimers that should have expired will fire (just like with settimeofday) and the adjustment will then be made. thanks -john -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 11, 2011 at 1:49 AM, john stultz <johnstul@us.ibm.com> wrote: > Leapsecond processing is done via an absolute hrtimer. Thus when the > time offset is set, the hrtimers that should have expired will fire > (just like with settimeofday) and the adjustment will then be made. How do you convert relative time to absolute time? It's not trivial because TAI offset is also a variable. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 10, 2011 at 4:17 PM, Richard Cochran <richardcochran@gmail.com> wrote: > the PTP Hardware Clocks for which this whole patch > set was created in the first place will keep their time as TAI. Are you sure of that? I don't have the standard handy (it's non-free, right?) but it seems that the Annex B states differently. But that's not the point anyway. My concern is that your patch not only adds the useless (and broken) feature to the existing syscall but also makes a permanent change to the public interface for your own use. That's what I'm against. So if you stop touching the struct timex, I won't complain anymore. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2011-01-11 at 05:45 +0900, Kuwahara,T. wrote: > On Tue, Jan 11, 2011 at 1:49 AM, john stultz <johnstul@us.ibm.com> wrote: > > Leapsecond processing is done via an absolute hrtimer. Thus when the > > time offset is set, the hrtimers that should have expired will fire > > (just like with settimeofday) and the adjustment will then be made. > > How do you convert relative time to absolute time? It's not trivial > because TAI offset is also a variable. I don't believe I understand what you're getting at. The proposed interface is almost identical in functionality to a userland application doing the following: offset = my_calculate_offset(); clock_gettime(CLOCK_REALTIME, &now); newtime = my_add_ts(now, offset); settimeofday(&newtime, 0); The only difference is that you avoid the error from the delay between the gettime call and the settime call. It just adds the offset directly to the CLOCK_REALTIME. thanks -john -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2011-01-11 at 05:47 +0900, Kuwahara,T. wrote: > On Mon, Jan 10, 2011 at 4:17 PM, Richard Cochran > <richardcochran@gmail.com> wrote: > > the PTP Hardware Clocks for which this whole patch > > set was created in the first place will keep their time as TAI. > > Are you sure of that? I don't have the standard handy (it's non-free, > right?) but it seems that the Annex B states differently. But that's > not the point anyway. My concern is that your patch not only adds the > useless (and broken) feature to the existing syscall but also makes a > permanent change to the public interface for your own use. That's > what I'm against. So if you stop touching the struct timex, I won't > complain anymore. You still haven't explained *why* you're so protective of the timex and adjtimex interfaces. While I do want to keep compatible the functionality where possible, I don't see why Linux should be limited by what other OSes do. Injecting an offset to the system time seems like a reasonable thing for adjtimex to do (rather then adding a new syscall). Further utilizing a new mode bit for this functionality seems reasonable and cleaner then your suggestions for utilizing existing mode bits in combined with other magic bits. If there is a compelling reason why not to do this, do please let us know! We might just agree with you after hearing it. :) thanks -john -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 11, 2011 at 05:47:13AM +0900, Kuwahara,T. wrote: > On Mon, Jan 10, 2011 at 4:17 PM, Richard Cochran > <richardcochran@gmail.com> wrote: > > the PTP Hardware Clocks for which this whole patch > > set was created in the first place will keep their time as TAI. > > Are you sure of that? Yes. > I don't have the standard handy (it's non-free, right?) but it seems > that the Annex B states differently. IEEE Std 1588-2008, Page 41: The timescale PTP: In normal operation, the epoch is the PTP epoch and the timescale is continuous; see 7.2.4. The unit of measure of time is the SI second as realized on the rotating geoid. IEEE Std 1588-2008, Page 42: The PTP epoch is 1 January 1970 00:00:00 TAI, which is 31 December 1969 23:59:51.999918 UTC. The standard does permit using some other, arbitrary timescale, "set by an administrative procedure." But no other timescales are standardized by 1588. > But that's not the point anyway. My concern is that your patch not > only adds the useless (and broken) feature to the existing syscall > but also makes a permanent change to the public interface for your > own use. It is neither useless or broken. Using the new interface, you can estimate offset and frequency for a few seconds and jump directly into a closely synchronized state (like within 100 nanoseconds) relative to another clock on the network. That is pretty useful. Your issue with leap seconds is really a non-issue. The time value of the ADJ_SETOFFSET mode is a time interval, *not* an absolute time. > That's what I'm against. So if you stop touching the struct timex, > I won't complain anymore. I have no love for the timex interface. However, I used it for the new syscall because Arnd Bergmann and John Stultz specifically asked for it. Also, several other people have reviewed the new call without objecting to the new mode. Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 11, 2011 at 6:06 AM, john stultz <johnstul@us.ibm.com> wrote: > On Tue, 2011-01-11 at 05:45 +0900, Kuwahara,T. wrote: >> On Tue, Jan 11, 2011 at 1:49 AM, john stultz <johnstul@us.ibm.com> wrote: >> > Leapsecond processing is done via an absolute hrtimer. Thus when the >> > time offset is set, the hrtimers that should have expired will fire >> > (just like with settimeofday) and the adjustment will then be made. >> >> How do you convert relative time to absolute time? It's not trivial >> because TAI offset is also a variable. > > I don't believe I understand what you're getting at. > > The proposed interface is almost identical in functionality to a > userland application doing the following: > > offset = my_calculate_offset(); > clock_gettime(CLOCK_REALTIME, &now); > newtime = my_add_ts(now, offset); > settimeofday(&newtime, 0); > > The only difference is that you avoid the error from the delay between > the gettime call and the settime call. It just adds the offset directly > to the CLOCK_REALTIME. For example, what time was it 3 seconds after 2008-12-31 23:59:59? You may say, of course it's 2009-01-01 00:00:02. But it's not true. You wonder why? Because a leap second had been added at midnight. unix time UTC offset ---------- -------- --- 1230767997 23:59:57 -2 1230767998 23:59:58 -1 1230767999 23:59:59 0 1230768000 23:59:60 1 (leap second) 1230768000 00:00:00 2 1230768001 00:00:01 3 1230768002 00:00:02 4 That's why I said it's not trivial, and that your patch is broken and thus useless. Unfortunately, there's no remedy for this as long as a nonlinear timescale such as the unix time is being used, since the leap second insertion/deletion is a non-deterministic event. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2011-01-12 at 05:32 +0900, Kuwahara,T. wrote: > On Tue, Jan 11, 2011 at 6:06 AM, john stultz <johnstul@us.ibm.com> wrote: > > On Tue, 2011-01-11 at 05:45 +0900, Kuwahara,T. wrote: > >> On Tue, Jan 11, 2011 at 1:49 AM, john stultz <johnstul@us.ibm.com> wrote: > >> > Leapsecond processing is done via an absolute hrtimer. Thus when the > >> > time offset is set, the hrtimers that should have expired will fire > >> > (just like with settimeofday) and the adjustment will then be made. > >> > >> How do you convert relative time to absolute time? It's not trivial > >> because TAI offset is also a variable. > > > > I don't believe I understand what you're getting at. > > > > The proposed interface is almost identical in functionality to a > > userland application doing the following: > > > > offset = my_calculate_offset(); > > clock_gettime(CLOCK_REALTIME, &now); > > newtime = my_add_ts(now, offset); > > settimeofday(&newtime, 0); > > > > The only difference is that you avoid the error from the delay between > > the gettime call and the settime call. It just adds the offset directly > > to the CLOCK_REALTIME. > > For example, what time was it 3 seconds after 2008-12-31 23:59:59? > You may say, of course it's 2009-01-01 00:00:02. But it's not true. > You wonder why? Because a leap second had been added at midnight. > > unix time UTC offset > ---------- -------- --- > 1230767997 23:59:57 -2 > 1230767998 23:59:58 -1 > 1230767999 23:59:59 0 > 1230768000 23:59:60 1 (leap second) > 1230768000 00:00:00 2 > 1230768001 00:00:01 3 > 1230768002 00:00:02 4 > > That's why I said it's not trivial, and that your patch is broken and > thus useless. So the kernel handles leap second insertion via a timer. Thus at the end of 23:59:59, it will inject a leapsecond by setting the time back by one second (back to 23:59:59) and setting the TIME_OOP flag. This timer is an absolute timer, so if someone moves the clock forward across the 23:59:59 boundary, the adjustment will still be made. The patch is not broken, nor useless. > Unfortunately, there's no remedy for this as long as a > nonlinear timescale such as the unix time is being used, since the > leap second insertion/deletion is a non-deterministic event. Indeed. Leapseconds in unix time are frustrating to deal with, and complicates applications having to be careful with them. But we do manage them properly and applications can detect that they are pending (via checking for TIME_INS) or if they are in effect (TIME_OOP). Richards patch is in no different from settimeofday() solutions for correcting for an offset, with the exception of the fact that it does not introduce error due to delay beteween any gettime and settime call. If your complaint is that leapseconds are ugly to deal with in unix time, then I agree. However, I'm at a total loss for understanding your passionate objections to Richard's patch. thanks -john -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 12, 2011 at 5:55 AM, john stultz <johnstul@us.ibm.com> wrote: > So the kernel handles leap second insertion via a timer. Thus at the end > of 23:59:59, it will inject a leapsecond by setting the time back by one > second (back to 23:59:59) and setting the TIME_OOP flag. > > This timer is an absolute timer, so if someone moves the clock forward > across the 23:59:59 boundary, the adjustment will still be made. > > The patch is not broken, nor useless. It takes into account only one upcoming leap second, but ignores all the others. That's not sufficient for arbitrary adjustments. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2011-01-13 at 05:39 +0900, Kuwahara,T. wrote: > On Wed, Jan 12, 2011 at 5:55 AM, john stultz <johnstul@us.ibm.com> wrote: > > So the kernel handles leap second insertion via a timer. Thus at the end > > of 23:59:59, it will inject a leapsecond by setting the time back by one > > second (back to 23:59:59) and setting the TIME_OOP flag. > > > > This timer is an absolute timer, so if someone moves the clock forward > > across the 23:59:59 boundary, the adjustment will still be made. > > > > The patch is not broken, nor useless. > > It takes into account only one upcoming leap second, but ignores all > the others. That's not sufficient for arbitrary adjustments. If an application wants to manage the full historical table of leapseconds and compensate appropriately, then that's fine. The interface proposed still functions in a reasonable manner. Again, I agree that leapseconds are annoying to deal with. It would be great if time() was defined as TAI time instead of UTC. I'm actually hoping to provide a CLOCK_TAI clockid someday. thanks -john -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index c631168..d492887 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -119,14 +119,21 @@ return div_s64(offset64 << (NTP_SCALE_SHIFT - SHIFT_FLL), secs); } -static void ntp_update_offset(long offset) +static void ntp_update_offset(long offset, struct timespec *ts) { s64 freq_adj; s64 offset64; long secs; - if (!(time_status & STA_PLL)) + if (!(time_status & STA_PLL)) { + offset64 = offset; + if (!(time_status & STA_NANO)) + offset64 *= NSEC_PER_USEC; + + set_normalized_timespec(ts, ts->tv_sec, offset64 + ts->tv_nsec); + return; + } if (!(time_status & STA_NANO)) offset *= NSEC_PER_USEC; @@ -430,7 +437,7 @@ time_tai = txc->constant; if (txc->modes & ADJ_OFFSET) - ntp_update_offset(txc->offset); + ntp_update_offset(txc->offset, ts); if (txc->modes & ADJ_TICK) tick_usec = txc->tick; @@ -526,6 +533,9 @@ write_sequnlock_irq(&xtime_lock); + if ((txc->modes & ADJ_OFFSET) && !(time_status & STA_PLL)) + do_settimeofday(&ts); + txc->time.tv_sec = ts.tv_sec; txc->time.tv_usec = ts.tv_nsec; if (!(time_status & STA_NANO))
On Sat, Jan 1, 2011 at 4:12 AM, Richard Cochran <richardcochran@gmail.com> wrote: > +#define ADJ_SETOFFSET 0x0040 /* add 'time' to current time */ As you know, it conflicts with MOD_PPSMAX. And also, it is logically the same as ADJ_OFFSET, unless the kernel PLL is enabled explicitly. So here's my simple solution: --- -- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html