Skip to contentSkip to conversation

fio: fix cycles_start build issue

PATCH In review Christian Ehrhardt 2 participants 3 messages Feb 27, 2015 – Mar 2, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fio: fix cycles_start build issue

Hi,
a recent checkout of fio-git revealed a build issue on s390.
For whatever reason my git send-email setup is broken so I paste it in the mail and hope it doesn't get mangled, let me know if that fails.

---

Subject: [PATCH] fio: fix cycles_start build issue

From: Christian Ehrhardt <[email protected]>

There is an issue introduced with "commit 73df3e07 gettime: offset CPU cycle counter by initial value". For architectures which define ARCH_CPU_CLOCK_CYCLES_PER_USEC (currently only s390) this causes a build error

gettime.c:174:11: error: ‘cycles_start’ undeclared (first use in this function)
if (t < cycles_start && !cycles_wrap)

To make sure variables and code are only compiled in the same cases I added a clock definition called ARCH_CPU_CLOCK_WRAPS. We could merge ARCH_CPU_CLOCK_WRAPS and the existing ARCH_CPU_CLOCK_CYCLES_PER_USEC into one if you prefer, so far nobody else uses it.

To avoid cluttering all architecture headers I enabled it by default in arch-generic.h, so any arch not needing the wrap handling can undef ARCH_CPU_CLOCK_WRAPS later in their headers.

---

[diffstat]

 arch/arch-generic.h |    1 +
 arch/arch-s390.h    |    1 +
 gettime.c           |    7 +++++++
 3 files changed, 9 insertions(+)

[diff]

arch/arch-generic.h +1 -0
index a0b71f8..825a829 100644
@@ -2,6 +2,7 @@
 #define ARCH_GENERIC_H
 
 #define FIO_ARCH	(arch_generic)
+#define ARCH_CPU_CLOCK_WRAPS
 
 #define nop			do { } while (0)
 #define read_barrier()		__asm__ __volatile__("": : :"memory")
arch/arch-s390.h +1 -0
index 169282b..0c93424 100644
@@ -39,6 +39,7 @@ static inline unsigned long long get_cpu_clock(void)
 }
 
 #define ARCH_CPU_CLOCK_CYCLES_PER_USEC 1
+#undef ARCH_CPU_CLOCK_WRAPS
 #define ARCH_HAVE_CPU_CLOCK
 
 #define ARCH_HAVE_INIT
gettime.c +7 -0
index 6863ce3..29884a1 100644
@@ -17,6 +17,8 @@
 static unsigned long cycles_per_usec;
 static unsigned long inv_cycles_per_usec;
 static uint64_t max_cycles_for_mult;
+#endif
+#ifdef ARCH_CPU_CLOCK_WRAPS
 static unsigned long long cycles_start, cycles_wrap;
 #endif
 int tsc_reliable = 0;
@@ -171,6 +173,7 @@ static void __fio_gettime(struct timeval *tp)
 #endif
 
 		t = get_cpu_clock();
+#ifdef ARCH_CPU_CLOCK_WRAPS
 		if (t < cycles_start && !cycles_wrap)
 			cycles_wrap = 1;
 		else if (cycles_wrap && t >= cycles_start && !tv->warned) {
@@ -179,6 +182,8 @@ static void __fio_gettime(struct timeval *tp)
 		}
 
 		t -= cycles_start;
+#endif
+
 		tv->last_cycles = t;
 		tv->last_tv_valid = 1;
 #ifdef ARCH_CPU_CLOCK_CYCLES_PER_USEC
@@ -311,8 +316,10 @@ static int calibrate_cpu_clock(void)
 	inv_cycles_per_usec = 16777216UL / cycles_per_usec;
 	max_cycles_for_mult = ~0ULL / inv_cycles_per_usec;
 	dprint(FD_TIME, "inv_cycles_per_usec=%lu\n", inv_cycles_per_usec);
+#ifdef ARCH_CPU_CLOCK_WRAPS
 	cycles_start = get_cpu_clock();
 	dprint(FD_TIME, "cycles_start=%llu\n", cycles_start);
+#endif
 	return 0;
 }
 #else
#1 lore raw
Quoted 4 lines · Christian Ehrhardt

Hi,
a recent checkout of fio-git revealed a build issue on s390. For whatever reason my git send-email setup is broken so I paste it in the mail and hope it doesn't get mangled, let me know if that fails.

It mangled it horribly, but I just hand applied it. One issue, though:

arch/arch-generic.h line 5

Quoted 9 lines
arch/arch-generic.h +1 -0
index a0b71f8..825a829 100644
@@ -2,6 +2,7 @@
  #define ARCH_GENERIC_H
 
  #define FIO_ARCH    (arch_generic)
+#define ARCH_CPU_CLOCK_WRAPS

This wont work like I suspect you think it does. This define needs to go in the arch.h header, if you want all archs to catch it.

Signature · 1 line
Jens Axboe
#2 lore raw
Quoted 15 lines · Jens Axboe
Quoted 4 lines · Christian Ehrhardt

Hi,
a recent checkout of fio-git revealed a build issue on s390. For whatever reason my git send-email setup is broken so I paste it in the mail and hope it doesn't get mangled, let me know if that fails.

It mangled it horribly, but I just hand applied it. One issue, though:

Quoted 7 lines
arch/arch-generic.h +0 -0
index a0b71f8..825a829 100644
@@ -2,6 +2,7 @@
  #define ARCH_GENERIC_H
 
Quoted 6 lines

arch/arch-generic.h line 5

Quoted 2 lines
arch/arch-generic.h +1 -0
  #define FIO_ARCH    (arch_generic)
+#define ARCH_CPU_CLOCK_WRAPS

This wont work like I suspect you think it does. This define needs to go in the arch.h header, if you want all archs to catch it.

I can only agree and thank you for unmangling the content. Thanks !