* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
@ 2004-02-13 0:14 ` David Mosberger
2004-02-13 1:05 ` john stultz
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: David Mosberger @ 2004-02-13 0:14 UTC (permalink / raw)
To: linux-ia64
>>>>> On Wed, 11 Feb 2004 19:22:21 -0800, john stultz <johnstul@us.ibm.com> said:
John> All, This patch provides access to the cyclone time source
John> found on IBM EXA based systems (x450 and x455). This is needed
John> on multi-node systems where the CPU ITCs are not synchronized,
John> causing possible time inconsistencies.
John> I tried my best to properly follow the time_interpolator
John> interface, but please let me know if you see any mistakes.
John> Any comments or suggestions would be greatly appreciated.
It looks mostly OK to me. Some comments:
- It seems to me cyclone.c should check whether the SAL ITCdrift flag
is on and register the cyclone time-interpolator only if it is
turned on.
- When accessing iomemory space, please use readX()/writeX(). For
example:
+ offset = cyclone_timer[0];
should be:
offset = readl (cyclone_timer);
- The formatting is a bit weird. You can format it for 100 columns
wide and it would be nice if you could drop all the trailing
whitespace.
- This:
+ u32 offset;
[snip]
+ offset = (offset*NSEC_PER_SEC)/CYCLONE_TIMER_FREQ;
will overflow in about 4.3 seconds. Not likely to happen but
it might be safer to declare "offset" as "u64".
--david
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
2004-02-13 0:14 ` David Mosberger
@ 2004-02-13 1:05 ` john stultz
2004-02-13 1:16 ` David Mosberger
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: john stultz @ 2004-02-13 1:05 UTC (permalink / raw)
To: linux-ia64
On Thu, 2004-02-12 at 16:14, David Mosberger wrote:
> >>>>> On Wed, 11 Feb 2004 19:22:21 -0800, john stultz <johnstul@us.ibm.com> said:
>
> John> All, This patch provides access to the cyclone time source
> John> found on IBM EXA based systems (x450 and x455). This is needed
> John> on multi-node systems where the CPU ITCs are not synchronized,
> John> causing possible time inconsistencies.
>
> John> I tried my best to properly follow the time_interpolator
> John> interface, but please let me know if you see any mistakes.
>
> John> Any comments or suggestions would be greatly appreciated.
>
> It looks mostly OK to me. Some comments:
>
> - It seems to me cyclone.c should check whether the SAL ITCdrift flag
> is on and register the cyclone time-interpolator only if it is
> turned on.
Hmm. As cyclone is only enabled for x450/x455 systems, I'm not sure if
that additional check is necessary, but I'll try to look into that.
> - When accessing iomemory space, please use readX()/writeX(). For
> example:
>
> + offset = cyclone_timer[0];
>
> should be:
>
> offset = readl (cyclone_timer);
Ok, will fix.
> - The formatting is a bit weird. You can format it for 100 columns
> wide and it would be nice if you could drop all the trailing
> whitespace.
Gah! Sorry, I'm terrible w/ whitespace.
> - This:
> + u32 offset;
> [snip]
> + offset = (offset*NSEC_PER_SEC)/CYCLONE_TIMER_FREQ;
> will overflow in about 4.3 seconds. Not likely to happen but
> it might be safer to declare "offset" as "u64".
Yep, I caught that right after I sent out the patch.
Thanks so much for the review! I'll apply your suggestions and resend.
thanks again,
-john
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
2004-02-13 0:14 ` David Mosberger
2004-02-13 1:05 ` john stultz
@ 2004-02-13 1:16 ` David Mosberger
2004-02-13 1:41 ` Chris McDermott
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: David Mosberger @ 2004-02-13 1:16 UTC (permalink / raw)
To: linux-ia64
>>>>> On Thu, 12 Feb 2004 17:05:50 -0800, john stultz <johnstul@us.ibm.com> said:
John> Hmm. As cyclone is only enabled for x450/x455 systems, I'm not
John> sure if that additional check is necessary, but I'll try to
John> look into that.
At the very least, verify that the SAL ITCdrift bit is turned on by
the firmware. Otherwise, the lightweight handler of gettimeofday()
will still use ITC-based interpolation.
--david
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (2 preceding siblings ...)
2004-02-13 1:16 ` David Mosberger
@ 2004-02-13 1:41 ` Chris McDermott
2004-02-13 2:07 ` David Mosberger
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Chris McDermott @ 2004-02-13 1:41 UTC (permalink / raw)
To: linux-ia64
On Thu, Feb 12, 2004 at 17:16 -0800, David Mosberger wrote:
> >>>>> On Thu, 12 Feb 2004 17:05:50 -0800, john stultz <johnstul@us.ibm.com> said:
>
> John> Hmm. As cyclone is only enabled for x450/x455 systems, I'm not
> John> sure if that additional check is necessary, but I'll try to
> John> look into that.
>
> At the very least, verify that the SAL ITCdrift bit is turned on by
> the firmware. Otherwise, the lightweight handler of gettimeofday()
> will still use ITC-based interpolation.
>
> --david
> -
David,
The x450/x455 doesn't set/implement the SAL ITC drift bit. Could you point
us to where this is documentated? The latest SAL spec (Dec 2003) didn't
have reference to this flag being implemented in the Platform Features
Descriptor Entry in the SST.
thanks,
--
-chris
Chris McDermott
IBM Linux Technology Center (LTC)
xSeries Platform Enablement
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (3 preceding siblings ...)
2004-02-13 1:41 ` Chris McDermott
@ 2004-02-13 2:07 ` David Mosberger
2004-02-13 2:29 ` john stultz
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: David Mosberger @ 2004-02-13 2:07 UTC (permalink / raw)
To: linux-ia64
>>>>> On Thu, 12 Feb 2004 17:41:44 -0800, Chris McDermott <lcm@us.ibm.com> said:
Chris> The x450/x455 doesn't set/implement the SAL ITC drift
Chris> bit. Could you point us to where this is documentated? The
Chris> latest SAL spec (Dec 2003) didn't have reference to this flag
Chris> being implemented in the Platform Features Descriptor Entry
Chris> in the SST.
Hmmh, you're right: I don't see it either in the public documentation.
I got confirmation from Intel on 8 Nov 2001 that bit 3 of the
platform-feature vector will be defined as the ITC_DRIFT bit and the
SAL documentation was supposed to have been updated Q1 2002. Looks to
me like something got lost along the way. Can someone from Intel
clarify?
--david
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (4 preceding siblings ...)
2004-02-13 2:07 ` David Mosberger
@ 2004-02-13 2:29 ` john stultz
2004-02-13 7:09 ` David Mosberger
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: john stultz @ 2004-02-13 2:29 UTC (permalink / raw)
To: linux-ia64
On Thu, 2004-02-12 at 17:16, David Mosberger wrote:
> >>>>> On Thu, 12 Feb 2004 17:05:50 -0800, john stultz <johnstul@us.ibm.com> said:
>
> John> Hmm. As cyclone is only enabled for x450/x455 systems, I'm not
> John> sure if that additional check is necessary, but I'll try to
> John> look into that.
>
> At the very least, verify that the SAL ITCdrift bit is turned on by
> the firmware. Otherwise, the lightweight handler of gettimeofday()
> will still use ITC-based interpolation.
I'm not sure I see how gettimeofday will use ITC based interpolation if
the ITCdrift is off. Sure, the ITC will register a timesource, but
init_cyclone_clock runs after time_init and has a negative drift value,
so on the x450/x455 the cyclone interpolator will be used.
Am I missing something?
thanks
-john
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (5 preceding siblings ...)
2004-02-13 2:29 ` john stultz
@ 2004-02-13 7:09 ` David Mosberger
2004-02-13 18:55 ` Mallick, Asit K
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: David Mosberger @ 2004-02-13 7:09 UTC (permalink / raw)
To: linux-ia64
>>>>> On Thu, 12 Feb 2004 18:29:35 -0800, john stultz <johnstul@us.ibm.com> said:
John> I'm not sure I see how gettimeofday will use ITC based
John> interpolation if the ITCdrift is off. Sure, the ITC will
John> register a timesource, but init_cyclone_clock runs after
John> time_init and has a negative drift value, so on the x450/x455
John> the cyclone interpolator will be used.
John> Am I missing something?
Yes: it's the _lightweight_ gettimeofday() handler that will get
you. See arch/ia64/kernel/fsys.S:fsys_gettimeofday().
To date no distribution ships with light-weight syscall supported
enabled, but the support is in the current glibc, so it's only a
matter of time before this would start to bite.
--david
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (6 preceding siblings ...)
2004-02-13 7:09 ` David Mosberger
@ 2004-02-13 18:55 ` Mallick, Asit K
2004-02-17 2:46 ` john stultz
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Mallick, Asit K @ 2004-02-13 18:55 UTC (permalink / raw)
To: linux-ia64
>Hmmh, you're right: I don't see it either in the public documentation.
>I got confirmation from Intel on 8 Nov 2001 that bit 3 of the
>platform-feature vector will be defined as the ITC_DRIFT bit and the
>SAL documentation was supposed to have been updated Q1 2002. Looks to
>me like something got lost along the way. Can someone from Intel
>clarify?
>
We are checking with the SAL spec owners and let you know.
Thanks,
Asit
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (7 preceding siblings ...)
2004-02-13 18:55 ` Mallick, Asit K
@ 2004-02-17 2:46 ` john stultz
2004-02-17 21:41 ` David Mosberger
2004-02-18 5:20 ` [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A1 john stultz
10 siblings, 0 replies; 16+ messages in thread
From: john stultz @ 2004-02-17 2:46 UTC (permalink / raw)
To: linux-ia64
On Thu, 2004-02-12 at 23:09, David Mosberger wrote:
> >>>>> On Thu, 12 Feb 2004 18:29:35 -0800, john stultz <johnstul@us.ibm.com> said:
>
> John> I'm not sure I see how gettimeofday will use ITC based
> John> interpolation if the ITCdrift is off. Sure, the ITC will
> John> register a timesource, but init_cyclone_clock runs after
> John> time_init and has a negative drift value, so on the x450/x455
> John> the cyclone interpolator will be used.
>
> John> Am I missing something?
>
> Yes: it's the _lightweight_ gettimeofday() handler that will get
> you. See arch/ia64/kernel/fsys.S:fsys_gettimeofday().
>
> To date no distribution ships with light-weight syscall supported
> enabled, but the support is in the current glibc, so it's only a
> matter of time before this would start to bite.
Ah! Thanks for the pointer! I wouldn't have caught that otherwise.
So, as it seems the hardware currently doesn't set the ITCdrift flag,
should I then set it in my init code? Or is there some other way of
disabling fsys support, possibly on a per-syscall basis?
thanks
-john
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (8 preceding siblings ...)
2004-02-17 2:46 ` john stultz
@ 2004-02-17 21:41 ` David Mosberger
2004-02-18 5:20 ` [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A1 john stultz
10 siblings, 0 replies; 16+ messages in thread
From: David Mosberger @ 2004-02-17 21:41 UTC (permalink / raw)
To: linux-ia64
>>>>> On Mon, 16 Feb 2004 18:46:58 -0800, john stultz <johnstul@us.ibm.com> said:
John> So, as it seems the hardware currently doesn't set the
John> ITCdrift flag, should I then set it in my init code?
As a temporary workaround, that seems like a reasonable solution.
John> Or is there some other way of disabling fsys support, possibly
John> on a per-syscall basis?
You can't disable fsys-support itself, but you can turn off the use of
light-weight handlers by booting the kernel with option "nolwsys". I
wouldn't recommend that though because it's a sledgehammer and will
disable all other lightweight syscalls as well (getpid, getppid,
set_tid_address, and sigprocmask, for now).
--david
^ permalink raw reply [flat|nested] 16+ messages in thread* [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A1
2004-02-12 3:22 [RFC][PATCH] linux-2.6.3-rc2_ia64-cyclone_A0 john stultz
` (9 preceding siblings ...)
2004-02-17 21:41 ` David Mosberger
@ 2004-02-18 5:20 ` john stultz
2004-02-18 6:42 ` possible time_interpolator bug? john stultz
10 siblings, 1 reply; 16+ messages in thread
From: john stultz @ 2004-02-18 5:20 UTC (permalink / raw)
To: linux-ia64
All,
This patch provides access to the cyclone time source found on
IBM EXA based systems (x450 and x455). This is needed on multi-node
systems where the CPU ITCs are not synchronized, causing possible time
inconsistencies.
Since the last release, I've implemented a number of suggested cleanups
and fixes, but please let me know if you have any additional suggestions
or comments.
thanks
-john
diff -Nru a/arch/ia64/Kconfig b/arch/ia64/Kconfig
--- a/arch/ia64/Kconfig Tue Feb 17 21:16:09 2004
+++ b/arch/ia64/Kconfig Tue Feb 17 21:16:09 2004
@@ -245,6 +245,12 @@
Say Y here to enable machine check support for IA-64. If you're
unsure, answer Y.
+config IA64_CYCLONE
+ bool "Support Cyclone(EXA) Time Source"
+ help
+ Say Y here to enable support for IBM EXA Cyclone time source.
+ If you're unsure, answer N.
+
config PM
bool "Power Management support"
depends on IA64_GENERIC || IA64_DIG || IA64_HP_ZX1
diff -Nru a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
--- a/arch/ia64/kernel/Makefile Tue Feb 17 21:16:09 2004
+++ b/arch/ia64/kernel/Makefile Tue Feb 17 21:16:09 2004
@@ -18,6 +18,7 @@
obj-$(CONFIG_MODULES) += module.o
obj-$(CONFIG_SMP) += smp.o smpboot.o
obj-$(CONFIG_PERFMON) += perfmon_default_smpl.o
+obj-$(CONFIG_IA64_CYCLONE) += cyclone.o
# The gate DSO image is built using a special linker script.
targets += gate.so gate-syms.o
diff -Nru a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c
--- a/arch/ia64/kernel/acpi.c Tue Feb 17 21:16:09 2004
+++ b/arch/ia64/kernel/acpi.c Tue Feb 17 21:16:09 2004
@@ -49,6 +49,8 @@
#include <asm/page.h>
#include <asm/system.h>
#include <asm/numa.h>
+#include <asm/sal.h>
+#include <asm/cyclone.h>
#define PREFIX "ACPI: "
@@ -304,6 +306,22 @@
return 0;
}
+/* 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))){
+
+ /* Unfortunatly ITC_DRIFT is not yet part of the
+ * official SAL spec, so the ITC_DRIFT bit is not
+ * set by the BIOS on this hardware.
+ */
+ sal_platform_features |= IA64_SAL_PLATFORM_FEATURE_ITC_DRIFT;
+
+ /*Start cyclone clock*/
+ cyclone_setup(0);
+ }
+}
static int __init
acpi_parse_madt (unsigned long phys_addr, unsigned long size)
@@ -327,6 +345,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/cyclone.c b/arch/ia64/kernel/cyclone.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/arch/ia64/kernel/cyclone.c Tue Feb 17 21:16:09 2004
@@ -0,0 +1,156 @@
+#include <linux/smp.h>
+#include <linux/time.h>
+#include <linux/errno.h>
+
+/* IBM Summit (EXA) Cyclone counter code*/
+#define CYCLONE_CBAR_ADDR 0xFEB00CD0
+#define CYCLONE_PMCC_OFFSET 0x51A0
+#define CYCLONE_MPMC_OFFSET 0x51D0
+#define CYCLONE_MPCS_OFFSET 0x51A8
+#define CYCLONE_TIMER_FREQ 100000000
+
+int use_cyclone;
+int __init cyclone_setup(char *str)
+{
+ use_cyclone = 1;
+ return 1;
+}
+
+static u32* volatile cyclone_timer; /* Cyclone MPMC0 register */
+static u32 last_update_cyclone;
+
+static unsigned long offset_base;
+
+static unsigned long get_offset_cyclone(void)
+{
+ u32 now;
+ unsigned long offset;
+
+ /* Read the cyclone timer */
+ now = readl(cyclone_timer);
+ /* .. relative to previous update*/
+ offset = now - last_update_cyclone;
+
+ /* convert cyclone ticks to nanoseconds */
+ offset = (offset*NSEC_PER_SEC)/CYCLONE_TIMER_FREQ;
+
+ /* our adjusted time in nanoseconds */
+ return offset_base + (unsigned long)offset;
+}
+
+static void update_cyclone(long delta_nsec)
+{
+ u32 now;
+ unsigned long offset;
+
+ /* Read the cyclone timer */
+ now = readl(cyclone_timer);
+ /* .. relative to previous update*/
+ offset = now - last_update_cyclone;
+
+ /* convert cyclone ticks to nanoseconds */
+ offset = (offset*NSEC_PER_SEC)/CYCLONE_TIMER_FREQ;
+
+ /* Be careful about signed/unsigned comparisons here: */
+ if (delta_nsec < 0 || (unsigned long) delta_nsec < offset)
+ offset_base = offset_base + offset - delta_nsec;
+ else
+ offset_base = 0;
+
+ last_update_cyclone = now;
+}
+
+static void reset_cyclone(void)
+{
+ offset_base = 0;
+ last_update_cyclone = readl(cyclone_timer);
+}
+
+struct time_interpolator cyclone_interpolator = {
+ .get_offset = get_offset_cyclone,
+ .update = update_cyclone,
+ .reset = reset_cyclone,
+ .frequency = CYCLONE_TIMER_FREQ,
+ .drift = -100,
+};
+
+int __init init_cyclone_clock(void)
+{
+ u64* reg;
+ u64 base; /* saved cyclone base address */
+ u64 offset; /* offset from pageaddr to cyclone_timer register */
+ int i;
+
+ if (!use_cyclone)
+ return -ENODEV;
+
+ printk(KERN_INFO "Summit chipset: Starting Cyclone Counter.\n");
+
+ /* find base address */
+ offset = (CYCLONE_CBAR_ADDR);
+ reg = (u64*)ioremap_nocache(offset, sizeof(u64));
+ if(!reg){
+ printk(KERN_ERR "Summit chipset: Could not find valid CBAR register.\n");
+ use_cyclone = 0;
+ return -ENODEV;
+ }
+ base = readq(reg);
+ if(!base){
+ printk(KERN_ERR "Summit chipset: Could not find valid CBAR value.\n");
+ use_cyclone = 0;
+ return -ENODEV;
+ }
+ iounmap(reg);
+
+ /* setup PMCC */
+ offset = (base + CYCLONE_PMCC_OFFSET);
+ reg = (u64*)ioremap_nocache(offset, sizeof(u64));
+ if(!reg){
+ printk(KERN_ERR "Summit chipset: Could not find valid PMCC register.\n");
+ use_cyclone = 0;
+ return -ENODEV;
+ }
+ writel(0x00000001,reg);
+ iounmap(reg);
+
+ /* setup MPCS */
+ offset = (base + CYCLONE_MPCS_OFFSET);
+ reg = (u64*)ioremap_nocache(offset, sizeof(u64));
+ if(!reg){
+ printk(KERN_ERR "Summit chipset: Could not find valid MPCS register.\n");
+ use_cyclone = 0;
+ return -ENODEV;
+ }
+ writel(0x00000001,reg);
+ iounmap(reg);
+
+ /* map in cyclone_timer */
+ offset = (base + CYCLONE_MPMC_OFFSET);
+ cyclone_timer = (u32*)ioremap_nocache(offset, sizeof(u32));
+ if(!cyclone_timer){
+ printk(KERN_ERR "Summit chipset: Could not find valid MPMC register.\n");
+ use_cyclone = 0;
+ return -ENODEV;
+ }
+
+ /*quick test to make sure its ticking*/
+ for(i=0; i<3; i++){
+ u32 old = readl(cyclone_timer);
+ int stall = 100;
+ while(stall--) barrier();
+ if(readl(cyclone_timer) = old){
+ printk(KERN_ERR "Summit chipset: Counter not counting! DISABLED\n");
+ iounmap(cyclone_timer);
+ cyclone_timer = 0;
+ use_cyclone = 0;
+ return -ENODEV;
+ }
+ }
+ /* initialize last tick */
+ last_update_cyclone = readl(cyclone_timer);
+ register_time_interpolator(&cyclone_interpolator);
+
+ return 0;
+}
+
+__initcall(init_cyclone_clock);
diff -Nru a/include/asm-ia64/cyclone.h b/include/asm-ia64/cyclone.h
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/include/asm-ia64/cyclone.h Tue Feb 17 21:16:09 2004
@@ -0,0 +1,15 @@
+#ifndef ASM_IA64_CYCLONE_H
+#define ASM_IA64_CYCLONE_H
+
+#ifdef CONFIG_IA64_CYCLONE
+extern int use_cyclone;
+extern int __init cyclone_setup(char*);
+#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");
+}
+#endif /* CONFIG_IA64_CYCLONE */
+#endif /* !ASM_IA64_CYCLONE_H */
^ permalink raw reply [flat|nested] 16+ messages in thread* possible time_interpolator bug?
@ 2004-02-18 6:42 ` john stultz
2004-02-18 6:58 ` David Mosberger
0 siblings, 1 reply; 16+ messages in thread
From: john stultz @ 2004-02-18 6:42 UTC (permalink / raw)
To: linux-ia64
All,
Using my ia64-cyclone patch I've been occasionally noticing time
inconsistencies near second overflows. Being as the whole purpose of the
patch is to avoid time inconsistencies, I've been wearing my forehead
raw trying to find the bug in my implementation of the cyclone time
interpolator.
I believe I have found either a bug in the time_interpolator code, or a
bug in my understanding of how it is supposed to work.
Looking in kernel/timer.c at update_wall_time_one_tick() and
update_wall_time().
First in update_wall_time():
static void update_wall_time(unsigned long ticks)
{
do {
ticks--;
update_wall_time_one_tick();
} while (ticks);
if (xtime.tv_nsec >= 1000000000) {
xtime.tv_nsec -= 1000000000;
xtime.tv_sec++;
time_interpolator_update(NSEC_PER_SEC);
second_overflow();
}
}
So each second, we call "_update(NSEC_PER_SEC)", however for each tick
we call update_wall_time_one_tick() which calls "_update(delta_nsec)".
Thus in a period of one second, we call _update() HZ times w/
~NSEC_PER_SEC/HZ as an argument, then at the second overflow we call it
again w/ NSEC_PER_SEC.
It seems that calling _update() on the second overflow is unnecessary.
And because in my implementation of update_cyclone(), we then clear the
offset_base variable, this causes time inconsistencies.
It would be noted that time inconsistencies are not seen using the ITC
interpolator, however the ITC interpolator has an empty _update
function.
Am I just missing something here?
thanks
-john
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: possible time_interpolator bug?
2004-02-18 6:42 ` possible time_interpolator bug? john stultz
@ 2004-02-18 6:58 ` David Mosberger
2004-02-23 19:44 ` [PATCH] linux-2.6.3_time-interpolator-fix_A0 john stultz
0 siblings, 1 reply; 16+ messages in thread
From: David Mosberger @ 2004-02-18 6:58 UTC (permalink / raw)
To: linux-ia64
>>>>> On Tue, 17 Feb 2004 22:42:54 -0800, john stultz <johnstul@us.ibm.com> said:
John> So each second, we call "_update(NSEC_PER_SEC)", however for
John> each tick we call update_wall_time_one_tick() which calls
John> "_update(delta_nsec)".
John> Thus in a period of one second, we call _update() HZ times w/
John> ~NSEC_PER_SEC/HZ as an argument, then at the second overflow
John> we call it again w/ NSEC_PER_SEC.
John> It seems that calling _update() on the second overflow is
John> unnecessary.
Not just unnecessary, but wrong!
John> It would be noted that time inconsistencies are not seen using
John> the ITC interpolator, however the ITC interpolator has an
John> empty _update function.
Yes.
John> Am I just missing something here?
I don't think so. Good bug hunting. Some time ago SGI reported some
jumpiness but I never got a definite answer whether the problem had
been solved.
You may want to run this by someone who actually understands the code
in timer.c but I'm pretty sure you're right.
Thanks,
--david
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] linux-2.6.3_time-interpolator-fix_A0
2004-02-18 6:58 ` David Mosberger
@ 2004-02-23 19:44 ` john stultz
2004-02-23 19:55 ` David Mosberger
0 siblings, 1 reply; 16+ messages in thread
From: john stultz @ 2004-02-23 19:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: Chris McDermott, ia64, lkml, David Mosberger
[Resending to Andrew and LKML]
All,
In developing the ia64-cyclone patch, which implements a cyclone based
time interpolator, I found the following bug which could cause time
inconsistencies.
In update_wall_time_one_tick(), which is called each timer interrupt, we
call time_interpolator_update(delta_nsec) where delta_nsec is
approximately NSEC_PER_SEC/HZ. This directly correlates with the changes
to xtime which occurs in update_wall_time_one_tick().
However in update_wall_time(), on a second overflow, we again call
time_interpolator_update(NSEC_PER_SEC). However while the components of
xtime are being changed, the overall value of xtime does not (nsec is
decremented NSEC_PER_SEC and sec is incremented). Thus this call to
time_interpolator_update is incorrect.
This patch removes the incorrect call to time_interpolator_update and
was found to resolve the time inconsistencies I had seen while
developing the ia64-cyclone patch.
Please consider for inclusion.
diff -Nru a/kernel/timer.c b/kernel/timer.c
--- a/kernel/timer.c Wed Feb 18 12:04:03 2004
+++ b/kernel/timer.c Wed Feb 18 12:04:03 2004
@@ -677,7 +677,6 @@
if (xtime.tv_nsec >= 1000000000) {
xtime.tv_nsec -= 1000000000;
xtime.tv_sec++;
- time_interpolator_update(NSEC_PER_SEC);
second_overflow();
}
}
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] linux-2.6.3_time-interpolator-fix_A0
2004-02-23 19:44 ` [PATCH] linux-2.6.3_time-interpolator-fix_A0 john stultz
@ 2004-02-23 19:55 ` David Mosberger
0 siblings, 0 replies; 16+ messages in thread
From: David Mosberger @ 2004-02-23 19:55 UTC (permalink / raw)
To: john stultz; +Cc: Andrew Morton, Chris McDermott, ia64, lkml, David Mosberger
>>>>> On Mon, 23 Feb 2004 11:44:29 -0800, john stultz <johnstul@us.ibm.com> said:
John> This patch removes the incorrect call to
John> time_interpolator_update and was found to resolve the time
John> inconsistencies I had seen while developing the ia64-cyclone
John> patch.
John> Please consider for inclusion.
In case it isn't clear: this patch has my support.
--david
^ permalink raw reply [flat|nested] 16+ messages in thread