[patch 0/2] timing and nop fixes for the s390 architecture

[patch 0/2] timing and nop fixes for the s390 architecture

[ehrhardt]

Testing recent fio code revealed a few issues on s390.
The patches in this series aim to fix them.

[patch 1/2] [PATCH] fio: fix s390 time accounting

[ehrhardt]

[-- Attachment #1: fix-time-accounting-s390.patch --]
[-- Type: text/plain, Size: 3197 bytes --]

From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>

The current timer implementation could cause time warps on s390 which ends up
as time bound jobs that would never end, because they always reset themself to
the old time.

When touching this code anyway, we also change it to use the faster stckf
and avoid the calibration as we can control the result to be usecs.
This also eliminates a few calculations cycle->usec in the hot path for the
timer.

In case other architectures have similar improved timers that might not be
usec based, but nsec based or such a thing any architecture can set
ARCH_CPU_CLOCK_CYCLES_PER_USEC to an appropriate per-arch value.
This leaves the infrastructure open for others and the compiler will throw
our division by 1 away anyway.

Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
---

[diffstat]
 arch/arch-s390.h |   13 +++++++++----
 gettime.c        |   14 +++++++++++---
 2 files changed, 20 insertions(+), 7 deletions(-)

[diff]


Index: fio/arch/arch-s390.h
===================================================================
--- fio.orig/arch/arch-s390.h
+++ fio/arch/arch-s390.h
@@ -22,14 +22,21 @@
 #define read_barrier()	asm volatile("bcr 15,0" : : : "memory")
 #define write_barrier()	asm volatile("bcr 15,0" : : : "memory")
 
+/*
+ * Fio needs monotonic (never lower), but not strict monotonic (never the same)
+ * so store clock fast is enough
+ */
 static inline unsigned long long get_cpu_clock(void)
 {
 	unsigned long long clk;
 
-	__asm__ __volatile__("stck %0" : "=Q" (clk) : : "cc");
-	return clk;
+	__asm__ __volatile__("stckf %0" : "=Q" (clk) : : "cc");
+	return clk>>12;
 }
 
+#define ARCH_CPU_CLOCK_CYCLES_PER_USEC 1
+#define ARCH_HAVE_CPU_CLOCK
+
 #define ARCH_HAVE_INIT
 extern int tsc_reliable;
 static inline int arch_init(char *envp[])
@@ -38,6 +45,4 @@ static inline int arch_init(char *envp[]
 	return 0;
 }
 
-#define ARCH_HAVE_CPU_CLOCK
-
 #endif
Index: fio/gettime.c
===================================================================
--- fio.orig/gettime.c
+++ fio/gettime.c
@@ -13,7 +13,7 @@
 #include "hash.h"
 #include "os/os.h"
 
-#ifdef ARCH_HAVE_CPU_CLOCK
+#if defined(ARCH_HAVE_CPU_CLOCK) && !defined(ARCH_CPU_CLOCK_CYCLES_PER_USEC)
 static unsigned long cycles_per_usec;
 static unsigned long inv_cycles_per_usec;
 #endif
@@ -177,7 +177,11 @@ static void *__fio_gettime(struct timeva
 		} else if (tv)
 			tv->last_cycles = t;
 
+#ifdef ARCH_CPU_CLOCK_CYCLES_PER_USEC
+		usecs = t / ARCH_CPU_CLOCK_CYCLES_PER_USEC;
+#else
 		usecs = (t * inv_cycles_per_usec) / 16777216UL;
+#endif
 		tp->tv_sec = usecs / 1000000;
 		tp->tv_usec = usecs % 1000000;
 		break;
@@ -229,7 +233,7 @@ void fio_gettime(struct timeval *tp, voi
 	}
 }
 
-#ifdef ARCH_HAVE_CPU_CLOCK
+#if defined(ARCH_HAVE_CPU_CLOCK) && !defined(ARCH_CPU_CLOCK_CYCLES_PER_USEC)
 static unsigned long get_cycles_per_usec(void)
 {
 	struct timeval s, e;
@@ -318,9 +322,13 @@ static int calibrate_cpu_clock(void)
 #else
 static int calibrate_cpu_clock(void)
 {
+#ifdef ARCH_CPU_CLOCK_CYCLES_PER_USEC
+	return 0;
+#else
 	return 1;
-}
 #endif
+}
+#endif // ARCH_HAVE_CPU_CLOCK
 
 #ifndef CONFIG_TLS_THREAD
 void fio_local_clock_init(int is_thread)

[patch 2/2] [PATCH] fio: fix s390 nop

[ehrhardt]

[-- Attachment #1: fix-s390-nop.diff --]
[-- Type: text/plain, Size: 900 bytes --]

From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>

When trying to run rate limited fio runs we encountered that the current
definition of a nop uses a privileged instruction which is not even a nop,
but a yield call to the hipervisor.
That leads to a SIGILL as it is privileged.

To solve that issue replace it with a nop, the assembler will take care
of it (likely to become a BCR 0,0)

Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
---

[diffstat]
 arch/arch-s390.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

[diff]
--- a/arch/arch-s390.h
+++ b/arch/arch-s390.h
@@ -18,7 +18,7 @@
 #define __NR_sys_vmsplice	309
 #endif
 
-#define nop		asm volatile ("diag 0,0,68" : : : "memory")
+#define nop		asm volatile("nop" : : : "memory")
 #define read_barrier()	asm volatile("bcr 15,0" : : : "memory")
 #define write_barrier()	asm volatile("bcr 15,0" : : : "memory")
 

Re: [patch 0/2] timing and nop fixes for the s390 architecture

[Jens Axboe]

On 2014-04-08 09:51, ehrhardt@linux.vnet.ibm.com wrote:
> Testing recent fio code revealed a few issues on s390.
> The patches in this series aim to fix them.

Thanks! Applied both.

-- 
Jens Axboe