linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
@ 2018-03-15 16:00 jason.vas.dias
  2018-03-15 16:00 ` [PATCH v4.16-rc5 1/3] " jason.vas.dias
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: jason.vas.dias @ 2018-03-15 16:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: x86, tglx, mingo, peterz, andi


  Resent to address reviewer comments.
   
  Currently, the VDSO does not handle
     clock_gettime( CLOCK_MONOTONIC_RAW, &ts )
  on Intel / AMD - it calls
     vdso_fallback_gettime()
  for this clock, which issues a syscall, having an unacceptably high
  latency (minimum measurable time or time between measurements)
  of 300-700ns on 2 2.8-3.9ghz Haswell x86_64 Family'_'Model : 06_3C
  machines under various versions of Linux.

  Sometimes, particularly when correlating elapsed time to performance
  counter values, user-space  code needs to know elapsed time from the
  perspective of the CPU no matter how "hot" / fast or "cold" / slow it
  might be running wrt NTP / PTP "real" time; when code needs this,
  the latencies associated with a syscall are often unacceptably high.

  I reported this as Bug #198161 :
    'https://bugzilla.kernel.org/show_bug.cgi?id=198961'
  and in previous posts with subjects matching 'CLOCK_MONOTONIC_RAW' .

  This patch handles CLOCK_MONOTONIC_RAW clock_gettime() in the VDSO ,
  by exporting the raw clock calibration, last cycles, last xtime_nsec,
  and last raw_sec value in the vsyscall_gtod_data during vsyscall_update() .

  Now the new do_monotonic_raw() function in the vDSO has a latency of @ 24ns
  on average, and the test program:
   tools/testing/selftest/timers/inconsistency-check.c
  succeeds with arguments: '-c 4 -t 120' or any arbitrary -t value.

  The patch is against Linus' latest 4.16-rc5 tree,
  current HEAD of :
    git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
  .

  This patch affects only files:

   arch/x86/include/asm/vgtod.h
   arch/x86/entry/vdso/vclock_gettime.c
   arch/x86/entry/vdso/vdso.lds.S
   arch/x86/entry/vdso/vdsox32.lds.S
   arch/x86/entry/vdso/vdso32/vdso32.lds.S
   arch/x86/entry/vsyscall/vsyscall_gtod.c

  There are 3 patches in the series :

   Patch #1 makes the VDSO handle clock_gettime(CLOCK_MONOTONIC_RAW) with rdtsc_ordered()

  Patches #2 & #3 should be considered "optional" :

   Patch #2 makes the VDSO handle clock_gettime(CLOCK_MONOTONIC_RAW) with a new rdtscp() function in msr.h

   Patch #3 makes the VDSO export TSC calibration data via a new function in the vDSO:
               unsigned int __vdso_linux_tsc_calibration ( struct linux_tsc_calibration *tsc_cal )
            that user code can optionally call.


   Patch #2 makes clock_gettime(CLOCK_MONOTONIC_RAW) calls somewhat faster
   than clock_gettime(CLOCK_MONOTONIC) calls.

   I think something like Patch #3 is necessary to export TSC calibration data to user-space TSC readers.

   It is entirely up to the kernel developers whether they want to include patches
   #2 and #3, but I think something like Patch #1 really needs to get into a future
   Linux release, as an unecessary latency of 200-1000ns for a timer that can tick
   3 times per nanosecond is unacceptable.

   Patches for kernels 3.10.0-21 and 4.9.65-rt23 (ARM) are attached to bug #198161. 


Thanks & Best Regards,
Jason Vas Dias

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4.16-rc5 1/3] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 16:00 [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW jason.vas.dias
@ 2018-03-15 16:00 ` jason.vas.dias
  2018-03-15 16:00 ` [PATCH v4.16-rc5 2/3] " jason.vas.dias
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: jason.vas.dias @ 2018-03-15 16:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: x86, tglx, mingo, peterz, andi

diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
index f19856d..fbc7371 100644
--- a/arch/x86/entry/vdso/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vclock_gettime.c
@@ -182,6 +182,18 @@ notrace static u64 vread_tsc(void)
 	return last;
 }
 
+notrace static u64 vread_tsc_raw(void)
+{
+	u64 tsc
+	  , last = gtod->raw_cycle_last;
+
+	tsc	      = rdtsc_ordered();
+	if (likely(tsc >= last))
+		return tsc;
+	asm volatile ("");
+	return last;
+}
+
 notrace static inline u64 vgetsns(int *mode)
 {
 	u64 v;
@@ -203,6 +215,27 @@ notrace static inline u64 vgetsns(int *mode)
 	return v * gtod->mult;
 }
 
+notrace static inline u64 vgetsns_raw(int *mode)
+{
+	u64 v;
+	cycles_t cycles;
+
+	if (gtod->vclock_mode == VCLOCK_TSC)
+		cycles = vread_tsc_raw();
+#ifdef CONFIG_PARAVIRT_CLOCK
+	else if (gtod->vclock_mode == VCLOCK_PVCLOCK)
+		cycles = vread_pvclock(mode);
+#endif
+#ifdef CONFIG_HYPERV_TSCPAGE
+	else if (gtod->vclock_mode == VCLOCK_HVCLOCK)
+		cycles = vread_hvclock(mode);
+#endif
+	else
+		return 0;
+	v = (cycles - gtod->raw_cycle_last) & gtod->raw_mask;
+	return v * gtod->raw_mult;
+}
+
 /* Code size doesn't matter (vdso is 4k anyway) and this is faster. */
 notrace static int __always_inline do_realtime(struct timespec *ts)
 {
@@ -246,6 +279,27 @@ notrace static int __always_inline do_monotonic(struct timespec *ts)
 	return mode;
 }
 
+notrace static __always_inline int do_monotonic_raw(struct timespec *ts)
+{
+	unsigned long seq;
+	u64 ns;
+	int mode;
+
+	do {
+		seq = gtod_read_begin(gtod);
+		mode = gtod->vclock_mode;
+		ts->tv_sec = gtod->monotonic_time_raw_sec;
+		ns = gtod->monotonic_time_raw_nsec;
+		ns += vgetsns_raw(&mode);
+		ns >>= gtod->raw_shift;
+	} while (unlikely(gtod_read_retry(gtod, seq)));
+
+	ts->tv_sec += __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
+	ts->tv_nsec = ns;
+
+	return mode;
+}
+
 notrace static void do_realtime_coarse(struct timespec *ts)
 {
 	unsigned long seq;
@@ -277,6 +331,10 @@ notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
 		if (do_monotonic(ts) == VCLOCK_NONE)
 			goto fallback;
 		break;
+	case CLOCK_MONOTONIC_RAW:
+		if (do_monotonic_raw(ts) == VCLOCK_NONE)
+			goto fallback;
+		break;
 	case CLOCK_REALTIME_COARSE:
 		do_realtime_coarse(ts);
 		break;
diff --git a/arch/x86/entry/vsyscall/vsyscall_gtod.c b/arch/x86/entry/vsyscall/vsyscall_gtod.c
index e1216dd..5af7093 100644
--- a/arch/x86/entry/vsyscall/vsyscall_gtod.c
+++ b/arch/x86/entry/vsyscall/vsyscall_gtod.c
@@ -45,6 +45,11 @@ void update_vsyscall(struct timekeeper *tk)
 	vdata->mult		= tk->tkr_mono.mult;
 	vdata->shift		= tk->tkr_mono.shift;
 
+	vdata->raw_cycle_last	= tk->tkr_raw.cycle_last;
+	vdata->raw_mask		= tk->tkr_raw.mask;
+	vdata->raw_mult		= tk->tkr_raw.mult;
+	vdata->raw_shift	= tk->tkr_raw.shift;
+
 	vdata->wall_time_sec		= tk->xtime_sec;
 	vdata->wall_time_snsec		= tk->tkr_mono.xtime_nsec;
 
@@ -74,5 +79,8 @@ void update_vsyscall(struct timekeeper *tk)
 		vdata->monotonic_time_coarse_sec++;
 	}
 
+	vdata->monotonic_time_raw_sec  = tk->raw_sec;
+	vdata->monotonic_time_raw_nsec = tk->tkr_raw.xtime_nsec;
+
 	gtod_write_end(vdata);
 }
diff --git a/arch/x86/include/asm/vgtod.h b/arch/x86/include/asm/vgtod.h
index fb856c9..24e4d45 100644
--- a/arch/x86/include/asm/vgtod.h
+++ b/arch/x86/include/asm/vgtod.h
@@ -22,6 +22,10 @@ struct vsyscall_gtod_data {
 	u64	mask;
 	u32	mult;
 	u32	shift;
+	u64	raw_cycle_last;
+	u64	raw_mask;
+	u32	raw_mult;
+	u32	raw_shift;
 
 	/* open coded 'struct timespec' */
 	u64		wall_time_snsec;
@@ -32,6 +36,8 @@ struct vsyscall_gtod_data {
 	gtod_long_t	wall_time_coarse_nsec;
 	gtod_long_t	monotonic_time_coarse_sec;
 	gtod_long_t	monotonic_time_coarse_nsec;
+	gtod_long_t	monotonic_time_raw_sec;
+	gtod_long_t	monotonic_time_raw_nsec;
 
 	int		tz_minuteswest;
 	int		tz_dsttime;

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4.16-rc5 2/3] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 16:00 [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW jason.vas.dias
  2018-03-15 16:00 ` [PATCH v4.16-rc5 1/3] " jason.vas.dias
