Page MenuHomeFreeBSD

clock_gettime: Add Linux aliases for CLOCK_*
ClosedPublic

Authored by imp on Jul 1 2021, 8:14 PM.

Details

Summary

Linux standardized what we call CLOCK_{REALTIME,MONOTONIC}_FAST as
CLOCK_{REALTIME,MONOTONIC}_COARSE. In addition, Linux spells
CLOCK_UPTIME as CLOCK_BOOTTIME.

Add aliases to time.h and document these new aliases in
clock_gettime(2).

Sponsored by: Netflix

Test Plan

Cast a wide net for this review since rwatson introduced them, but others have touched adjacent code
Linux does not have a CLOCK_UPTIME_COARSE, so I didn't create an alias for CLOCK_UPTIME_FAST.
However, it does have CLOCK_BOOTTIME instead of CLOCK_UPTIME, so I did that too.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

imp requested review of this revision.Jul 1 2021, 8:14 PM
imp edited the test plan for this revision. (Show Details)
kevans added inline comments.
lib/libc/sys/clock_gettime.2
112

s/wit/with/

lib/libc/sys/clock_gettime.2
112

Changed locally. Will update if there's other changes.

imp marked an inline comment as done.Jul 1 2021, 8:47 PM

We are already somewhat non-compliant to POSIX by providing more CLOCK_ symbols than defined by the standard. More, POSIX does not allow to pollute CLOCK_ from sys/time.h, but for time.h all symbols with CLOCK_ prefix are reserved.

I think that cleanup is due, but this change is fine until it.

This revision is now accepted and ready to land.Jul 1 2021, 8:49 PM
In D30988#697219, @kib wrote:

We are already somewhat non-compliant to POSIX by providing more CLOCK_ symbols than defined by the standard. More, POSIX does not allow to pollute CLOCK_ from sys/time.h, but for time.h all symbols with CLOCK_ prefix are reserved.

What's the proper symbol to hide these behind? Is it like errno.h (#ifndef _POSIX_SOURCE)? Or something else?

imp added a reviewer: dchagin.

Linux also uses CLOCK_BOOTTIME (see D30985). Might it be possible to also add this along with this?

In D30988#697785, @bz wrote:

Linux also uses CLOCK_BOOTTIME (see D30985). Might it be possible to also add this along with this?

I don't see why not, modulo kib@'s concerns about namespace pollution.

git 11:11am rebo:[15537]> git diff
diff --git a/sys/sys/time.h b/sys/sys/time.h
index 04b57cfea125..9fbae544cb27 100644
--- a/sys/sys/time.h
+++ b/sys/sys/time.h
@@ -482,6 +482,7 @@ struct clockinfo {
 #ifndef CLOCK_MONOTONIC
 #define        CLOCK_MONOTONIC 4
 #define        CLOCK_UPTIME    5               /* FreeBSD-specific. */
+#define        CLOCK_BOOTTIME          CLOCK_UPTIME            /* Linux compat */
 #define        CLOCK_UPTIME_PRECISE    7       /* FreeBSD-specific. */
 #define        CLOCK_UPTIME_FAST       8       /* FreeBSD-specific. */
 #define        CLOCK_REALTIME_PRECISE  9       /* FreeBSD-specific. */

is what you have in mind, eh?

my only worry about adding it is that I requested an exp-run with this change already, so I'd have to request another one I think... and I'm out of time this morning to take care of it before the BBQ I'm headed to...

In D30988#697788, @imp wrote:

my only worry about adding it is that I requested an exp-run with this change already, so I'd have to request another one I think... and I'm out of time this morning to take care of it before the BBQ I'm headed to...

ah, I did a half-assed version that's sufficient for exp-run testing and updated the bug:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256941

I'm not familiar with the process here, but if it hasn't started yet, then we'll be good.

In D30988#697789, @imp wrote:
In D30988#697788, @imp wrote:

my only worry about adding it is that I requested an exp-run with this change already, so I'd have to request another one I think... and I'm out of time this morning to take care of it before the BBQ I'm headed to...

ah, I did a half-assed version that's sufficient for exp-run testing and updated the bug:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256941

I'm not familiar with the process here, but if it hasn't started yet, then we'll be good.

.. for as long as my original #define was correct?

imp retitled this revision from clock_gettime: Add aliases for CLOCK_{REALTIME,MONOTONIC}_COARSE to clock_gettime: Add Linux aliases for CLOCK_*.Jul 4 2021, 12:14 AM
imp edited the summary of this revision. (Show Details)
imp edited the test plan for this revision. (Show Details)

CLOCK_BOOTTIME adjustments

This revision now requires review to proceed.Jul 4 2021, 12:15 AM
In D30988#697795, @bz wrote:

.. for as long as my original #define was correct?

I think that it was. Please verify I got it correct though here.

This revision is now accepted and ready to land.Jul 4 2021, 10:51 AM
This revision now requires review to proceed.Jul 5 2021, 10:04 PM
kib added inline comments.
sys/sys/_clock_id.h
88

I suggest to add 'Linux compat' herald comment right before '__BSD_VISIBLE' test for the whole block, and not repeat it for each line.

Might be, makes sense to do the same for 'FreeBSD-specific' comments above.

This revision is now accepted and ready to land.Jul 6 2021, 6:20 PM

update with kib's suggested comment

This revision now requires review to proceed.Jul 6 2021, 11:46 PM
This revision is now accepted and ready to land.Jul 7 2021, 12:02 PM
This revision now requires review to proceed.Jul 23 2021, 4:47 AM
This revision is now accepted and ready to land.Jul 23 2021, 7:19 PM
This revision was automatically updated to reflect the committed changes.