Skip to contentSkip to conversation

fix ia64 clocksource

PATCH In review Hidetoshi Seto 3 participants 5 messages Jul 18, 2007 – Jul 23, 2007

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover fix ia64 clocksource

Hi all,

This is a set of patch for ia64 clocksource to improve performance of fsys_gettimeofday (and also normal gettimeofday).

[1/3] copy time structures into fsys_gtod_data
[2/3] separate data for jitter compensation
[3/3] remove cmpxchg loop in gettimeofday

Finally, the results of my test at start:

Quoted 7 lines
# clocksource (Bob's) : default
CPU  0:  4.66 (usecs) (0 errors / 2145680 iterations)
CPU  1:  4.65 (usecs) (493 errors / 2148438 iterations)
CPU  2:  4.63 (usecs) (668 errors / 2159461 iterations)
CPU  3:  4.62 (usecs) (654 errors / 2163997 iterations)
# clocksource (Bob's) : nojitter
CPU  0:  0.14 (usecs) (0 errors / 70945550 iterations)
Quoted 8 lines
CPU  1:  0.14 (usecs) (470 errors / 71640889 iterations)
CPU  2:  0.14 (usecs) (664 errors / 70960917 iterations)
CPU  3:  0.14 (usecs) (571 errors / 70956121 iterations)
# clocksource (Bob's) : nolwsys
CPU  0:  2.88 (usecs) (0 errors / 3475147 iterations)
CPU  1:  2.88 (usecs) (0 errors / 3474881 iterations)
CPU  2:  2.96 (usecs) (0 errors / 3382229 iterations)
CPU  3:  2.97 (usecs) (0 errors / 3371004 iterations)

are now refined to:

Quoted 7 lines
# clocksource (fixed) : default
CPU  0:  1.33 (usecs) (0 errors / 7507837 iterations)
CPU  1:  1.31 (usecs) (0 errors / 7621659 iterations)
CPU  2:  1.27 (usecs) (0 errors / 7865412 iterations)
CPU  3:  1.27 (usecs) (0 errors / 7863362 iterations)
# clocksource (fixed) : nojitter
CPU  0:  0.14 (usecs) (0 errors / 69608888 iterations)
Quoted 8 lines
CPU  1:  0.14 (usecs) (0 errors / 70277433 iterations)
CPU  2:  0.14 (usecs) (0 errors / 69632925 iterations)
CPU  3:  0.14 (usecs) (0 errors / 69606531 iterations)
# clocksource (fixed) : nolwsys
CPU  0:  1.48 (usecs) (0 errors / 6770870 iterations)
CPU  1:  1.48 (usecs) (0 errors / 6777897 iterations)
CPU  2:  1.49 (usecs) (0 errors / 6728101 iterations)
CPU  3:  1.49 (usecs) (0 errors / 6703961 iterations)

Thanks,
H.Seto

#1 lore raw

Hi,

Sorry for the delay in response.
Hidetoshi Seto wrote: [Wed Jul 18 2007, 10:58:55AM EDT]

Quoted 10 lines

Hi all,

This is a set of patch for ia64 clocksource to improve performance of fsys_gettimeofday (and also normal gettimeofday).

[1/3] copy time structures into fsys_gtod_data
[2/3] separate data for jitter compensation
[3/3] remove cmpxchg loop in gettimeofday

Finally, the results of my test at start:

First I'd like to thank you for your work.

I recently inherited Pete's code because of his departure. We (HP) are puzzled by the poor performance and non monotonically increasing errors. Pete's results are in conflict with your results. Perhaps you could forward your test program. Unfortunately I don't have a clue what Pete used for testing.

Your analysis is very thorough and quite comprehendible. I also finally read your thread:
http://marc.info/?l=linux-ia64&m8362166319438&w=2

I need to make the cleanups Tony identified. After that how would you like to proceed? Combining your patch 1/3 and my 1/2 seems logical.

thanks again,

bob

[snip]

#2 lore raw
In reply to Hidetoshi Seto
Quoted 2 lines

I need to make the cleanups Tony identified. After that how would you like to proceed? Combining your patch 1/3 and my 1/2 seems logical.

I've done the cleanup, and merged Seto-san's parts 1,2,3 with your part 1, and updated it all for the tip of Linus' tree. Result is below.

