12345 comm page should fallback to syscall after excessive migration

Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Ryan Zezeski <rpz@joyent.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Dan McDonald <danmcd@joyent.com>

3 files changed, 161 insertions, 52 deletions

diff --git a/usr/src/lib/commpage/amd64/cp_subr.s b/usr/src/lib/commpage/amd64/cp_subr.s
index 779fc9bdd7..09b8deaf8d 100644
--- a/usr/src/lib/commpage/amd64/cp_subr.s
+++ b/usr/src/lib/commpage/amd64/cp_subr.s
@@ -10,7 +10,7 @@
  */
 
 /*
- * Copyright 2016 Joyent, Inc.
+ * Copyright 2019 Joyent, Inc.
  */
 
 #include <sys/asm_linkage.h>
@@ -33,6 +33,12 @@
 #define	NANOSEC		0x3b9aca00
 
 /*
+ * For __cp_tsc_read calls which incur looping retries due to CPU migration,
+ * this represents the maximum number of tries before bailing out.
+ */
+#define	TSC_READ_MAXLOOP	0x4
+
+/*
  * hrtime_t
  * __cp_tsc_read(comm_page_t *cp)
  *
@@ -41,7 +47,6 @@
 	ENTRY_NP(__cp_tsc_read)
 	movl	CP_TSC_TYPE(%rdi), %esi
 	movl	CP_TSC_NCPU(%rdi), %r8d
-	leaq	CP_TSC_SYNC_TICK_DELTA(%rdi), %r9
 
 	cmpl	$TSC_TSCP, %esi
 	jne	2f
@@ -53,40 +58,58 @@
 	 */
 	shlq	$0x20, %rdx
 	orq	%rdx, %rax
-	cmpl	$0, %esi
-	jne	1f
+
+	/*
+	 * A zeroed cp_tsc_ncpu (currently held in r8d) indicates that no
+	 * per-CPU TSC offsets are required.
+	 */
+	testl	%r8d, %r8d
+	jnz	1f
 	ret
+
 1:
 	/*
-	 * When cp_tsc_ncpu is non-zero, it indicates the length of the
-	 * cp_tsc_sync_tick_delta array, which contains per-CPU offsets for the
-	 * TSC.  The CPU ID furnished by the IA32_TSC_AUX register via rdtscp
-	 * is used to look up an offset value in that array and apply it to the
-	 * TSC reading.
+	 * A non-zero cp_tsc_ncpu indicates the array length of
+	 * cp_tsc_sync_tick_delta containing per-CPU offsets which are applied
+	 * to TSC readings.  The CPU ID furnished by the IA32_TSC_AUX register
+	 * via rdtscp (placed in rcx) is used to look up an offset value in
+	 * that array and apply it to the TSC value.
 	 */
+	leaq	CP_TSC_SYNC_TICK_DELTA(%rdi), %r9
 	movq	(%r9, %rcx, 8), %rdx
 	addq	%rdx, %rax
 	ret
 
 2:
 	/*
-	 * Without rdtscp, there is no way to perform a TSC reading and
-	 * simultaneously query the current CPU.  If tsc_ncpu indicates that
-	 * per-CPU TSC offsets are present, the ID of the current CPU is
-	 * queried before performing a TSC reading.  It will be later compared
-	 * to a second CPU ID lookup to catch CPU migrations.
+	 * TSC reading without RDTSCP
+	 *
+	 * Check if handling for per-CPU TSC offsets is required.  If not,
+	 * immediately skip to the the appropriate steps to perform a rdtsc.
 	 *
-	 * This method will catch all but the most pathological scheduling.
+	 * If per-CPU offsets are present, the TSC reading process is more
+	 * complicated.  Without rdtscp, there is no way to simultaneously read
+	 * the TSC and query the current CPU.  In order to "catch" migrations
+	 * during execution, the CPU ID is queried before and after rdtsc.  The
+	 * execution is repeated if results differ, subject to a loop limit.
 	 */
-	cmpl	$0, %r8d
-	je	3f
+	xorq	%r9, %r9
+	testl	%r8d, %r8d
+	jz	3f
+
+	/*
+	 * Load the address of the per-CPU offset array, since it is needed.
+	 * The attempted loop count is kept in r8.
+	 */
+	leaq	CP_TSC_SYNC_TICK_DELTA(%rdi), %r9
+	xorl	%r8d, %r8d
+
+	/* Query the CPU ID and stash it in r10 for later comparison */
 	movl	$GETCPU_GDT_OFFSET, %edx
 	lsl	%dx, %edx
-
-3:
-	/* Save the most recently queried CPU ID for later comparison. */
 	movl	%edx, %r10d
 
+3:
 	cmpl	$TSC_RDTSC_MFENCE, %esi
 	jne	4f
 	mfence
@@ -118,30 +141,51 @@
 6:
 	/*
 	 * Other protections should have prevented this function from being
-	 * called in the first place.  The only sane action is to abort.
-	 * The easiest means in this context is via SIGILL.
+	 * called in the first place.  Since callers must handle a failure from
+	 * CPU migration looping, yield the same result as a bail-out: 0
 	 */
