Re: [PATCH v3] x86/vdso: Handle clock_gettime(CLOCK_TAI) in vDSO

[Andy Lutomirski]

(Hi, Florian!)

On Fri, Aug 31, 2018 at 6:59 PM, Matt Rickard <matt@xxxxxxxxxxxxxxx> wrote:
> Process clock_gettime(CLOCK_TAI) in vDSO.
> This makes the call about as fast as CLOCK_REALTIME and CLOCK_MONOTONIC:
>
>   nanoseconds
>  before after clockname
>    ---- ----- ---------
>     233    87 CLOCK_TAI
>      96    93 CLOCK_REALTIME
>      88    87 CLOCK_MONOTONIC

Are you sure you did this right?  With the clocksource set to TSC
(which is the only reasonable choice unless KVM has seriously cleaned
up its act), with retpolines enabled, I get 24ns for CLOCK_MONOTONIC
without your patch and 32ns with your patch.  And there is indeed a
retpoline in the disassembled output:

  e5:   e8 07 00 00 00          callq  f1 <__vdso_clock_gettime+0x31>
  ea:   f3 90                   pause
  ec:   0f ae e8                lfence
  ef:   eb f9                   jmp    ea <__vdso_clock_gettime+0x2a>
  f1:   48 89 04 24             mov    %rax,(%rsp)
  f5:   c3                      retq

You're probably going to have to set -fno-jump-tables or do something
clever like adding a whole array of (seconds, nsec) in gtod and
indexing that array by the clock id.

Meanwhile, I wrote the following trivial patch to add a
__vdso_clock_gettime_monotonic export.  It runs in 21ns, and I suspect
that the speedup is even a bit bigger when cache-cold because it
avoids some branches.  What do you all think?  Florian, do you think
glibc would be willing to add some magic to turn
clock_gettime(CLOCK_MONOTONIC, t) into
__vdso_clock_gettime_monotonic(t) when CLOCK_MONOTONIC is a constant?
diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
index 91ed1bb2a3bb..4f22e9cb97a5 100644
--- a/arch/x86/entry/vdso/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vclock_gettime.c
@@ -319,6 +319,14 @@ notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
 int clock_gettime(clockid_t, struct timespec *)
 	__attribute__((weak, alias("__vdso_clock_gettime")));
 
+notrace int __vdso_clock_gettime_monotonic(struct timespec *ts)
+{
+	if (likely(do_monotonic(ts) != VCLOCK_NONE))
+		return 0;
+
+	return vdso_fallback_gettime(CLOCK_MONOTONIC, ts);
+}
+
 notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone *tz)
 {
 	if (likely(tv != NULL)) {
diff --git a/arch/x86/entry/vdso/vdso.lds.S b/arch/x86/entry/vdso/vdso.lds.S
index d3a2dce4cfa9..28e23cbc02c9 100644
--- a/arch/x86/entry/vdso/vdso.lds.S
+++ b/arch/x86/entry/vdso/vdso.lds.S
@@ -15,6 +15,11 @@
  * This controls what userland symbols we export from the vDSO.
  */
 VERSION {
+	LINUX_4.19 {
+	global:
+		__vdso_clock_gettime_monotonic;
+	};
+
 	LINUX_2.6 {
 	global:
 		clock_gettime;