Love the idea. The days of leap seconds are numbered though... will look at the code in detail tonight or early next week.

One concern: what dies code written for Linux do with EINVAL?

None of the Linux code I have seen checks the return value of clock_gettime(), since the return value on Linux is useless. The approaches I have seen taken are either hope-and-pray-the-result-is-right (the more common one) or to use some elaborate external checks (e.g. calling ntp_gettime() and checking for the TAI offset to be non-zero) to make sure that the kernel has the right TAI offset programmed in. We could conceivably add bug-for-bug compatibility and make it give UTC time *and* return EINVAL, but I doubt there is any code that really wants CLOCK_TAI to return UTC time and then suddenly jump 37 seconds later on.

As an aside, I tested the behavior of this code during leap seconds using this Linux test and it does the right thing, incrementing monotonically without glitches:

https://github.com/torvalds/linux/blob/5189dafa4cf950e675f02ee04b577dfbbad0d9b1/tools/testing/selftests/timers/leap-a-day.c

Just noticed you didn't upload a full context diff (-U99999). Please refresh with that.
I'm trying to check on the detail that clock_gettime() should return -1 when the clock cannot be fetched, with errno set to EINVAL (posix only defines that for 'clock type unknown' so there may be some ambuity).
I'm surprised that Linux doesn't follow this protocol and have a meaningful errno for clock_gettime(). Their time-keeping stuff is usually top notch and this seems overly sloppy.
We're almost to the point where we can just encode the TAI offset into the kernel and have it be right forever....

Yeah, Linux's lack of error propagation was a surprise to me too. I have somewhat mixed feelings about EINVAL as the error code: it is supposed to be for if the clock doesn't exist rather than clocks that are not configured properly, but the TAI clock also doesn't exactly exist in some sense before the TAI-UTC offset is programmed.

As a brief suppression of a very long rant, I really wish POSIX had defined time_t not to include leap seconds back in 2001 and we had just handled leap seconds through the time zone database as a detail of converting to local time. It would have been so easy and this is such an incredible mess. That we are now contemplating redefining UTC as TAI-37, I'm sure with a new "real UTC" timezone afterward, as the preferred way to cover up for sloppiness in POSIX is just mind-boggling.

I have ranted thousands of words on the non unique leap second mapping to time_t. It's the result of bull headed pragmatists vs overly pedantic time lords. Plus a desire not to break the programs that existed at the time. V6 or V7 unix was really the last chance to fix time_t, even though that spelling wouldn't exist for a while still...

At the risk of derailing this thread entirely, having now stared at this whole code path for a while and being ever-more-profoundly annoyed by the whole mess, there are very few places we actually depend on CLOCK_REALTIME being UTC besides that POSIX says it is. It looks like it would be pretty easy to add a CLOCK_UTC definition and then a kernel tunable to switch CLOCK_REALTIME between CLOCK_UTC and CLOCK_TAI. Having a CLOCK_UTC, even if it is defined equal to CLOCK_REALTIME, might also help us annotate the limited number of places we actually really want specifically UTC in our own code. It's a thought anyway -- please ignore for the purposes of this review.

As an even further aside, I wrote this code in the course of implementing a data-acquisition system for a telescope. So I should be the poster child for really wanting UTC timestamps. And yet...

In D46268#1055515, @nwhitehorn wrote:

At the risk of derailing this thread entirely, having now stared at this whole code path for a while and being ever-more-profoundly annoyed by the whole mess, there are very few places we actually depend on CLOCK_REALTIME being UTC besides that POSIX says it is. It looks like it would be pretty easy to add a CLOCK_UTC definition and then a kernel tunable to switch CLOCK_REALTIME between CLOCK_UTC and CLOCK_TAI. Having a CLOCK_UTC, even if it is defined equal to CLOCK_REALTIME, might also help us annotate the limited number of places we actually really want specifically UTC in our own code. It's a thought anyway -- please ignore for the purposes of this review.

As an even further aside, I wrote this code in the course of implementing a data-acquisition system for a telescope. So I should be the poster child for really wanting UTC timestamps. And yet...

Leap seconds must die. They are inherently evil, a poor match to too many user's time keeping needs and were a gross hack to keep UTC within 1s of UT1 so celestial navigators could not change their stuff. But IMO we'd have been better off if leap seconds never happened and the UTC/UT1/TAI drift turned into a table that updates every year or two the celestial navigators could easily cope. But that ship sailed in the 1972.

Most users don't care. those that do care, however, care off scale that their way is right and all other ways are wrong.

Today, most people that write control systems use CLOCK_MONOTONIC with various hack to synchronize it to some clock / timescale they care about (in the sense that they want to schedule at UTC X so they schedule for CLOCK_MONOTONIC Y which is typically X + delta(X), with various hacks to cope with leap seconds (which is why delta is a function).

So I think inventing a new CLOCK_UTC might be a mistake. CLOCK_MONOTONIC should be used most places where we do an absolute sleep. and CLOCK_REALTIME is almost always used to get a time to present to the user, which is UTC, except for the very rare folks that use a different time scale: GPS, Julian Date (!), Loran, GLOSNOS, etc...

So it's a mess, no matter what. I think we're better off being a mess that's compatible with Linux unless there's a really good reason not to.

<rant off>
So the only useful feedback I have is if you don't implement this for some system calls, that should be documented in their man pages.

Thanks -- my commit bit has sadly expired, so I will update the diff here a last time if you are able to commit. Otherwise, I can try to reactivate it.

In D46268#1094187, @imp wrote:

My apologies for being so tardy to review this. I found one issue where FBCLOCK_LEAPSEC is never set anywhere, suggesting the change is incomplete. I'd expect it be set for the new TAI stuff, so I don't see how these changes could work. But perhaps I'm missing something.

I just added this for conceptual symmetry with the existing API involving FFCLOCK_LEAPSEC so that someone in the future could use it. There is currently no consumer in the kernel for this API, because previously the entire snap2bintime() function was never called anywhere in the kernel when using the feedback clock. It is now called when using the feedback clock, but only in the context of getting the TAI time.

The rest is relatively. I don't think we need to have both a bool that says the tai offset is set and the tai offset. It will be positive for the foreseeable future. Either the earth will behave like it has for the last few hundred years and we'll generally see this number increase, or we'll have some effect that lasts for hundreds of years, needing 37 negative leap seconds, which would require about 200 years or so to accumulate looking at LOD trends even in the current anomaly. We'd have a very long time to fix it. But we'd likely never need to since leap seconds are scheduled to be phased out soon, and then we can just initialize time_tai = 37 in the kernel...

That is a fair point, will fix.

Oh, and one tiny man page tweak.

Thanks!

In D46268#1094187, @imp wrote:

My apologies for being so tardy to review this. I found one issue where FBCLOCK_LEAPSEC is never set anywhere, suggesting the change is incomplete. I'd expect it be set for the new TAI stuff, so I don't see how these changes could work. But perhaps I'm missing something.

I just added this for conceptual symmetry with the API involving FFCLOCK_LEAPSEC

The rest is relatively. I don't think we need to have both a bool that says the tai offset is set and the tai offset. It will be positive for the foreseeable future. Either the earth will behave like it has for the last few hundred years and we'll generally see this number increase, or we'll have some effect that lasts for hundreds of years, needing 37 negative leap seconds, which would require about 200 years or so to accumulate looking at LOD trends even in the current anomaly. We'd have a very long time to fix it. But we'd likely never need to since leap seconds are scheduled to be phased out soon, and then we can just initialize time_tai = 37 in the kernel...

Oh, and one tiny man page tweak.