-	ud2a
+	xorl	%eax, %eax
+	ret
 
 7:
 	shlq	$0x20, %rdx
 	orq	%rdx, %rax
 
 	/*
-	 * Query the current CPU again if a per-CPU offset is being applied to
-	 * the TSC reading.  If the result differs from the earlier reading,
-	 * then a migration has occured and the TSC must be read again.
+	 * With the TSC reading in-hand, check if any per-CPU offset handling
+	 * is required.  The address to the array of deltas (r9) will not have
+	 * been populated if offset handling is unecessary.
 	 */
-	cmpl	$0, %r8d
-	je	8f
+	testq	%r9, %r9
+	jnz	8f
+	ret
+
+8:
 	movl	$GETCPU_GDT_OFFSET, %edx
 	lsl	%dx, %edx
 	cmpl	%edx, %r10d
-	jne	3b
+	jne	9f
 	movq	(%r9, %rdx, 8), %rdx
 	addq	%rdx, %rax
-8:
 	ret
+
+9:
+	/*
+	 * It appears that a migration has occurred between the first CPU ID
+	 * query and now.  Check if the loop limit has been broken and retry if
+	 * that's not the case.
+	 */
+	cmpl	$TSC_READ_MAXLOOP, %r8d
+	jge	10f
+	incl	%r8d
+	movl	%edx, %r10d
+	jmp	3b
+
+10:
+	/* Loop limit was reached. Return bail-out value of 0. */
+	xorl	%eax, %eax
+	ret
+
 	SET_SIZE(__cp_tsc_read)
 
 
@@ -192,8 +236,15 @@
 	movq	%rdx, 0x8(%rsp)
 
 	call	__cp_tsc_read
-	movq	0x10(%rsp), %rdi
 
+	/*
+	 * Failure is inferred from a TSC reading of 0.  The normal fasttrap
+	 * mechanism can be used as a fallback in such cases.
+	 */
+	testq	%rax, %rax
+	jz	6f
+
+	movq	0x10(%rsp), %rdi
 	movl	0x18(%rsp), %r9d
 	movl	CP_HRES_LOCK(%rdi), %edx
 	andl	$0xfffffffe, %r9d
@@ -270,6 +321,12 @@
 5:
 	jmp	2b
 
+6:
+	movl	$T_GETHRTIME, %eax
+	int	$T_FASTTRAP
+	addq	$0x20, %rsp
+	ret
+
 	SET_SIZE(__cp_gethrtime)
 
 /*
diff --git a/usr/src/lib/commpage/common/cp_main.c b/usr/src/lib/commpage/common/cp_main.c
index 62f4a001ea..13a9aea3d7 100644
--- a/usr/src/lib/commpage/common/cp_main.c
+++ b/usr/src/lib/commpage/common/cp_main.c
@@ -10,7 +10,7 @@
  */
 
 /*
- * Copyright 2016 Joyent, Inc.
+ * Copyright 2019 Joyent, Inc.
  */
 
 #include <sys/comm_page.h>
@@ -48,6 +48,7 @@ __cp_can_gettime(comm_page_t *cp)
  * ASM-defined functions.
  */
 extern hrtime_t __cp_tsc_read(comm_page_t *);