@ 2018-03-15 16:00 ` jason.vas.dias
  2018-03-15 16:00 ` [PATCH v4.16-rc5 3/3] " jason.vas.dias
  2018-03-15 20:17 ` [PATCH v4.16-rc5 (3)] " Thomas Gleixner
  3 siblings, 0 replies; 11+ messages in thread
From: jason.vas.dias @ 2018-03-15 16:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: x86, tglx, mingo, peterz, andi

diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
index fbc7371..2c46675 100644
--- a/arch/x86/entry/vdso/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vclock_gettime.c
@@ -184,10 +184,9 @@ notrace static u64 vread_tsc(void)
 
 notrace static u64 vread_tsc_raw(void)
 {
-	u64 tsc
+	u64 tsc  = (gtod->has_rdtscp ? rdtscp((void *)0) : rdtsc_ordered())
 	  , last = gtod->raw_cycle_last;
 
-	tsc	      = rdtsc_ordered();
 	if (likely(tsc >= last))
 		return tsc;
 	asm volatile ("");
diff --git a/arch/x86/entry/vsyscall/vsyscall_gtod.c b/arch/x86/entry/vsyscall/vsyscall_gtod.c
index 5af7093..0327a95 100644
--- a/arch/x86/entry/vsyscall/vsyscall_gtod.c
+++ b/arch/x86/entry/vsyscall/vsyscall_gtod.c
@@ -16,6 +16,9 @@
 #include <linux/timekeeper_internal.h>
 #include <asm/vgtod.h>
 #include <asm/vvar.h>
+#include <asm/cpufeature.h>
+
+extern unsigned int tsc_khz;
 
 int vclocks_used __read_mostly;
 
@@ -49,6 +52,7 @@ void update_vsyscall(struct timekeeper *tk)
 	vdata->raw_mask		= tk->tkr_raw.mask;
 	vdata->raw_mult		= tk->tkr_raw.mult;
 	vdata->raw_shift	= tk->tkr_raw.shift;
+	vdata->has_rdtscp	= static_cpu_has(X86_FEATURE_RDTSCP);
 
 	vdata->wall_time_sec		= tk->xtime_sec;
 	vdata->wall_time_snsec		= tk->tkr_mono.xtime_nsec;
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 30df295..a5ff704 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -218,6 +218,37 @@ static __always_inline unsigned long long rdtsc_ordered(void)
 	return rdtsc();
 }
 
+/**
+ * rdtscp() - read the current TSC and (optionally) CPU number, with built-in
+ *            cancellation point replacing barrier - only available
+ *            if static_cpu_has(X86_FEATURE_RDTSCP) .
+ * returns:   The 64-bit Time Stamp Counter (TSC) value.
+ * Optionally, 'cpu_out' can be non-null, and on return it will contain
+ * the number (Intel CPU ID) of the CPU that the task is currently running on.
+ * As does EAX_EDT_RET, this uses the "open-coded asm" style to
+ * force the compiler + assembler to always use (eax, edx, ecx) registers,
+ * NOT whole (rax, rdx, rcx) on x86_64 , because only 32-bit
+ * variables are used - exactly the same code should be generated
+ * for this instruction on 32-bit as on 64-bit when this asm stanza is used.
+ * See: SDM , Vol #2, RDTSCP instruction.
+ */
+static __always_inline u64 rdtscp(u32 *cpu_out)
+{
+	u32	tsc_lo, tsc_hi, tsc_cpu;
+
+	asm volatile
+	("rdtscp"
+		:   "=a" (tsc_lo)
+		  , "=d" (tsc_hi)
+		  , "=c" (tsc_cpu)
+	); // : eax, edx, ecx used - NOT rax, rdx, rcx
+	if (unlikely(cpu_out != ((void *)0)))
+		*cpu_out = tsc_cpu;
+	return ((((u64)tsc_hi) << 32) |
+		(((u64)tsc_lo) & 0x0ffffffffULL)
+	       );
+}
+
 /* Deprecated, keep it for a cycle for easier merging: */
 #define rdtscll(now)	do { (now) = rdtsc_ordered(); } while (0)
 
diff --git a/arch/x86/include/asm/vgtod.h b/arch/x86/include/asm/vgtod.h
index 24e4d45..e7e4804 100644
--- a/arch/x86/include/asm/vgtod.h
+++ b/arch/x86/include/asm/vgtod.h
@@ -26,6 +26,7 @@ struct vsyscall_gtod_data {
 	u64	raw_mask;
 	u32	raw_mult;
 	u32	raw_shift;
+	u32     has_rdtscp;
 
 	/* open coded 'struct timespec' */
 	u64		wall_time_snsec;

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4.16-rc5 3/3] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 16:00 [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW jason.vas.dias
  2018-03-15 16:00 ` [PATCH v4.16-rc5 1/3] " jason.vas.dias
  2018-03-15 16:00 ` [PATCH v4.16-rc5 2/3] " jason.vas.dias
@ 2018-03-15 16:00 ` jason.vas.dias
  2018-03-17  7:06   ` kbuild test robot
  2018-03-17  7:27   ` kbuild test robot
  2018-03-15 20:17 ` [PATCH v4.16-rc5 (3)] " Thomas Gleixner
  3 siblings, 2 replies; 11+ messages in thread
From: jason.vas.dias @ 2018-03-15 16:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: x86, tglx, mingo, peterz, andi

diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
index 03f3904..61d9633 100644
--- a/arch/x86/entry/vdso/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vclock_gettime.c
@@ -21,12 +21,15 @@
 #include <linux/math64.h>
 #include <linux/time.h>
 #include <linux/kernel.h>
+#include <uapi/asm/vdso_tsc_calibration.h>
 
 #define gtod (&VVAR(vsyscall_gtod_data))
 
 extern int __vdso_clock_gettime(clockid_t clock, struct timespec *ts);
 extern int __vdso_gettimeofday(struct timeval *tv, struct timezone *tz);
 extern time_t __vdso_time(time_t *t);
+extern unsigned int __vdso_tsc_calibration(
+	struct linux_tsc_calibration_s *tsc_cal);
 
 #ifdef CONFIG_PARAVIRT_CLOCK
 extern u8 pvclock_page
@@ -383,3 +386,25 @@ notrace time_t __vdso_time(time_t *t)
 }
 time_t time(time_t *t)
 	__attribute__((weak, alias("__vdso_time")));
+
+notrace	unsigned int
+__vdso_linux_tsc_calibration(struct linux_tsc_calibration_s *tsc_cal)
+{
+	unsigned long seq;
+
+	do {
+		seq = gtod_read_begin(gtod);
+		if ((gtod->vclock_mode == VCLOCK_TSC) &&
+		    (tsc_cal != ((void *)0UL))) {
+			tsc_cal->tsc_khz = gtod->tsc_khz;
+			tsc_cal->mult    = gtod->raw_mult;
+			tsc_cal->shift   = gtod->raw_shift;
+			return 1;
+		}
+	} while (unlikely(gtod_read_retry(gtod, seq)));
+
+	return 0;
+}
+
+unsigned int linux_tsc_calibration(struct linux_tsc_calibration_s *tsc_cal)
+	__attribute((weak, alias("__vdso_linux_tsc_calibration")));
diff --git a/arch/x86/entry/vdso/vdso.lds.S b/arch/x86/entry/vdso/vdso.lds.S
index d3a2dce..e0b5cce 100644
--- a/arch/x86/entry/vdso/vdso.lds.S
+++ b/arch/x86/entry/vdso/vdso.lds.S
@@ -25,6 +25,8 @@ VERSION {
 		__vdso_getcpu;
 		time;
 		__vdso_time;
+		linux_tsc_calibration;
+		__vdso_linux_tsc_calibration;
 	local: *;
 	};
 }
diff --git a/arch/x86/entry/vdso/vdso32/vdso32.lds.S b/arch/x86/entry/vdso/vdso32/vdso32.lds.S
index 422764a..17fd07f 100644
--- a/arch/x86/entry/vdso/vdso32/vdso32.lds.S
+++ b/arch/x86/entry/vdso/vdso32/vdso32.lds.S
@@ -26,6 +26,7 @@ VERSION
 		__vdso_clock_gettime;
 		__vdso_gettimeofday;
 		__vdso_time;
+		__vdso_linux_tsc_calibration;
 	};
 
 	LINUX_2.5 {
diff --git a/arch/x86/entry/vdso/vdsox32.lds.S b/arch/x86/entry/vdso/vdsox32.lds.S
index 05cd1c5..7acac71 100644
--- a/arch/x86/entry/vdso/vdsox32.lds.S
+++ b/arch/x86/entry/vdso/vdsox32.lds.S
@@ -21,6 +21,7 @@ VERSION {
 		__vdso_gettimeofday;
 		__vdso_getcpu;
 		__vdso_time;
+		__vdso_linux_tsc_calibration;
 	local: *;
 	};
 }
diff --git a/arch/x86/include/uapi/asm/vdso_tsc_calibration.h b/arch/x86/include/uapi/asm/vdso_tsc_calibration.h
new file mode 100644
index 0000000..ce4b5a45
--- /dev/null
+++ b/arch/x86/include/uapi/asm/vdso_tsc_calibration.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _ASM_X86_VDSO_TSC_CALIBRATION_H
+#define _ASM_X86_VDSO_TSC_CALIBRATION_H
+/*
+ * Programs that want to use rdtsc / rdtscp instructions
+ * from user-space can make use of the Linux kernel TSC calibration
+ * by calling :
+ *    __vdso_linux_tsc_calibration(struct linux_tsc_calibration_s *);
+ * ( one has to resolve this symbol as in
+ *   tools/testing/selftests/vDSO/parse_vdso.c
+ * )
+ * which fills in a structure
+ * with the following layout :
+ */
+
+/** struct linux_tsc_calibration_s -
+ * mult:    amount to multiply 64-bit TSC value by
+ * shift:   the right shift to apply to (mult*TSC) yielding nanoseconds
+ * tsc_khz: the calibrated TSC frequency in KHz from which previous
+ *          members calculated
+ */
+struct linux_tsc_calibration_s {
+
+	unsigned int mult;
+	unsigned int shift;
+	unsigned int tsc_khz;
+
+};
+
+/* To use:
+ *
+ *  static unsigned
+ *  (*linux_tsc_cal)(struct linux_tsc_calibration_s *linux_tsc_cal) =
+ *    vdso_sym("LINUX_2.6", "__vdso_linux_tsc_calibration");
+ *  if(linux_tsc_cal == ((void *)0))
+ *  { fprintf(stderr,"the patch providing __vdso_linux_tsc_calibration"
+ *                   " is not applied to the kernel.\n");
+ *    return ERROR;
+ *  }
+ *  static struct linux_tsc_calibration clock_source={0};
+ *  if((clock_source.mult==0) && ! (*linux_tsc_cal)(&clock_source) )
+ *    fprintf(stderr,"TSC is not the system clocksource.\n");
+ *  unsigned int tsc_lo, tsc_hi, tsc_cpu;
+ *  asm volatile
+ *  ( "rdtscp" : (=a) tsc_hi,  (=d) tsc_lo, (=c) tsc_cpu );
+ *  unsigned long tsc = (((unsigned long)tsc_hi) << 32) | tsc_lo;
+ *  unsigned long nanoseconds =
+ *   (( clock_source . mult ) * tsc ) >> (clock_source . shift);
+ *
+ *  nanoseconds is now TSC value converted to nanoseconds,
+ *  according to Linux' clocksource calibration values.
+ *  Incidentally, 'tsc_cpu' is the number of the CPU the task is running on.
+ *
+ * But better results are obtained by applying this to the difference (delta)
+ * and adding this to some previous timespec value:
+ *   static u64 previous_tsc=0, previous_nsec=0, previous_sec=0;
+ *   u64  tsc      = rdtscp();
+ *   u64  delta    = tsc - previous_tsc;
+ *   u64  nsec     = ((delta * clock_source.mult) + previous_nsec )
+ *	           >> clock_source.shift;
+ *   ts->tv_sec    = previous_sec + (nsec / NSEC_PER_SEC);
+ *   ts->tv_nsec   = nsec % NSEC_PER_SEC;
+ *   previous_tsc  = tsc
+ *   previous_sec  = ts->tv_sec;
+ *   previous_nsec = ts->tv_nsec << clock_source.shift;
+ *   return ts;
+ * This is broadly like the approach taken by Linux kernel & in VDSO .
+ *
+ * Or, in user-space, with floating point, one could use the rdtscp value as
+ * number of picoseconds :
+ *     u64 ns = lround(   ((double)rdtscp())
+ *                     / (((double)clock_source.tsc_khz) / 1e3)
+ *                    );
+ * (ie. if tsc_khz is 3000 , there are 3 tsc ticks per nanosecond,
+ *  so divide tsc ticks by 3).
+ *
+ * There should actually be very little difference between the two
+ * values obtained (@ 0.02% ) by either method.
+ */
+
+#endif

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 16:00 [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW jason.vas.dias
                   ` (2 preceding siblings ...)
  2018-03-15 16:00 ` [PATCH v4.16-rc5 3/3] " jason.vas.dias
@ 2018-03-15 20:17 ` Thomas Gleixner
  2018-03-15 21:41   ` Jason Vas Dias
  3 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2018-03-15 20:17 UTC (permalink / raw)
  To: jason.vas.dias; +Cc: linux-kernel, x86, mingo, peterz, andi

Jason,

On Thu, 15 Mar 2018, jason.vas.dias@gmail.com wrote:

>   Resent to address reviewer comments.

I was being patient so far and tried to guide you through the patch
submission process, but unfortunately this turns out to be just waste of my
time.

You have not addressed any of the comments I made here:

[1]  https://lkml.kernel.org/r/alpine.DEB.2.21.1803141511340.2481@nanos.tec.linutronix.de
[2]  https://lkml.kernel.org/r/alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de

But still you claim that you addressed reviewer comments.

The delta between the two patch series shows clearly that you ignored both
review mails completely. You merily changed the implementation of a
function, which I not even reviewed, and fiddled in the big fat comment.

Feel free to ignore me, but rest assured that this is the last chance
before you end up in the /dev/null mail filter.

I recommend you to try to fixup only the first patch and once that is good
to go you might try to submit the rest. But please take your time and
address _ALL_ of my comments and follow the documented rules.

If you have questions regarding my comments or the process after working
through them and the Documentation I pointed to, feel free to ask them on
the list and not in private mail. If you think that my comments or
recommendations are wrong, discuss them on list instead of silently
ignoring them.

Yours grumpy

      tglx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 20:17 ` [PATCH v4.16-rc5 (3)] " Thomas Gleixner