Part 2 stayed the same, except that I also deleted Documentation/time_interpolators.txt

I'm going to send both to Linus later today ... so read fast and shout if I botched the merge.

Time taken for a single gettimeofday() call via the fsys.S path has increased by about 8 cycles, but this would only be significant for applications doing

Quoted 1 line

1,000,000 gettimeofday calls per second. The scalability problems are so

bad that I can't even get close to that rate on 4 cpus (peak cumulative rate is about 600,000). The patched system still has problems, but scalabilty is better (4 cpus can get to 1,100,000 cumulative calls per second).

-Tony

commit 89f8d0677c72742767f98b9f5fcfbd575e73d0ae

Author: Tony Luck <[email protected]>
Date:   Wed Jul 18 15:38:51 2007 -0700

[IA64] Convert to generic timekeeping/clocksource

This is a merge of Peter Keilty's initial patch for this with Hidetoshi Seto's fixes and scaling improvements.

Documentation/kernel-parameters.txt +2 -0
index 9a54148..68115d7 100644
@@ -1154,6 +1154,8 @@ and is between 256 and 4096 characters. It is defined in the file
 
 	nointroute	[IA-64]
 
+	nojitter	[IA64] Disables jitter checking for ITC timers.
+
 	nolapic		[IA-32,APIC] Do not enable or use the local APIC.
 
 	nolapic_timer	[IA-32,APIC] Do not use the local APIC timer.
arch/ia64/Kconfig +5 -1
index 616c96e..36c7b96 100644
@@ -62,7 +62,11 @@ config GENERIC_CALIBRATE_DELAY
 	bool
 	default y
 
-config TIME_INTERPOLATION
+config GENERIC_TIME
+	bool
+	default y
+
+config GENERIC_TIME_VSYSCALL
 	bool
 	default y
 
arch/ia64/configs/bigsur_defconfig +1 -1
index 90e9c2e..9eb48c0 100644
@@ -85,7 +85,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
arch/ia64/configs/gensparse_defconfig +1 -1
index 0d29aa2..3a9ed95 100644
@@ -86,7 +86,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
arch/ia64/configs/sim_defconfig +1 -1
index d9146c3..c420d9f 100644
@@ -86,7 +86,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
arch/ia64/configs/sn2_defconfig +1 -1
index 64e951d..4c9ffc4 100644
@@ -93,7 +93,7 @@ CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_FIND_NEXT_BIT=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_DMI=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
arch/ia64/configs/tiger_defconfig +1 -1
index a144693..e208747 100644
@@ -98,7 +98,7 @@ CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 # CONFIG_ARCH_HAS_ILOG2_U64 is not set
 CONFIG_GENERIC_FIND_NEXT_BIT=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_DMI=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
arch/ia64/configs/zx1_defconfig +1 -1
index 1c7955c..4a060fc 100644
@@ -96,7 +96,7 @@ CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 # CONFIG_ARCH_HAS_ILOG2_U64 is not set
 CONFIG_GENERIC_FIND_NEXT_BIT=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_DMI=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
arch/ia64/defconfig +1 -1
index 90bd960..461f8ee 100644
@@ -98,7 +98,7 @@ CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 # CONFIG_ARCH_HAS_ILOG2_U64 is not set
 CONFIG_GENERIC_FIND_NEXT_BIT=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_DMI=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
arch/ia64/kernel/asm-offsets.c +22 -13
index 2236fab..0aebc6f 100644
@@ -7,6 +7,7 @@
 #define ASM_OFFSETS_C 1
 
 #include <linux/sched.h>
+#include <linux/clocksource.h>
 
 #include <asm-ia64/processor.h>
 #include <asm-ia64/ptrace.h>
@@ -15,6 +16,7 @@
 #include <asm-ia64/mca.h>
 
 #include "../kernel/sigframe.h"