+extern hrtime_t __cp_gethrtime_fasttrap();
 
 /*
  * These are cloned from TSC and time related code in the kernel.  The should
@@ -58,7 +59,7 @@ extern hrtime_t __cp_tsc_read(comm_page_t *);
 #define	NANOSEC		1000000000LL
 
 #define	TSC_CONVERT_AND_ADD(tsc, hrt, scale) do {		\
-	uint32_t *_l = (uint32_t *)&(tsc); 			\
+	uint32_t *_l = (uint32_t *)&(tsc);			\
 	uint64_t sc = (uint32_t)(scale);			\
 	(hrt) += (uint64_t)(_l[1] * sc) << NSEC_SHIFT;		\
 	(hrt) += (uint64_t)(_l[0] * sc) >> (32 - NSEC_SHIFT);	\
@@ -95,6 +96,14 @@ __cp_gethrtime(comm_page_t *cp)
 		tsc_last = cp->cp_tsc_last;
 		hrt = cp->cp_tsc_hrtime_base;
 		tsc = __cp_tsc_read(cp);
+
+		/*
+		 * A TSC reading of 0 indicates the special case of an error
+		 * bail-out.  Rely on the fasttrap to supply an hrtime value.
+		 */
+		if (tsc == 0) {
+			return (__cp_gethrtime_fasttrap());
+		}
 	} while ((old_hres_lock & ~1) != cp->cp_hres_lock);
 
 	if (tsc >= tsc_last) {
diff --git a/usr/src/lib/commpage/i386/cp_subr.s b/usr/src/lib/commpage/i386/cp_subr.s
index 990864d0ba..d1e07008c4 100644
--- a/usr/src/lib/commpage/i386/cp_subr.s
+++ b/usr/src/lib/commpage/i386/cp_subr.s
@@ -10,7 +10,7 @@
  */
 
 /*
- * Copyright 2016 Joyent, Inc.
+ * Copyright 2019 Joyent, Inc.
  */
 
 #include <sys/asm_linkage.h>
@@ -21,20 +21,33 @@
 
 #define	GETCPU_GDT_OFFSET	SEL_GDT(GDT_CPUID, SEL_UPL)
 
+/*
+ * For __cp_tsc_read calls which incur looping retries due to CPU migration,
+ * this represents the maximum number of tries before bailing out.
+ */
+#define	TSC_READ_MAXLOOP	0x4
+
 	.file	"cp_subr.s"
 
 /*
  * hrtime_t
- * __cp_tsc_read(uint_t cp_tsc_type)
+ * __cp_tsc_read(comm_page_t *cp)
  *
  * Stack usage: 0x18 bytes
+ *
+ * %ebp-0x00 - frame pointer
+ * %ebp-0x04 - saved %edi
+ * %ebp-0x08 - saved %esi
+ * %ebp-0x0c - CPU ID
+ * %ebp-0x10 - loop count
+ * %ebp-0x14 - saved %ebx (during cpuid)
  */
 	ENTRY_NP(__cp_tsc_read)
 	pushl	%ebp
 	movl	%esp, %ebp
 	pushl	%edi
 	pushl	%esi
-	subl	$0x4, %esp
+	subl	$0x8, %esp
 
 	movl	0x8(%ebp), %edi
 	movl	CP_TSC_TYPE(%edi), %eax
@@ -45,7 +58,7 @@
 	cmpl	$0, %esi
 	jne	2f
 1:
-	addl	$0x4, %esp
+	addl	$0x8, %esp
 	popl	%esi
 	popl	%edi
 	leave
@@ -65,11 +78,13 @@
 	jmp	1b
 
 3:
-	cmpl	$0, %esi
-	je	4f
+	testl	%esi, %esi
+	jz	4f
+
+	movl	$0, (%esp)
 	mov	$GETCPU_GDT_OFFSET, %eax
 	lsl	%ax, %eax
-	movl	%eax, (%esp)
+	movl	%eax, 0x4(%esp)
 	movl	CP_TSC_TYPE(%edi), %eax
 
 4:
@@ -99,10 +114,12 @@
 7:
 	/*
 	 * Other protections should have prevented this function from being
-	 * called in the first place.  The only sane action is to abort.
-	 * The easiest means in this context is via SIGILL.
+	 * called in the first place.  Since callers must handle a failure from
+	 * CPU migration looping, yield the same result as a bail-out: 0
 	 */
-	ud2a
+	xorl	%eax, %eax
+	xorl	%edx, %edx
+	jmp	1b
 
 8:
 
@@ -114,27 +131,53 @@
 	 */
 	movl	$GETCPU_GDT_OFFSET, %ecx
 	lsl	%cx, %ecx
-	movl	(%esp), %esi
+	movl	0x4(%esp), %esi
 	cmpl	%ecx, %esi
-	je	9f
+	jne	9f
+
+	/* Grab the per-cpu offset and add it to the TSC result */
+	leal	CP_TSC_SYNC_TICK_DELTA(%edi), %esi
+	leal	(%esi, %ecx, 8), %ecx
+	addl	(%ecx), %eax
+	adcl	0x4(%ecx), %edx
+	jmp	1b
+
+9:
 	/*
-	 * There was a CPU migration, perform another reading.
+	 * It appears that a migration has occurred between the first CPU ID
+	 * query and now.  Check if the loop limit has been broken and retry if
+	 * that's not the case.
 	 */
+	movl	(%esp), %eax
+	cmpl	$TSC_READ_MAXLOOP, %eax
+	jge	10f
+
+	incl	%eax
 	movl	%eax, (%esp)
+	movl	%ecx, 0x4(%esp)
 	movl	CP_TSC_NCPU(%edi), %esi
 	movl	CP_TSC_TYPE(%edi), %eax
 	jmp	4b
 
-9:
-	/* Grab the per-cpu offset and add it to the TSC result */
-	leal	CP_TSC_SYNC_TICK_DELTA(%edi), %esi
-	leal	(%esi, %ecx, 8), %ecx
-	addl	(%ecx), %eax
-	adcl	0x4(%ecx), %edx
+10:
+	/* Loop limit was reached. Return bail-out value of 0. */
+	xorl	%eax, %eax
+	xorl	%edx, %edx
 	jmp	1b
+
 	SET_SIZE(__cp_tsc_read)
 
 /*
+ * hrtime_t
+ * __cp_gethrtime_fasttrap()
+ */
+	ENTRY_NP(__cp_gethrtime_fasttrap)
+	movl	$T_GETHRTIME, %eax
+	int	$T_FASTTRAP
+	ret
+	SET_SIZE(__cp_gethrtime_fasttrap)
+
+/*
  * uint_t
  * __cp_getcpu(uint_t cp_tsc_type)
  */