fix ia64 clocksource : separate data for jitter compensation
PATCH 2/3 fix ia64 clocksource : separate data for jitter compensation
This is 2 of 3 patches for ia64 clocksource.
My former patch (1 of 3) revealed that "default" has problem. It was assumed that something in combination of "fsys + jitter" is wrong since both of "nojitter (= fsys)" and "nolwsys (= non-fsys
+ jitter)" provides better result.
Especially it is assumed as serious bug that "non-fsys + jitter", written in C is faster than "fsys + jitter", written in assembler.
I picked up some points:
- It seems that logics are not so different.
- both uses jitter-related data in fsys_gtod_data
- only fsys uses rests of contents of fsys_gtod_data, while non-fsys still uses xtime.
- xtime_lock is larger lock than gtod_lock.
So summary was:
"reading one structure with small lock"
is slower than
"reading two structure with large lock"
I came near to give up, but for some reason, an inspiration hit me, and at last I caught a line.
Quoted 1 line
}
__attribute__((aligned (L1_CACHE_BYTES)));
The imagined situation is that a read from fsys_gtod_data was disturbed by concurrent cmpxchg of itc_lastcycle, which located in same cacheline.
If so, it can be explained that why read from xtime is faster than read from fsys_gtod_data.
So I made this patch to:
- separate itc_lastcycle from fsys_gtod_data and put it on other cacheline.
- separate itc_jitter from fsys_gtod_data too. in fact, itc_lastcycle and itc_jitter are used from both of fsys and non-fsys.
Fortunately this patch works well, then the results:
Quoted 7 lines
# savextime : default CPU 0: 5.49 (usecs) (0 errors / 1822461 iterations) CPU 1: 5.46 (usecs) (0 errors / 1830527 iterations) CPU 2: 5.45 (usecs) (0 errors / 1834697 iterations) CPU 3: 5.46 (usecs) (0 errors / 1831464 iterations) # savextime : nojitter CPU 0: 0.14 (usecs) (0 errors / 70918167 iterations)
Quoted 8 lines
CPU 1: 0.14 (usecs) (0 errors / 71620242 iterations) CPU 2: 0.14 (usecs) (0 errors / 70946394 iterations) CPU 3: 0.14 (usecs) (0 errors / 70933043 iterations) # savextime : nolwsys CPU 0: 2.89 (usecs) (0 errors / 3459498 iterations) CPU 1: 2.89 (usecs) (0 errors / 3458876 iterations) CPU 2: 2.96 (usecs) (0 errors / 3377799 iterations) CPU 3: 2.97 (usecs) (0 errors / 3372353 iterations)
become more reasonable, expected level:
Quoted 7 lines
# separatejitter : default CPU 0: 1.50 (usecs) (0 errors / 6677159 iterations) CPU 1: 1.49 (usecs) (0 errors / 6697159 iterations) CPU 2: 1.50 (usecs) (0 errors / 6664672 iterations) CPU 3: 1.50 (usecs) (0 errors / 6668999 iterations) # separatejitter : nojitter CPU 0: 0.14 (usecs) (0 errors / 70580221 iterations)
Quoted 8 lines
CPU 1: 0.14 (usecs) (0 errors / 71275618 iterations) CPU 2: 0.14 (usecs) (0 errors / 70626121 iterations) CPU 3: 0.14 (usecs) (0 errors / 70603364 iterations) # separatejitter : nolwsys CPU 0: 2.26 (usecs) (0 errors / 4417197 iterations) CPU 1: 2.26 (usecs) (0 errors / 4415829 iterations) CPU 2: 2.27 (usecs) (0 errors / 4402768 iterations) CPU 3: 2.27 (usecs) (0 errors / 4406101 iterations)
(Continue to next patch.)
Thanks,
H.Seto
-----
arch/ia64/kernel/asm-offsets.c | 6 +++--- arch/ia64/kernel/fsys.S | 7 +++++-- arch/ia64/kernel/fsyscall_gtod_data.h | 5 ++++- arch/ia64/kernel/time.c | 12 +++++++----- 4 files changed, 19 insertions(+), 11 deletions(-)
Index: linux-2.6.22/arch/ia64/kernel/asm-offsets.c =================================--- linux-2.6.22.orig/arch/ia64/kernel/asm-offsets.c +++ linux-2.6.22/arch/ia64/kernel/asm-offsets.c
@@ -274,8 +274,8 @@
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_LASTCYCLE_OFFSET,
- offsetof (struct fsyscall_gtod_data_t, itc_lastcycle));
DEFINE(IA64_ITC_JITTER_OFFSET,
- offsetof (struct fsyscall_gtod_data_t, itc_jitter));
+ offsetof (struct itc_jitter_data_t, itc_jitter));
+ DEFINE(IA64_ITC_LASTCYCLE_OFFSET,
+ offsetof (struct itc_jitter_data_t, itc_lastcycle));
}
Index: linux-2.6.22/arch/ia64/kernel/fsys.S =================================--- linux-2.6.22.orig/arch/ia64/kernel/fsys.S +++ linux-2.6.22/arch/ia64/kernel/fsys.S
@@ -150,6 +150,9 @@
#if IA64_GTOD_LOCK_OFFSET !=0
#error fsys_gettimeofday incompatible with changes to struct fsyscall_gtod_data_t #endif
+#if IA64_ITC_JITTER_OFFSET !=0
+#error fsys_gettimeofday incompatible with changes to struct itc_jitter_data_t +#endif
#define CLOCK_REALTIME 0
#define CLOCK_MONOTONIC 1
#define CLOCK_DIVIDE_BY_1000 0x4000
@@ -215,13 +218,13 @@
add r2 = TI_FLAGS+IA64_TASK_SIZE,r16
movl r20 = fsyscall_gtod_data // load fsyscall gettimeofday data address ;;
- add r29 = IA64_ITC_JITTER_OFFSET,r20
+ movl r29 = itc_jitter_data // itc_jitter
add r22 = IA64_GTOD_WALL_TIME_OFFSET,r20 // wall_time
ld4 r2 = [r2] // process work pending flags
;;
(p15) add r22 = IA64_GTOD_MONO_TIME_OFFSET,r20 // monotonic_time
add r21 = IA64_CLKSRC_MMIO_OFFSET,r20
- add r19 = IA64_ITC_LASTCYCLE_OFFSET,r20
+ add r19 = IA64_ITC_LASTCYCLE_OFFSET,r29
and r2 = TIF_ALLWORK_MASK,r2
(p6) br.cond.spnt.few .fail_einval // ? deferred branch
;;
Index: linux-2.6.22/arch/ia64/kernel/fsyscall_gtod_data.h =================================--- linux-2.6.22.orig/arch/ia64/kernel/fsyscall_gtod_data.h +++ linux-2.6.22/arch/ia64/kernel/fsyscall_gtod_data.h
@@ -14,7 +14,10 @@
u32 clk_shift;
void *clk_fsys_mmio;
cycle_t clk_cycle_last;
- cycle_t itc_lastcycle;
+} __attribute__ ((aligned (L1_CACHE_BYTES)));
+
+struct itc_jitter_data_t {
int itc_jitter;
+ cycle_t itc_lastcycle;
} __attribute__ ((aligned (L1_CACHE_BYTES)));
Index: linux-2.6.22/arch/ia64/kernel/time.c =================================--- linux-2.6.22.orig/arch/ia64/kernel/time.c +++ linux-2.6.22/arch/ia64/kernel/time.c
@@ -37,6 +37,8 @@
.lock = SEQLOCK_UNLOCKED,
};
+struct itc_jitter_data_t itc_jitter_data;
+
volatile int time_keeper_id = 0; /* smp_processor_id() of time-keeper */
#ifdef CONFIG_IA64_DEBUG_IRQ
@@ -236,7 +238,7 @@
* are too large.
*/
if (!nojitter)
- fsyscall_gtod_data.itc_jitter = 1;
+ itc_jitter_data.itc_jitter = 1;
#endif
}
@@ -258,10 +260,10 @@
u64 lcycle;
u64 now;
- if (!fsyscall_gtod_data.itc_jitter)
+ if (!itc_jitter_data.itc_jitter)
return get_cycles();
do {
- lcycle = fsyscall_gtod_data.itc_lastcycle;
+ lcycle = itc_jitter_data.itc_lastcycle;
now = get_cycles();
if (lcycle && time_after(lcycle, now))
return lcycle;
@@ -271,14 +273,14 @@
* force to retry until the write lock is released. */
if (spin_is_locked(&xtime_lock.lock)) {
- fsyscall_gtod_data.itc_lastcycle = now;
+ itc_jitter_data.itc_lastcycle = now;
return now;
}
/* Keep track of the last timer value returned.
* The use of cmpxchg here will cause contention in
* an SMP environment.
*/
- } while (likely(cmpxchg(&fsyscall_gtod_data.itc_lastcycle,
+ } while (likely(cmpxchg(&itc_jitter_data.itc_lastcycle,
lcycle, now) != lcycle));
return now;
}