+#include "../kernel/fsyscall_gtod_data.h"
 
 #define DEFINE(sym, val) \
         asm volatile("\n->" #sym " %0 " #val : : "i" (val))
@@ -256,17 +258,24 @@ void foo(void)
 	BLANK();
 
 	/* used by fsys_gettimeofday in arch/ia64/kernel/fsys.S */
-	DEFINE(IA64_TIME_INTERPOLATOR_ADDRESS_OFFSET, offsetof (struct time_interpolator, addr));
-	DEFINE(IA64_TIME_INTERPOLATOR_SOURCE_OFFSET, offsetof (struct time_interpolator, source));
-	DEFINE(IA64_TIME_INTERPOLATOR_SHIFT_OFFSET, offsetof (struct time_interpolator, shift));
-	DEFINE(IA64_TIME_INTERPOLATOR_NSEC_OFFSET, offsetof (struct time_interpolator, nsec_per_cyc));
-	DEFINE(IA64_TIME_INTERPOLATOR_OFFSET_OFFSET, offsetof (struct time_interpolator, offset));
-	DEFINE(IA64_TIME_INTERPOLATOR_LAST_CYCLE_OFFSET, offsetof (struct time_interpolator, last_cycle));
-	DEFINE(IA64_TIME_INTERPOLATOR_LAST_COUNTER_OFFSET, offsetof (struct time_interpolator, last_counter));
-	DEFINE(IA64_TIME_INTERPOLATOR_JITTER_OFFSET, offsetof (struct time_interpolator, jitter));
-	DEFINE(IA64_TIME_INTERPOLATOR_MASK_OFFSET, offsetof (struct time_interpolator, mask));
-	DEFINE(IA64_TIME_SOURCE_CPU, TIME_SOURCE_CPU);
-	DEFINE(IA64_TIME_SOURCE_MMIO64, TIME_SOURCE_MMIO64);
-	DEFINE(IA64_TIME_SOURCE_MMIO32, TIME_SOURCE_MMIO32);
-	DEFINE(IA64_TIMESPEC_TV_NSEC_OFFSET, offsetof (struct timespec, tv_nsec));
+	DEFINE(IA64_GTOD_LOCK_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, lock));
+	DEFINE(IA64_GTOD_WALL_TIME_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, wall_time));
+	DEFINE(IA64_GTOD_MONO_TIME_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, monotonic_time));
+	DEFINE(IA64_CLKSRC_MASK_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, clk_mask));
+	DEFINE(IA64_CLKSRC_MULT_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, clk_mult));
+	DEFINE(IA64_CLKSRC_SHIFT_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, clk_shift));
+	DEFINE(IA64_CLKSRC_MMIO_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, clk_fsys_mmio));
+	DEFINE(IA64_CLKSRC_CYCLE_LAST_OFFSET,
+		offsetof (struct fsyscall_gtod_data_t, clk_cycle_last));
+	DEFINE(IA64_ITC_JITTER_OFFSET,
+		offsetof (struct itc_jitter_data_t, itc_jitter));
+	DEFINE(IA64_ITC_LASTCYCLE_OFFSET,
+		offsetof (struct itc_jitter_data_t, itc_lastcycle));
 }
arch/ia64/kernel/cyclone.c +32 -14

100 lines not on this page · raw message

arch/ia64/kernel/fsys.S +88 -91

245 lines not on this page · raw message

arch/ia64/kernel/fsyscall_gtod_data.h new file +23 -0
new file mode 100644
index 0000000..490dab5
@@ -0,0 +1,23 @@
+/*
+ * (c) Copyright 2007 Hewlett-Packard Development Company, L.P.
+ *        Contributed by Peter Keilty <[email protected]>
+ *
+ * fsyscall gettimeofday data
+ */
+
+struct fsyscall_gtod_data_t {
+	seqlock_t	lock;
+	struct timespec	wall_time;
+	struct timespec monotonic_time;
+	cycle_t		clk_mask;
+	u32		clk_mult;
+	u32		clk_shift;
+	void		*clk_fsys_mmio;
+	cycle_t		clk_cycle_last;
+} __attribute__ ((aligned (L1_CACHE_BYTES)));
+
+struct itc_jitter_data_t {
+	int		itc_jitter;
+	cycle_t		itc_lastcycle;
+} __attribute__ ((aligned (L1_CACHE_BYTES)));
+
arch/ia64/kernel/time.c +87 -8

136 lines not on this page · raw message

arch/ia64/sn/kernel/sn2/timer.c +21 -8
index 56a88b6..19e25d2 100644
@@ -11,6 +11,7 @@
 #include <linux/sched.h>
 #include <linux/time.h>
 #include <linux/interrupt.h>
+#include <linux/clocksource.h>
 
 #include <asm/hw_irq.h>
 #include <asm/system.h>
