linux-2.4.23-pre9_ia64-cyclone_A0

[Resent to the proper mailing list.]

Hi all,
This patch was developed for the distros to support the cyclone time
source on IBM x455 systems. x455 systems are multi-node NUMA systems which suffer from unsynced ITCs, which can cause non-monotonically increasing results from gettimeofday(). Very similar code was implemented in the i386 arch to resolve the same issue on x440s.

This patch is against the ia64 bk (~2.4.23-pre9) tree. I don't have a 2.5 version ready yet, but I'm currently working to port this code to the to use the struct time_interpolator interface. So a 2.5 patch should follow soon.

Any comments or feedback would be appreciated.

thanks
-john

diff -Nru a/arch/ia64/config.in b/arch/ia64/config.in
@@ -119,6 +119,8 @@
 tristate '/proc/pal support' CONFIG_IA64_PALINFO
 tristate '/proc/efi/vars support' CONFIG_EFI_VARS
 
+bool 'Support Cyclone(EXA) Clock' CONFIG_IA64_CYCLONE
+
 bool 'Networking support' CONFIG_NET
 bool 'System V IPC' CONFIG_SYSVIPC
 bool 'BSD Process Accounting' CONFIG_BSD_PROCESS_ACCT

diff -Nru a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
@@ -26,5 +26,6 @@
 obj-$(CONFIG_SMP) += smp.o smpboot.o
 obj-$(CONFIG_IA64_MCA) += mca.o mca_asm.o
 obj-$(CONFIG_IA64_BRL_EMU) += brl_emu.o
+obj-$(CONFIG_IA64_CYCLONE) += cyclone.o
 
 include $(TOPDIR)/Rules.make

diff -Nru a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c
@@ -49,6 +49,7 @@
 #include <asm/page.h>
 #include <asm/system.h>
 #include <asm/numa.h>
+#include <asm/cyclone.h>
 
 
 #define PREFIX			"ACPI: "

@@ -201,6 +202,16 @@
 	return AE_OK;
 }
 
+/* Hook from generic ACPI tables.c */
+void __init acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+{
+	if (!strncmp(oem_id, "IBM", 3) &&
+	    (!strncmp(oem_table_id, "SERMOW", 6))){
+		/*Start cyclone clock*/
+		cyclone_setup(0);
+	}
+}
+
 #endif /* CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI_BOOT

@@ -430,6 +441,10 @@
 		ipi_base_addr = (unsigned long) ioremap(acpi_madt->lapic_address, 0);
 
 	printk(KERN_INFO PREFIX "Local APIC address 0x%lx\n", ipi_base_addr);
+
+	acpi_madt_oem_check(acpi_madt->header.oem_id, 
+		acpi_madt->header.oem_table_id);
+
 	return 0;
 }
 

diff -Nru a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
@@ -22,6 +22,7 @@
 #include <asm/ptrace.h>
 #include <asm/sal.h>
 #include <asm/system.h>
+#include <asm/cyclone.h>
 
 extern rwlock_t xtime_lock;
 extern unsigned long wall_jiffies;

@@ -68,6 +69,9 @@
 	unsigned long now, last_tick;
 #	define time_keeper_id	0	/* smp_processor_id() of time-keeper */
 
+	if(use_cyclone)
+		return do_gettimeoffset_cyclone() + lost * (1000000 / HZ);
+
 	last_tick = (cpu_data(time_keeper_id)->itm_next
 		     - (lost + 1)*cpu_data(time_keeper_id)->itm_delta);
 

@@ -178,6 +182,8 @@
 			write_lock(&xtime_lock);
 			do_timer(regs);
 			local_cpu_data->itm_next = new_itm;
+			if(use_cyclone)
+				mark_timeoffset_cyclone();
 			write_unlock(&xtime_lock);
 		} else
 			local_cpu_data->itm_next = new_itm;

@@ -293,4 +299,6 @@
 	register_percpu_irq(IA64_TIMER_VECTOR, &timer_irqaction);
 	efi_gettimeofday((struct timeval *) &xtime);
 	ia64_init_itm();
+	if(use_cyclone)
+		init_cyclone_clock();
 }

diff -Nru a/include/asm-ia64/cyclone.h b/include/asm-ia64/cyclone.h
@@ -0,0 +1,22 @@
+#ifndef ASM_IA64_CYCLONE_H
+#define ASM_IA64_CYCLONE_H
+
+#ifdef	CONFIG_IA64_CYCLONE
+extern int use_cyclone;
+extern int __init cyclone_setup(char*);
+extern unsigned long do_gettimeoffset_cyclone(void);
+extern void mark_timeoffset_cyclone(void);
+extern void __init init_cyclone_clock(void);
+#else	/* CONFIG_IA64_CYCLONE */
+#define use_cyclone 0
+static inline void cyclone_setup(char* s) 
+{
+	printk(KERN_ERR "Cyclone Counter: System not configured"
+					" w/ CONFIG_IA64_CYCLONE.\n");
+}
+static inline unsigned long do_gettimeoffset_cyclone(void){}
+static inline void mark_timeoffset_cyclone(void){}
+static inline void init_cyclone_clock(void){}
+#endif	/* CONFIG_IA64_CYCLONE */
+
+#endif	/* !ASM_IA64_CYCLONE_H */

[Resent to the proper mailing list]

Yep, I'm looking at exactly that for my 2.5 patch. But thanks for the pointer.

Hmmm. I think I've grasped the issue there. It seems the problem is not calculating the equivalent of delay_at_last_interrupt found in the i386 time code. I'll see if I cannot come up with something similar.

Also I'd definitely like to share more infrastructure w/ the SN2 folks, is there a pointer to their 2.4 gettimeoffset() patch around?

thanks
-john

Actually, on second though, I don't believe this is necessary as every tick we increment last_tick_cyclone by one tick, rather then zeroing it out. In that way, even if the interrupt is delayed we don't lose time.

I may be missing some subtlety, but I think the point made above was considered in my patch.

thanks
-john

OK, that's good.

Apart from that correctness question, I have some concerns about how the code is structured. I don't think I've seen the actual SGI patch either, but based on John Hawkes' email (URL above), the hook looks something like:

+ if (ia64_platform_timer_interrupt)
+ (*ia64_platform_timer_interrupt)();

That's far better than adding stuff like this:

+ if(use_cyclone)
+ return do_gettimeoffset_cyclone() + lost * (1000000 / HZ);

because the former is much more generic.

I also don't like adding asm-ia64/cyclone.c and ia64/kernel/cyclone.c. Those files are machine-specific and don't belong in the generic ia64 area. (In fact, they look functionally identical to the i386 code; could this all be consolidated in something under drivers/ and shared?)

Bjorn

Very much agreed.

My plan is to do exactly that (and a bit more) for 2.7. As for 2.4 and 2.5, I'm a bit more hesitant about consolidating the code across arches. Outside the constants and init function not much is really shared (although they are structurally similar), and there are still some subtle differences between those (casting changes and fixmap vs ioremap). At this point I'd like to avoid the code churn in the i386 dir. However I'd be happy to more the ia64 bits into drivers/ as you suggest.

thanks for the feedback!
-john