@ 2018-03-15 21:41   ` Jason Vas Dias
  2018-03-15 22:41     ` Thomas Gleixner
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Vas Dias @ 2018-03-15 21:41 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel, x86, mingo, peterz, andi

[-- Attachment #1: Type: text/plain, Size: 2942 bytes --]

Hi Thomas -
RE:
On 15/03/2018, Thomas Gleixner <tglx@linutronix.de> wrote:
> Jason,
>
> On Thu, 15 Mar 2018, jason.vas.dias@gmail.com wrote:
>
>>   Resent to address reviewer comments.
>
> I was being patient so far and tried to guide you through the patch
> submission process, but unfortunately this turns out to be just waste of my
> time.
>
> You have not addressed any of the comments I made here:
>
> [1]
> https://lkml.kernel.org/r/alpine.DEB.2.21.1803141511340.2481@nanos.tec.linutronix.de
> [2]
> https://lkml.kernel.org/r/alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de
>

I'm really sorry about that - I did not see those mails ,
and have searched for them in my inbox -
are you sure they were sent to 'linux-kernel@vger.kernel.org' ?
That is the only list I am subscribed to .
I clicked on the links , but the 'To:' field is just
'linux-kernel' .

If I had seen those messages before I re-submitted,
those issues would have been fixed.

checkpatch.pl did not report them -
I ran it with all patches and it reported
no errors .

And I did send the test results in a previous mail -

$ gcc -m64 -o timer timer.c

( must be compiled in 64-bit mode).

This is using the new rdtscp() function :
$ ./timer -r 100
...
Total time: 0.000002806S - Average Latency: 0.000000028S N zero
deltas: 0 N inconsistent deltas: 0
Average of 100 average latencies of 100 samples : 0.000000027S

This is using the rdtsc_ordered() function:

$ ./timer -m -r 100
Total time: 0.000005269S - Average Latency: 0.000000052S N zero
deltas: 0 N inconsistent deltas: 0
Average of 100 average latencies of 100 samples : 0.000000047S

timer.c is a very short program that just reads N_SAMPLES (a
compile-time option)
timespecs using either CLOCK_MONOTONIC_RAW (no -m) or CLOCK_MONOTONIC
first parameter to clock_gettime(),  then
computes the deltas as long long, then averages them , counting any
zero deltas, or deltas where the previous timespec is somehow
greater than the current timespec, which are reported as
inconsitencies (note 'inconistent deltas:0' and 'zero deltas: 0' in output).

So my initial claim that rdtscp() can be twice as fast as rdtsc_ordered()
was not far-fetched - this is what I am seeing .

I think this is because of the explicit barrier() call in rdtsc_ordered() .
This must be slower than than the internal processor pipeline
"cancellation point" (barrier) used by the rdtscp instruction itself.
This is the only reason for the rdtscp call  -  plus all modern Intel
& AMD CPUs support it, and it DOES solve the ordering problem,
whereby instructions in one pipeline of a task can get different
rdtsc() results than instructions in another pipeline.

I will document the results better in the ChangeLog , fix all issues
you identified, and resend .

I did not mean to ignore your comments -
those mails are nowhere in my Inbox -
please ,  confirm the actual email address
they are getting sent to.

Thanks & Regards,
Jason

[-- Attachment #2: timer.c --]
[-- Type: text/x-csrc, Size: 3601 bytes --]

/* 
 * Program to measure high-res timer latency.
 *
 */
#include <stdint.h>
#include <stdbool.h>
#include <sys/types.h>
#include <unistd.h>
#include <time.h>
#include <errno.h>
#include <alloca.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

#ifndef N_SAMPLES
#define N_SAMPLES 100
#endif
#define _STR(_S_) #_S_
#define STR(_S_) _STR(_S_)

#define TS2NS(_TS_) ((((unsigned long long)(_TS_).tv_sec)*1000000000ULL) + (((unsigned long long)((_TS_).tv_nsec)))) 

int main(int argc, char *const* argv, char *const* envp)
{ struct timespec sample[N_SAMPLES+1];
  unsigned int cnt=N_SAMPLES, s=0 , avg_n=0;
  unsigned long long
    deltas [ N_SAMPLES ]
    , t1, t2, sum=0, zd=0, ic=0, d
    , t_start, avg_ns, *avgs=0;
  clockid_t clk = CLOCK_MONOTONIC_RAW;
  bool do_dump = false;
  int argn=1, repeat=1;
  for(; argn < argc; argn+=1)
    if( argv[argn] != NULL )
      if( *(argv[argn]) == '-')
	switch( *(argv[argn]+1) )
	{ case 'm':
	  case 'M':
	    clk = CLOCK_MONOTONIC;
	    break;
	  case 'd':
	  case 'D':
	    do_dump = true;
	    break;
     	  case 'r':
      	  case 'R':
	    if( (argn < argc) && (argv[argn+1] != NULL))
	      repeat = atoi(argv[argn+=1]);
	    break;
	  case '?':
	  case 'h':
	  case 'u':
	  case 'U':
	  case 'H':
	    fprintf(stderr,"Usage: timer_latency [\n\t-m : use CLOCK_MONOTONIC clock (not CLOCK_MONOTONIC_RAW)\n\t-d : dump timespec contents. N_SAMPLES: " STR(N_SAMPLES) "\n\t"
	            "-r <repeat count>\n]\t" 
	            "Calculates average timer latency (minimum time that can be measured) over N_SAMPLES.\n"
	           );
	    return 0;
	}
  if( repeat > 1 )
  { avgs=alloca(sizeof(unsigned long long) * (N_SAMPLES + 1));
    if( ((unsigned long) avgs) & 7 )
      avgs = ((unsigned long long*)(((unsigned char*)avgs)+(8-((unsigned long) avgs) & 7)));
  }
  do {
    cnt=N_SAMPLES;
    s=0;
  do
  { if( 0 != clock_gettime(clk, &sample[s++]) )
    { fprintf(stderr,"oops, clock_gettime() failed: %d: '%s'.\n", errno, strerror(errno));
      return 1;
    }
  }while( --cnt );
  clock_gettime(clk, &sample[s]);
  for(s=1; s < (N_SAMPLES+1); s+=1)
  { t1 = TS2NS(sample[s-1]);
    t2 = TS2NS(sample[s]);
    if ( (t1 > t2)
       ||(sample[s-1].tv_sec > sample[s].tv_sec)
       ||((sample[s-1].tv_sec == sample[s].tv_sec)
        &&(sample[s-1].tv_nsec > sample[s].tv_nsec)
         )
       )
    { fprintf(stderr,"Inconsistency: %llu %llu %lu.%lu %lu.%lu\n", t1 , t2
            , sample[s-1].tv_sec, sample[s-1].tv_nsec
            , sample[s].tv_sec,   sample[s].tv_nsec
      );
      ic+=1;
      continue;
    }
    d = t2 - t1;
    if ( d == 0 )
    {  zd += 1;
    }
    deltas[s-1] = d;
    if(do_dump)
      fprintf(stderr, "%lu %lu %llu\n",
              sample[s].tv_sec, sample[s].tv_nsec, d
             );
  }
  for(s = 0, sum=0; s < N_SAMPLES; s+=1)
    sum += deltas[s];
  fprintf(stderr,"sum: %llu\n",sum);
  avg_ns = sum / N_SAMPLES;
  t_start = TS2NS(sample[0]);
  t1=(t2 - t_start);
  printf("Total time: %1.1llu.%9.9lluS - Average Latency: %1.1llu.%9.9lluS N zero deltas: %u N inconsistent deltas: %u\n",
          t1 / 1000000000,       t1 % 1000000000,
          avg_ns / 1000000000,   avg_ns % 1000000000 ,
          zd, ic
        );
  if (avgs != ((void*)0UL))
    avgs[avg_n++] = avg_ns;
  } while (--repeat);
  if (avgs != ((void*)0UL))
  { for( s=0, sum=0; s < avg_n; s+=1)
      sum += avgs[s];
    printf("Average of %u average latencies of " STR(N_SAMPLES) " samples : %1.1llu.%9.9lluS\n"
          , avg_n, (sum / N_SAMPLES) / 1000000000, (sum / N_SAMPLES) % 1000000000
          );
  }
  return 0;
}


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 21:41   ` Jason Vas Dias
@ 2018-03-15 22:41     ` Thomas Gleixner
  2018-03-16 13:30       ` Jason Vas Dias
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2018-03-15 22:41 UTC (permalink / raw)
  To: Jason Vas Dias; +Cc: linux-kernel, x86, mingo, peterz, andi

On Thu, 15 Mar 2018, Jason Vas Dias wrote:
> On 15/03/2018, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Thu, 15 Mar 2018, jason.vas.dias@gmail.com wrote:
> >
> >>   Resent to address reviewer comments.
> >
> > I was being patient so far and tried to guide you through the patch
> > submission process, but unfortunately this turns out to be just waste of my
> > time.
> >
> > You have not addressed any of the comments I made here:
> >
> > [1]
> > https://lkml.kernel.org/r/alpine.DEB.2.21.1803141511340.2481@nanos.tec.linutronix.de
> > [2]
> > https://lkml.kernel.org/r/alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de
> >
> 
> I'm really sorry about that - I did not see those mails ,
> and have searched for them in my inbox -

That's close to the 'my dog ate the homework' excuse.

Of course they were sent to the list and to you personally as I used
reply-all. From the mail server log:

2018-03-14 15:27:27 1ew7NH-00039q-Hv <= tglx@linutronix.de id=alpine.DEB.2.21.1803141511340.2481@nanos.tec.linutronix.de

2018-03-14 15:27:30 1ew7NH-00039q-Hv => jason.vas.dias@gmail.com R=dnslookup T=remote_smtp H=gmail-smtp-in.l.google.com [2a00:1450:4013:c01::1a] X=TLS1.2:RSA_AES_128_CBC_SHA1:128 DN="C=US,ST=California,L=Mountain View,O=Google Inc,CN=mx.google.com"

2018-03-14 15:27:31 1ew7NH-00039q-Hv => linux-kernel@vger.kernel.org R=dnslookup T=remote_smtp H=vger.kernel.org [209.132.180.67]

<SNIP other recipients on CC list>

2018-03-14 15:27:47 1ew7NH-00039q-Hv Completed

If those messages would not have been delivered to
linux-kernel@vger.kernel.org they would hardly be on the mailing list
archive, right?

And they both got delivered to your gmail account as well.

> are you sure they were sent to 'linux-kernel@vger.kernel.org' ?
> That is the only list I am subscribed to .
> I clicked on the links , but the 'To:' field is just
> 'linux-kernel' .

There is no 'To' field on that page.

  [prev in list] [next in list] [prev in thread] [next in thread] 

  List:       linux-kernel

List != To.

As any other sane archive it strips the To and Cc fields for obvious
reasons.

> If I had seen those messages before I re-submitted,
> those issues would have been fixed.
> 
> checkpatch.pl did not report them -
> I ran it with all patches and it reported
> no errors .

patch 1/3:

ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 119 lines checked

patch 2/3:

ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 71 lines checked

patch 3/3:

WARNING: externs should be avoided in .c files
#24: FILE: arch/x86/entry/vdso/vclock_gettime.c:31:
+extern unsigned int __vdso_tsc_calibration(

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#93: 
new file mode 100644

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 2 warnings, 143 lines checked

It reports an error for every single patch of your latest submission.

> And I did send the test results in a previous mail -

In private mail which I ignore if there is no real good reason. And just
for the record. This private mail contains the following headers:

In-Reply-To: <alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de>
References: <1521001222-10712-1-git-send-email-jason.vas.dias@gmail.com>
 <1521001222-10712-3-git-send-email-jason.vas.dias@gmail.com>
    <alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de>
From: Jason Vas Dias <jason.vas.dias@gmail.com>
Date: Wed, 14 Mar 2018 15:08:55 +0000
Message-ID: <CALyZvKwB667X-ADq4PE8p7_oC2-gdJWQcw4ch4NAadmw9ZoSmw@mail.gmail.com>
Subject: Re: [PATCH v4.16-rc5 2/3] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW

So now, if you take the message ID which is in the In-Reply-To: field and
compare it to the message ID which I used for link [2]:

In-Reply-To: <alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de>
> > https://lkml.kernel.org/r/alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de

you might notice that these are identical. So how did you end up replying
to a mail which you never received?

Nice try. I'm really fed up with this.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 22:41     ` Thomas Gleixner
@ 2018-03-16 13:30       ` Jason Vas Dias
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Vas Dias @ 2018-03-16 13:30 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel, x86, mingo, peterz, andi

Good day -

RE:
On 15/03/2018, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 15 Mar 2018, Jason Vas Dias wrote:
>> On 15/03/2018, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Thu, 15 Mar 2018, jason.vas.dias@gmail.com wrote:
>> >
>> >>   Resent to address reviewer comments.
>> >
>> > I was being patient so far and tried to guide you through the patch
>> > submission process, but unfortunately this turns out to be just waste of
>> > my
>> > time.
>> >
>> > You have not addressed any of the comments I made here:
>> >
>> > [1]
>> > https://lkml.kernel.org/r/alpine.DEB.2.21.1803141511340.2481@nanos.tec.linutronix.de
>> > [2]
>> > https://lkml.kernel.org/r/alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de
>> >
>>
>> I'm really sorry about that - I did not see those mails ,
>> and have searched for them in my inbox -
>
> That's close to the 'my dog ate the homework' excuse.
>


Nevertheless, those messages are NOT in my inbox, nor
can I find them on the list - a google search for
'alpine.DEB.2.21.1803141511340.2481' or
'alpine.DEB.2.21.1803141527300.2481' returns
only the last two mails on the subject , where
you included the links to https://lkml.kernel.org.

I don't know what went wrong here, but I did not
receive those mails until you informed me of them
yesterday evening, when I immediately regenerated
the Patch #1 incorporating fixes for your comments,
and sent it with Subject:
  '[PATCH v4.16-rc5 1/1] x86/vdso: VDSO should handle\
   clock_gettime(CLOCK_MONOTONIC_RAW) without syscall
  '
This version re-uses the 'gtod->cycles' value, which as you point
out, is the same as 'tk->tkr_raw.cycle_last'  -
so I removed vread_tsc_raw() .


> Of course they were sent to the list and to you personally as I used
> reply-all. From the mail server log:
>
> 2018-03-14 15:27:27 1ew7NH-00039q-Hv <= tglx@linutronix.de
> id=alpine.DEB.2.21.1803141511340.2481@nanos.tec.linutronix.de
>
> 2018-03-14 15:27:30 1ew7NH-00039q-Hv => jason.vas.dias@gmail.com R=dnslookup
> T=remote_smtp H=gmail-smtp-in.l.google.com [2a00:1450:4013:c01::1a]
> X=TLS1.2:RSA_AES_128_CBC_SHA1:128 DN="C=US,ST=California,L=Mountain
> View,O=Google Inc,CN=mx.google.com"
>
> 2018-03-14 15:27:31 1ew7NH-00039q-Hv => linux-kernel@vger.kernel.org
> R=dnslookup T=remote_smtp H=vger.kernel.org [209.132.180.67]
>
> <SNIP other recipients on CC list>
>
> 2018-03-14 15:27:47 1ew7NH-00039q-Hv Completed
>
> If those messages would not have been delivered to
> linux-kernel@vger.kernel.org they would hardly be on the mailing list
> archive, right?
>

Yes, I cannot explain why I did not receive them .

I guess I should consider gmail an unreliable delivery
method and use the lkml.org web interface to check
for replies - I will do this from now one.

> And they both got delivered to your gmail account as well.
>

No, they are not in my gmail account Inbox or folders.


> ERROR: Missing Signed-off-by: line(s)
> total: 1 errors, 0 warnings, 71 lines checked
>

I do not know how to fix this error - I was hoping
someone on the list might enlighten me.

>
> WARNING: externs should be avoided in .c files
> #24: FILE: arch/x86/entry/vdso/vclock_gettime.c:31:
> +extern unsigned int __vdso_tsc_calibration(
>

I thought that must be a script bug, since no extern
is being declared by that line; it is an external function
declaration, just like the unmodified line that precedes it.


> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #93:
> new file mode 100644
>
> ERROR: Missing Signed-off-by: line(s)
>
> total: 1 errors, 2 warnings, 143 lines checked
>
> It reports an error for every single patch of your latest submission.
>
>> And I did send the test results in a previous mail -
>
> In private mail which I ignore if there is no real good reason. And just
> for the record. This private mail contains the following headers:
>
> In-Reply-To: <alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de>
> References: <1521001222-10712-1-git-send-email-jason.vas.dias@gmail.com>
>  <1521001222-10712-3-git-send-email-jason.vas.dias@gmail.com>
>     <alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de>
> From: Jason Vas Dias <jason.vas.dias@gmail.com>
> Date: Wed, 14 Mar 2018 15:08:55 +0000
> Message-ID:
> <CALyZvKwB667X-ADq4PE8p7_oC2-gdJWQcw4ch4NAadmw9ZoSmw@mail.gmail.com>
> Subject: Re: [PATCH v4.16-rc5 2/3] x86/vdso: on Intel, VDSO should handle
> CLOCK_MONOTONIC_RAW
>
> So now, if you take the message ID which is in the In-Reply-To: field and
> compare it to the message ID which I used for link [2]:
>
> In-Reply-To: <alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de>
>> > https://lkml.kernel.org/r/alpine.DEB.2.21.1803141527300.2481@nanos.tec.linutronix.de
>
> you might notice that these are identical. So how did you end up replying
> to a mail which you never received?
>
> Nice try. I'm really fed up with this.
>

The only thing I'm trying to do is fix the implementation
of clock_gettime(CLOCK_MONOTONIC_RAW) .

I was replying to the message you sent , which contained a
reference header for the messages you sent but which I did
not receive . I am sorry this happened, but it is beyond my
control.

As soon as I can, I am going to rent a host in a data-center
and set up my own email server. If this experience has taught
me anything,  it  is that gmail is not of sufficient quality for
reliable transmission of email - I am sorry I am stuck with it
for now.

I will check the Web-based LKML archives instead of my
Gmail inbox for your replies in future - sorry !

But what of the patch I sent last night, which does
address all of your concerns and passes all checkpatch.pl tests?

$ scripts/checkpatch.pl
/tmp/vdso_vclock_gettime_CLOCK_MONOTONIC_RAW_4.16-rc5_basic.patch
total: 0 errors, 0 warnings, 132 lines checked

/tmp/vdso_vclock_gettime_CLOCK_MONOTONIC_RAW_4.16-rc5_basic.patch has
no obvious style problems and is ready for submission.

Once this is submitted, maybe we can work on improving the performance of
both clock_gettime implementations in the vDSO by use of rdtscp , and
exporting some clock calibration to user-space.

Thanks & Best Regards,
Jason

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4.16-rc5 3/3] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 16:00 ` [PATCH v4.16-rc5 3/3] " jason.vas.dias
@ 2018-03-17  7:06   ` kbuild test robot
  2018-03-17  7:27   ` kbuild test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-03-17  7:06 UTC (permalink / raw)
  To: jason.vas.dias; +Cc: kbuild-all, linux-kernel, x86, tglx, mingo, peterz, andi

[-- Attachment #1: Type: text/plain, Size: 3627 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/jason-vas-dias-gmail-com/x86-vdso-on-Intel-VDSO-should-handle-CLOCK_MONOTONIC_RAW/20180317-143702
config: x86_64-randconfig-x010-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86/entry/vdso/vclock_gettime.c: In function '__vdso_linux_tsc_calibration':
>> arch/x86/entry/vdso/vclock_gettime.c:399:27: error: 'struct vsyscall_gtod_data' has no member named 'tsc_khz'
       tsc_cal->tsc_khz = gtod->tsc_khz;
                              ^~

vim +399 arch/x86/entry/vdso/vclock_gettime.c

   324	
   325	notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
   326	{
   327		switch (clock) {
   328		case CLOCK_REALTIME:
   329			if (do_realtime(ts) == VCLOCK_NONE)
   330				goto fallback;
   331			break;
   332		case CLOCK_MONOTONIC:
   333			if (do_monotonic(ts) == VCLOCK_NONE)
   334				goto fallback;
   335			break;
   336		case CLOCK_MONOTONIC_RAW:
   337			if (do_monotonic_raw(ts) == VCLOCK_NONE)
   338				goto fallback;
   339			break;
   340		case CLOCK_REALTIME_COARSE:
   341			do_realtime_coarse(ts);
   342			break;
   343		case CLOCK_MONOTONIC_COARSE:
   344			do_monotonic_coarse(ts);
   345			break;
   346		default:
   347			goto fallback;
   348		}
   349	
   350		return 0;
   351	fallback:
   352		return vdso_fallback_gettime(clock, ts);
   353	}
   354	int clock_gettime(clockid_t, struct timespec *)
   355		__attribute__((weak, alias("__vdso_clock_gettime")));
   356	
   357	notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone *tz)
   358	{
   359		if (likely(tv != NULL)) {
   360			if (unlikely(do_realtime((struct timespec *)tv) == VCLOCK_NONE))
   361				return vdso_fallback_gtod(tv, tz);
   362			tv->tv_usec /= 1000;
   363		}
   364		if (unlikely(tz != NULL)) {
   365			tz->tz_minuteswest = gtod->tz_minuteswest;
   366			tz->tz_dsttime = gtod->tz_dsttime;
   367		}
   368	
   369		return 0;
   370	}
   371	int gettimeofday(struct timeval *, struct timezone *)
   372		__attribute__((weak, alias("__vdso_gettimeofday")));
   373	
   374	/*
   375	 * This will break when the xtime seconds get inaccurate, but that is
   376	 * unlikely
   377	 */
   378	notrace time_t __vdso_time(time_t *t)
   379	{
   380		/* This is atomic on x86 so we don't need any locks. */
   381		time_t result = READ_ONCE(gtod->wall_time_sec);
   382	
   383		if (t)
   384			*t = result;
   385		return result;
   386	}
   387	time_t time(time_t *t)
   388		__attribute__((weak, alias("__vdso_time")));
   389	
   390	notrace	unsigned int
   391	__vdso_linux_tsc_calibration(struct linux_tsc_calibration_s *tsc_cal)
   392	{
   393		unsigned long seq;
   394	
   395		do {
   396			seq = gtod_read_begin(gtod);
   397			if ((gtod->vclock_mode == VCLOCK_TSC) &&
   398			    (tsc_cal != ((void *)0UL))) {
 > 399				tsc_cal->tsc_khz = gtod->tsc_khz;
   400				tsc_cal->mult    = gtod->raw_mult;
   401				tsc_cal->shift   = gtod->raw_shift;
   402				return 1;
   403			}
   404		} while (unlikely(gtod_read_retry(gtod, seq)));
   405	
   406		return 0;
   407	}
   408	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26111 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4.16-rc5 3/3] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
  2018-03-15 16:00 ` [PATCH v4.16-rc5 3/3] " jason.vas.dias
  2018-03-17  7:06   ` kbuild test robot
@ 2018-03-17  7:27   ` kbuild test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-03-17  7:27 UTC (permalink / raw)
  To: jason.vas.dias; +Cc: kbuild-all, linux-kernel, x86, tglx, mingo, peterz, andi

[-- Attachment #1: Type: text/plain, Size: 3728 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/jason-vas-dias-gmail-com/x86-vdso-on-Intel-VDSO-should-handle-CLOCK_MONOTONIC_RAW/20180317-143702
config: i386-randconfig-x013-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from arch/x86/entry/vdso/vdso32/vclock_gettime.c:33:0:
   arch/x86/entry/vdso/vdso32/../vclock_gettime.c: In function '__vdso_linux_tsc_calibration':
>> arch/x86/entry/vdso/vdso32/../vclock_gettime.c:399:27: error: 'struct vsyscall_gtod_data' has no member named 'tsc_khz'
       tsc_cal->tsc_khz = gtod->tsc_khz;
                              ^~

vim +399 arch/x86/entry/vdso/vdso32/../vclock_gettime.c

   324	
   325	notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
   326	{
   327		switch (clock) {
   328		case CLOCK_REALTIME:
   329			if (do_realtime(ts) == VCLOCK_NONE)
   330				goto fallback;
   331			break;
   332		case CLOCK_MONOTONIC:
   333			if (do_monotonic(ts) == VCLOCK_NONE)
   334				goto fallback;
   335			break;
   336		case CLOCK_MONOTONIC_RAW:
   337			if (do_monotonic_raw(ts) == VCLOCK_NONE)
   338				goto fallback;
   339			break;
   340		case CLOCK_REALTIME_COARSE:
   341			do_realtime_coarse(ts);
   342			break;
   343		case CLOCK_MONOTONIC_COARSE:
   344			do_monotonic_coarse(ts);
   345			break;
   346		default:
   347			goto fallback;
   348		}
   349	
   350		return 0;
   351	fallback:
   352		return vdso_fallback_gettime(clock, ts);
   353	}
   354	int clock_gettime(clockid_t, struct timespec *)
   355		__attribute__((weak, alias("__vdso_clock_gettime")));
   356	
   357	notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone *tz)
   358	{
   359		if (likely(tv != NULL)) {
   360			if (unlikely(do_realtime((struct timespec *)tv) == VCLOCK_NONE))
   361				return vdso_fallback_gtod(tv, tz);
   362			tv->tv_usec /= 1000;
   363		}
   364		if (unlikely(tz != NULL)) {
   365			tz->tz_minuteswest = gtod->tz_minuteswest;
   366			tz->tz_dsttime = gtod->tz_dsttime;
   367		}
   368	
   369		return 0;
   370	}
   371	int gettimeofday(struct timeval *, struct timezone *)
   372		__attribute__((weak, alias("__vdso_gettimeofday")));
   373	
   374	/*
   375	 * This will break when the xtime seconds get inaccurate, but that is
   376	 * unlikely
   377	 */
   378	notrace time_t __vdso_time(time_t *t)
   379	{
   380		/* This is atomic on x86 so we don't need any locks. */
   381		time_t result = READ_ONCE(gtod->wall_time_sec);
   382	
   383		if (t)
   384			*t = result;
   385		return result;
   386	}
   387	time_t time(time_t *t)
   388		__attribute__((weak, alias("__vdso_time")));
   389	
   390	notrace	unsigned int
   391	__vdso_linux_tsc_calibration(struct linux_tsc_calibration_s *tsc_cal)
   392	{
   393		unsigned long seq;
   394	
   395		do {
   396			seq = gtod_read_begin(gtod);
   397			if ((gtod->vclock_mode == VCLOCK_TSC) &&
   398			    (tsc_cal != ((void *)0UL))) {
 > 399				tsc_cal->tsc_khz = gtod->tsc_khz;
   400				tsc_cal->mult    = gtod->raw_mult;
   401				tsc_cal->shift   = gtod->raw_shift;
   402				return 1;
   403			}
   404		} while (unlikely(gtod_read_retry(gtod, seq)));
   405	
   406		return 0;
   407	}
   408	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37068 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW
@ 2018-03-14  4:20 jason.vas.dias
  0 siblings, 0 replies; 11+ messages in thread
From: jason.vas.dias @ 2018-03-14  4:20 UTC (permalink / raw)
  To: linux-kernel; +Cc: x86, tglx, mingo, peterz, andi



  Currently the VDSO does not handle
     clock_gettime( CLOCK_MONOTONIC_RAW, &ts )
  on Intel / AMD - it calls
     vdso_fallback_gettime()
  for this clock, which issues a syscall, having an unacceptably high
  latency (minimum measurable time or time between measurements)
  of 300-700ns on 2 2.8-3.9ghz Haswell x86_64 Family'_'Model : 06_3C
  machines under various versions of Linux.

  Sometimes, particularly when correlating elapsed time to performance
  counter values, user-space  code needs to know elapsed time from the
  perspective of the CPU no matter how "hot" / fast or "cold" / slow it
  might be running wrt NTP / PTP "real" time; when code needs this,
  the latencies associated with a syscall are often unacceptably high.

  I reported this as Bug #198161 :
    'https://bugzilla.kernel.org/show_bug.cgi?id=198961'
  and in previous posts with subjects matching 'CLOCK_MONOTONIC_RAW' .
     
  This patch handles CLOCK_MONOTONIC_RAW clock_gettime() in the VDSO ,
  by exporting the raw clock calibration, last cycles, last xtime_nsec,
  and last raw_sec value in the vsyscall_gtod_data during vsyscall_update() .

  Now the new do_monotonic_raw() function in the vDSO has a latency of @ 24ns
  on average, and the test program:
   tools/testing/selftest/timers/inconsistency-check.c
  succeeds with arguments: '-c 4 -t 120' or any arbitrary -t value.

  The patch is against Linus' latest 4.16-rc5 tree,
  current HEAD of :
    git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
  .

  This patch affects only files:
  
   arch/x86/include/asm/vgtod.h
   arch/x86/entry/vdso/vclock_gettime.c
   arch/x86/entry/vdso/vdso.lds.S
   arch/x86/entry/vdso/vdsox32.lds.S
   arch/x86/entry/vdso/vdso32/vdso32.lds.S      
   arch/x86/entry/vsyscall/vsyscall_gtod.c

  There are 3 patches in the series :

   Patch #1 makes the VDSO handle clock_gettime(CLOCK_MONOTONIC_RAW) with rdtsc_ordered()

   Patch #2 makes the VDSO handle clock_gettime(CLOCK_MONOTONIC_RAW) with a new rdtscp() function in msr.h

   Patch #3 makes the VDSO export TSC calibration data via a new function in the vDSO: 
               unsigned int __vdso_linux_tsc_calibration ( struct linux_tsc_calibration *tsc_cal )
            that user code can optionally call.

   Patches #2 & #3 should be considered "optional" .

   Patch #2 makes clock_gettime(CLOCK_MONOTONIC_RAW) calls have @ half the latency
   of clock_gettime(CLOCK_MONOTONIC) calls.

   I think something like Patch #3 is necessary to export TSC calibration data to user-space TSC readers.


Best Regards,
Jason Vas Dias

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-03-17  7:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-15 16:00 [PATCH v4.16-rc5 (3)] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW jason.vas.dias
2018-03-15 16:00 ` [PATCH v4.16-rc5 1/3] " jason.vas.dias
2018-03-15 16:00 ` [PATCH v4.16-rc5 2/3] " jason.vas.dias
2018-03-15 16:00 ` [PATCH v4.16-rc5 3/3] " jason.vas.dias
2018-03-17  7:06   ` kbuild test robot
2018-03-17  7:27   ` kbuild test robot
2018-03-15 20:17 ` [PATCH v4.16-rc5 (3)] " Thomas Gleixner
2018-03-15 21:41   ` Jason Vas Dias
2018-03-15 22:41     ` Thomas Gleixner
2018-03-16 13:30       ` Jason Vas Dias
  -- strict thread matches above, loose matches on Subject: below --
2018-03-14  4:20 jason.vas.dias

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).