@@ -22,11 +23,21 @@
 
 extern unsigned long sn_rtc_cycles_per_second;
 
-static struct time_interpolator sn2_interpolator = {
-	.drift = -1,
-	.shift = 10,
-	.mask = (1LL << 55) - 1,
-	.source = TIME_SOURCE_MMIO64
+static void __iomem *sn2_mc;
+
+static cycle_t read_sn2(void)
+{
+	return (cycle_t)readq(sn2_mc);
+}
+
+static struct clocksource clocksource_sn2 = {
+        .name           = "sn2_rtc",
+        .rating         = 300,
+        .read           = read_sn2,
+        .mask           = (1LL << 55) - 1,
+        .mult           = 0,
+        .shift          = 10,
+        .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 /*
@@ -47,9 +58,11 @@ ia64_sn_udelay (unsigned long usecs)
 
 void __init sn_timer_init(void)
 {
-	sn2_interpolator.frequency = sn_rtc_cycles_per_second;
-	sn2_interpolator.addr = RTC_COUNTER_ADDR;
-	register_time_interpolator(&sn2_interpolator);
+	sn2_mc = RTC_COUNTER_ADDR;
+	clocksource_sn2.fsys_mmio = RTC_COUNTER_ADDR;
+	clocksource_sn2.mult = clocksource_hz2mult(sn_rtc_cycles_per_second,
+							clocksource_sn2.shift);
+	clocksource_register(&clocksource_sn2);
 
 	ia64_udelay = &ia64_sn_udelay;
 }
drivers/acpi/processor_idle.c +2 -2
index 80ffc78..bb5d23b 100644
@@ -475,7 +475,7 @@ static void acpi_processor_idle(void)
 		/* Get end time (ticks) */
 		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
 
-#ifdef CONFIG_GENERIC_TIME
+#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
 		/* TSC halts in C2, so notify users */
 		mark_tsc_unstable("possible TSC halt in C2");
 #endif
@@ -517,7 +517,7 @@ static void acpi_processor_idle(void)
 			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
 		}
 
-#ifdef CONFIG_GENERIC_TIME
+#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
 		/* TSC halts in C3, so notify users */
 		mark_tsc_unstable("TSC halts in C3");
 #endif
drivers/char/hpet.c +39 -31

114 lines not on this page · raw message

include/linux/clocksource.h +6 -0
index bf297b0..562c86a 100644
@@ -67,6 +67,12 @@ struct clocksource {
 	unsigned long flags;
 	cycle_t (*vread)(void);
 	void (*resume)(void);
+#ifdef CONFIG_IA64
+	void *fsys_mmio;        /* used by fsyscall asm code */
+#define CLKSRC_FSYS_MMIO_SET(mmio, addr)      ((mmio) = (addr))
+#else
+#define CLKSRC_FSYS_MMIO_SET(mmio, addr)      (0)
+#endif
 
 	/* timekeeping specific data, ignore */
 	cycle_t cycle_interval;
Acked-by on the v1 series
#3 lore raw
In reply to Hidetoshi Seto

luck wrote: [Fri Jul 20 2007, 12:33:43PM EDT]

Quoted 11 lines
Quoted 2 lines

I need to make the cleanups Tony identified. After that how would you like to proceed? Combining your patch 1/3 and my 1/2 seems logical.

I've done the cleanup, and merged Seto-san's parts 1,2,3 with your part 1, and updated it all for the tip of Linus' tree. Result is below.

Part 2 stayed the same, except that I also deleted Documentation/time_interpolators.txt

I'm going to send both to Linus later today ... so read fast and shout if I botched the merge.

Looks good to me. Thanks!

bob

[snip]

author Acked-by on the v1 series
#4 lore raw
In reply to Hidetoshi Seto
Quoted 10 lines · Tony
Quoted 2 lines

I need to make the cleanups Tony identified. After that how would you like to proceed? Combining your patch 1/3 and my 1/2 seems logical.

I've done the cleanup, and merged Seto-san's parts 1,2,3 with your part 1, and updated it all for the tip of Linus' tree. Result is below.

Part 2 stayed the same, except that I also deleted Documentation/time_interpolators.txt

I'm going to send both to Linus later today ... so read fast and shout if I botched the merge.

No problem at all. Thanks!

H.Seto