[PATCH 00/10] clocksource_register_khz/hz cleanups (for 2.6.38)

[PATCH 00/10] clocksource_register_khz/hz cleanups (for 2.6.38)

[John Stultz]

clocksource_register_hz/khz() properly calculates a 
clocksource mult and shift value for a clocksource,
removing much of the copy/pasted or arch-specific helper 
functions that do so.

The common x86 clocksources have been converted to use
clocksource_register_hz/clocksource_register_khz and I've
seen no related bug reports, so I think the infrastructure
is fairly sound. So I wanted to start consolidating the
other arches to also use the clocksource_register_hz/khz
method.

I was a bit too late sending some of these patches out for
the 2.6.37 cycle, so I wanted to send these out early for 
2.6.38.

While all of these patches are untested, they are pretty
straight forward, so please consider including these into
your 2.6.38 queue for testing.

thanks
-john


CC: Chris McDermott <lcm@us.ibm.com>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: Chris Zankel <chris@zankel.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Dimitri Sivanich <sivanich@sgi.com>
CC: Glauber Costa <glommer@redhat.com>
CC: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
CC: jacob.jun.pan@intel.com
CC: Jeremy Fitzhardinge <jeremy@xensource.com>
CC: Jesper Nilsson <jesper.nilsson@axis.com>
CC: Kyle McMartin <kyle@mcmartin.ca>
CC: Michal Simek <monstr@monstr.eu>
CC: Mikael Starvik <starvik@axis.com>
CC: Mike Frysinger <vapier@gentoo.org>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Tony Luck <tony.luck@intel.com>

John Stultz (10):
  x86: Convert untested clocksources to clocksource_register_hz/khz
  ia64: convert to clocksource_register_hz/khz
  cris: convert to clocksource_register_khz
  tile: convert to use clocksource_register_hz
  parisc: convert to clocksource_register_hz/khz
  microblaze: convert to clocksource_register_hz/khz
  avr32: Convert to clocksource_register_hz
  blackfin: convert to clocksource_register_hz
  xtensa: convert to clocksource_register_hz/khz
  sparc: convert to clocksource_register_hz/khz

 arch/avr32/kernel/time.c         |    5 +----
 arch/blackfin/kernel/time-ts.c   |   35 ++---------------------------------
 arch/cris/arch-v32/kernel/time.c |    4 +---
 arch/ia64/kernel/cyclone.c       |    6 +-----
 arch/ia64/kernel/time.c          |    9 ++-------
 arch/ia64/sn/kernel/sn2/timer.c  |    6 +-----
 arch/microblaze/kernel/timer.c   |    6 +-----
 arch/parisc/kernel/time.c        |    6 +-----
 arch/sparc/kernel/time_64.c      |    4 +---
 arch/tile/kernel/time.c          |    5 +----
 arch/x86/kernel/apb_timer.c      |   10 +---------
 arch/x86/kernel/i8253.c          |    6 +-----
 arch/x86/kernel/kvmclock.c       |    6 +-----
 arch/x86/lguest/boot.c           |    4 +---
 arch/x86/platform/uv/uv_time.c   |    6 +-----
 arch/x86/xen/time.c              |    6 +-----
 arch/xtensa/kernel/time.c        |   13 +------------
 drivers/char/hpet.c              |    6 +-----
 drivers/clocksource/cyclone.c    |   10 ++--------
 19 files changed, 22 insertions(+), 131 deletions(-)

-- 
1.7.3.2.146.gca209

[PATCH 01/10] x86: Convert untested clocksources to clocksource_register_hz/khz

[John Stultz]

This converts the remaining x86 clocksources to use
clocksource_register_hz/khz. I was not able to test them, so
any extra assistance to make sure time functions properly with
the clocksources changeed here would be appreciated!

CC: jacob.jun.pan@intel.com
CC: Glauber Costa <glommer@redhat.com>
CC: Dimitri Sivanich <sivanich@sgi.com>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: Jeremy Fitzhardinge <jeremy@xensource.com>
CC: Chris McDermott <lcm@us.ibm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/x86/kernel/apb_timer.c    |   10 +---------
 arch/x86/kernel/i8253.c        |    6 +-----
 arch/x86/kernel/kvmclock.c     |    6 +-----
 arch/x86/lguest/boot.c         |    4 +---
 arch/x86/platform/uv/uv_time.c |    6 +-----
 arch/x86/xen/time.c            |    6 +-----
 drivers/clocksource/cyclone.c  |   10 ++--------
 7 files changed, 8 insertions(+), 40 deletions(-)

diff --git a/arch/x86/kernel/apb_timer.c b/arch/x86/kernel/apb_timer.c
index 92543c7..2734428 100644
--- a/arch/x86/kernel/apb_timer.c
+++ b/arch/x86/kernel/apb_timer.c
@@ -177,7 +177,6 @@ static struct clocksource clocksource_apbt = {
 	.rating		= APBT_CLOCKSOURCE_RATING,
 	.read		= apbt_read_clocksource,
 	.mask		= APBT_MASK,
-	.shift		= APBT_SHIFT,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 	.resume		= apbt_restart_clocksource,
 };
@@ -592,14 +591,7 @@ static int apbt_clocksource_register(void)
 	if (t1 == apbt_read_clocksource(&clocksource_apbt))
 		panic("APBT counter not counting. APBT disabled\n");
 
-	/*
-	 * initialize and register APBT clocksource
-	 * convert that to ns/clock cycle
-	 * mult = (ns/c) * 2^APBT_SHIFT
-	 */
-	clocksource_apbt.mult = div_sc(MSEC_PER_SEC,
-				       (unsigned long) apbt_freq, APBT_SHIFT);
-	clocksource_register(&clocksource_apbt);
+	clocksource_register_khz(&clocksource_apbt,(u32)apbt_freq*1000);
 
 	return 0;
 }
diff --git a/arch/x86/kernel/i8253.c b/arch/x86/kernel/i8253.c
index 2dfd315..212fe65 100644
--- a/arch/x86/kernel/i8253.c
+++ b/arch/x86/kernel/i8253.c
@@ -188,8 +188,6 @@ static struct clocksource pit_cs = {
 	.rating		= 110,
 	.read		= pit_read,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.mult		= 0,
-	.shift		= 20,
 };
 
 static int __init init_pit_clocksource(void)
@@ -205,9 +203,7 @@ static int __init init_pit_clocksource(void)
 	    pit_ce.mode != CLOCK_EVT_MODE_PERIODIC)
 		return 0;
 
-	pit_cs.mult = clocksource_hz2mult(CLOCK_TICK_RATE, pit_cs.shift);
-
-	return clocksource_register(&pit_cs);
+	return clocksource_register_hz(&pit_cs, CLOCK_TICK_RATE);
 }
 arch_initcall(init_pit_clocksource);
 
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index ca43ce3..8e50e68 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -26,8 +26,6 @@
 #include <asm/x86_init.h>
 #include <asm/reboot.h>
 
-#define KVM_SCALE 22
-
 static int kvmclock = 1;
 static int msr_kvm_system_time = MSR_KVM_SYSTEM_TIME;
 static int msr_kvm_wall_clock = MSR_KVM_WALL_CLOCK;
@@ -120,8 +118,6 @@ static struct clocksource kvm_clock = {
 	.read = kvm_clock_get_cycles,
 	.rating = 400,
 	.mask = CLOCKSOURCE_MASK(64),
-	.mult = 1 << KVM_SCALE,
-	.shift = KVM_SCALE,
 	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -214,7 +210,7 @@ void __init kvmclock_init(void)
 	machine_ops.crash_shutdown  = kvm_crash_shutdown;
 #endif
 	kvm_get_preset_lpj();
-	clocksource_register(&kvm_clock);
+	clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
 	pv_info.paravirt_enabled = 1;
 	pv_info.name = "KVM";
 
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 73b1e1a..483e636 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -910,8 +910,6 @@ static struct clocksource lguest_clock = {
 	.rating		= 200,
 	.read		= lguest_clock_read,
 	.mask		= CLOCKSOURCE_MASK(64),
-	.mult		= 1 << 22,
-	.shift		= 22,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -994,7 +992,7 @@ static void lguest_time_init(void)
 	/* Set up the timer interrupt (0) to go to our simple timer routine */
 	set_irq_handler(0, lguest_time_irq);
 
-	clocksource_register(&lguest_clock);
+	clocksource_register_hz(&lguest_clock, NSEC_PER_SEC);
 
 	/* We can't set cpumask in the initializer: damn C limitations!  Set it
 	 * here and register our timer device. */
diff --git a/arch/x86/platform/uv/uv_time.c b/arch/x86/platform/uv/uv_time.c
index 56e421b..6f86d74 100644
--- a/arch/x86/platform/uv/uv_time.c
+++ b/arch/x86/platform/uv/uv_time.c
@@ -40,7 +40,6 @@ static struct clocksource clocksource_uv = {
 	.rating		= 400,
 	.read		= uv_read_rtc,
 	.mask		= (cycle_t)UVH_RTC_REAL_TIME_CLOCK_MASK,
-	.shift		= 10,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -370,14 +369,11 @@ static __init int uv_rtc_setup_clock(void)
 	if (!is_uv_system())
 		return -ENODEV;
 
-	clocksource_uv.mult = clocksource_hz2mult(sn_rtc_cycles_per_second,
-				clocksource_uv.shift);
-
 	/* If single blade, prefer tsc */
 	if (uv_num_possible_blades() == 1)
 		clocksource_uv.rating = 250;
 
-	rc = clocksource_register(&clocksource_uv);
+	rc = clocksource_register_hz(&clocksource_uv, sn_rtc_cycles_per_second);
 	if (rc)
 		printk(KERN_INFO "UV RTC clocksource failed rc %d\n", rc);
 	else
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index b2bb5aa..ce02ef8 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -26,8 +26,6 @@
 
 #include "xen-ops.h"
 
-#define XEN_SHIFT 22
-
 /* Xen may fire a timer up to this many ns early */
 #define TIMER_SLOP	100000
 #define NS_PER_TICK	(1000000000LL / HZ)
@@ -211,8 +209,6 @@ static struct clocksource xen_clocksource __read_mostly = {
 	.rating = 400,
 	.read = xen_clocksource_get_cycles,
 	.mask = ~0,
-	.mult = 1<<XEN_SHIFT,		/* time directly in nanoseconds */
-	.shift = XEN_SHIFT,
 	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -444,7 +440,7 @@ static __init void xen_time_init(void)
 	int cpu = smp_processor_id();
 	struct timespec tp;
 
-	clocksource_register(&xen_clocksource);
+	clocksource_register_hz(&xen_clocksource, NSEC_PER_SEC);
 
 	if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL) == 0) {
 		/* Successfully turned off 100Hz tick, so we have the
diff --git a/drivers/clocksource/cyclone.c b/drivers/clocksource/cyclone.c
index 64e528e..72f811f 100644
--- a/drivers/clocksource/cyclone.c
+++ b/drivers/clocksource/cyclone.c
@@ -29,8 +29,6 @@ static struct clocksource clocksource_cyclone = {
 	.rating		= 250,
 	.read		= read_cyclone,
 	.mask		= CYCLONE_TIMER_MASK,
-	.mult		= 10,
-	.shift		= 0,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -108,12 +106,8 @@ static int __init init_cyclone_clocksource(void)
 	}
 	cyclone_ptr = cyclone_timer;
 
-	/* sort out mult/shift values: */
-	clocksource_cyclone.shift = 22;
-	clocksource_cyclone.mult = clocksource_hz2mult(CYCLONE_TIMER_FREQ,
-						clocksource_cyclone.shift);
-
-	return clocksource_register(&clocksource_cyclone);
+	return clocksource_register_hz(&clocksource_cyclone,
+					CYCLONE_TIMER_FREQ);
 }
 
 arch_initcall(init_cyclone_clocksource);
-- 
1.7.3.2.146.gca209

[PATCH 02/10] ia64: convert to clocksource_register_hz/khz

[John Stultz]

This converts the ia64 clocksources to use clocksource_register_hz/khz

This is untested, so any assistance in testing would be appreciated!

CC: Tony Luck <tony.luck@intel.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/ia64/kernel/cyclone.c      |    6 +-----
 arch/ia64/kernel/time.c         |    9 ++-------
 arch/ia64/sn/kernel/sn2/timer.c |    6 +-----
 drivers/char/hpet.c             |    6 +-----
 4 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/arch/ia64/kernel/cyclone.c b/arch/ia64/kernel/cyclone.c
index d52f1f7..f64097b 100644
--- a/arch/ia64/kernel/cyclone.c
+++ b/arch/ia64/kernel/cyclone.c
@@ -31,8 +31,6 @@ static struct clocksource clocksource_cyclone = {
         .rating         = 300,
         .read           = read_cyclone,
         .mask           = (1LL << 40) - 1,
-        .mult           = 0, /*to be caluclated*/
-        .shift          = 16,
         .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -118,9 +116,7 @@ int __init init_cyclone_clock(void)
 	/* initialize last tick */
 	cyclone_mc = cyclone_timer;
 	clocksource_cyclone.fsys_mmio = cyclone_timer;
-	clocksource_cyclone.mult = clocksource_hz2mult(CYCLONE_TIMER_FREQ,
-						clocksource_cyclone.shift);
-	clocksource_register(&clocksource_cyclone);
+	clocksource_register_hz(&clocksource_cyclone, CYCLONE_TIMER_FREQ);
 
 	return 0;
 }
diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index ed6f22e..4b981e5 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -73,8 +73,6 @@ static struct clocksource clocksource_itc = {
 	.rating         = 350,
 	.read           = itc_get_cycles,
 	.mask           = CLOCKSOURCE_MASK(64),
-	.mult           = 0, /*to be calculated*/
-	.shift          = 16,
 	.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 #ifdef CONFIG_PARAVIRT
 	.resume		= paravirt_clocksource_resume,
@@ -374,11 +372,8 @@ ia64_init_itm (void)
 	ia64_cpu_local_tick();
 
 	if (!itc_clocksource) {
-		/* Sort out mult/shift values: */
-		clocksource_itc.mult =
-			clocksource_hz2mult(local_cpu_data->itc_freq,
-						clocksource_itc.shift);
-		clocksource_register(&clocksource_itc);
+		clocksource_register_hz(&clocksource_itc,
+						local_cpu_data->itc_freq );
 		itc_clocksource = &clocksource_itc;
 	}
 }
diff --git a/arch/ia64/sn/kernel/sn2/timer.c b/arch/ia64/sn/kernel/sn2/timer.c
index 21d6f09..c34efda 100644
--- a/arch/ia64/sn/kernel/sn2/timer.c
+++ b/arch/ia64/sn/kernel/sn2/timer.c
@@ -33,8 +33,6 @@ static struct clocksource clocksource_sn2 = {
         .rating         = 450,
         .read           = read_sn2,
         .mask           = (1LL << 55) - 1,
-        .mult           = 0,
-        .shift          = 10,
         .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -57,9 +55,7 @@ ia64_sn_udelay (unsigned long usecs)
 void __init sn_timer_init(void)
 {
 	clocksource_sn2.fsys_mmio = RTC_COUNTER_ADDR;
-	clocksource_sn2.mult = clocksource_hz2mult(sn_rtc_cycles_per_second,
-							clocksource_sn2.shift);
-	clocksource_register(&clocksource_sn2);
+	clocksource_register_hz(&clocksource_sn2, sn_rtc_cycles_per_second);
 
 	ia64_udelay = &ia64_sn_udelay;
 }
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 55b8667..d938311 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -85,8 +85,6 @@ static struct clocksource clocksource_hpet = {
 	.rating		= 250,
 	.read		= read_hpet,
 	.mask		= CLOCKSOURCE_MASK(64),
-	.mult		= 0,		/* to be calculated */
-	.shift		= 10,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 static struct clocksource *hpet_clocksource;
@@ -935,9 +933,7 @@ int hpet_alloc(struct hpet_data *hdp)
 	if (!hpet_clocksource) {
 		hpet_mctr = (void __iomem *)&hpetp->hp_hpet->hpet_mc;
 		CLKSRC_FSYS_MMIO_SET(clocksource_hpet.fsys_mmio, hpet_mctr);
-		clocksource_hpet.mult = clocksource_hz2mult(hpetp->hp_tick_freq,
-						clocksource_hpet.shift);
-		clocksource_register(&clocksource_hpet);
+		clocksource_register_hz(&clocksource_hpet, hpetp->hp_tick_freq);
 		hpetp->hp_clocksource = &clocksource_hpet;
 		hpet_clocksource = &clocksource_hpet;
 	}
-- 
1.7.3.2.146.gca209

[PATCH 03/10] cris: convert to clocksource_register_khz

[John Stultz]

This converts the cris architecture to use clocksource_register_khz

This is untested, so any help from maintainers would be appreciated.

CC: Mikael Starvik <starvik@axis.com>
CC: Jesper Nilsson <jesper.nilsson@axis.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/cris/arch-v32/kernel/time.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/cris/arch-v32/kernel/time.c b/arch/cris/arch-v32/kernel/time.c
index a545211..64a1233 100644
--- a/arch/cris/arch-v32/kernel/time.c
+++ b/arch/cris/arch-v32/kernel/time.c
@@ -47,14 +47,12 @@ static struct clocksource cont_rotime = {
 	.rating = 300,
 	.read   = read_cont_rotime,
 	.mask   = CLOCKSOURCE_MASK(32),
-	.shift  = 10,
 	.flags  = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 static int __init etrax_init_cont_rotime(void)
 {
-	cont_rotime.mult = clocksource_khz2mult(100000, cont_rotime.shift);
-	clocksource_register(&cont_rotime);
+	clocksource_register_khz(&cont_rotime, 100000);
 	return 0;
 }
 arch_initcall(etrax_init_cont_rotime);
-- 
1.7.3.2.146.gca209

[PATCH 04/10] tile: convert to use clocksource_register_hz

[John Stultz]

Convert tile to use clocksource_register_hz.

Untested. Help from maintainers would be appreciated.

CC: Chris Metcalf <cmetcalf@tilera.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/tile/kernel/time.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/arch/tile/kernel/time.c b/arch/tile/kernel/time.c
index 6bed820..8c06cb2 100644
--- a/arch/tile/kernel/time.c
+++ b/arch/tile/kernel/time.c
@@ -76,7 +76,6 @@ static struct clocksource cycle_counter_cs = {
 	.rating = 300,
 	.read = clocksource_get_cycles,
 	.mask = CLOCKSOURCE_MASK(64),
-	.shift = 22,   /* typical value, e.g. x86 tsc uses this */
 	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -89,8 +88,6 @@ void __init setup_clock(void)
 	cycles_per_sec = hv_sysconf(HV_SYSCONF_CPU_SPEED);
 	sched_clock_mult =
 		clocksource_hz2mult(cycles_per_sec, SCHED_CLOCK_SHIFT);
-	cycle_counter_cs.mult =
-		clocksource_hz2mult(cycles_per_sec, cycle_counter_cs.shift);
 }
 
 void __init calibrate_delay(void)
@@ -105,7 +102,7 @@ void __init calibrate_delay(void)
 void __init time_init(void)
 {
 	/* Initialize and register the clock source. */
-	clocksource_register(&cycle_counter_cs);
+	clocksource_register_hz(&cycle_counter_cs, cycles_per_sec);
 
 	/* Start up the tile-timer interrupt source on the boot cpu. */
 	setup_tile_timer();
-- 
1.7.3.2.146.gca209

Re: [PATCH 04/10] tile: convert to use clocksource_register_hz

[Chris Metcalf]

On 11/1/2010 4:12 PM, John Stultz wrote:
> Convert tile to use clocksource_register_hz.
>
> Untested. Help from maintainers would be appreciated.
>
> CC: Chris Metcalf <cmetcalf@tilera.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>
> ---
>  arch/tile/kernel/time.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/arch/tile/kernel/time.c b/arch/tile/kernel/time.c
> index 6bed820..8c06cb2 100644
> --- a/arch/tile/kernel/time.c
> +++ b/arch/tile/kernel/time.c
> @@ -76,7 +76,6 @@ static struct clocksource cycle_counter_cs = {
>  	.rating = 300,
>  	.read = clocksource_get_cycles,
>  	.mask = CLOCKSOURCE_MASK(64),
> -	.shift = 22,   /* typical value, e.g. x86 tsc uses this */
>  	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
>  };

We were using clocksource_calc_mult_shift() for a while to compute both
this value and our sched_clock() value, and, like
clocksource_register_hz(), it suggested that a shift value of 31 was best.

Perhaps unsurprisingly, we saw wraparound with sched_clock(), which is why
we use a fixed shift value of "10" to avoid problems.  This is clearly
required since sched_clock() values are fairly long-lived.

I admit that I don't really understand what the wraparound implications of
using a shift of "31" for the clocksource is, but since it will cause the
clocksource to wrap around fairly quickly, I wanted to make sure this shift
value was OK for whatever uses that clocksource is put to.  (The underlying
clock in question is a 64-bit cycle counter, so with "<< 31" at e.g. 1 GHz
it will wrap negative after about four seconds.)

> @@ -89,8 +88,6 @@ void __init setup_clock(void)
>  	cycles_per_sec = hv_sysconf(HV_SYSCONF_CPU_SPEED);
>  	sched_clock_mult =
>  		clocksource_hz2mult(cycles_per_sec, SCHED_CLOCK_SHIFT);
> -	cycle_counter_cs.mult =
> -		clocksource_hz2mult(cycles_per_sec, cycle_counter_cs.shift);
>  }
>  
>  void __init calibrate_delay(void)
> @@ -105,7 +102,7 @@ void __init calibrate_delay(void)
>  void __init time_init(void)
>  {
>  	/* Initialize and register the clock source. */
> -	clocksource_register(&cycle_counter_cs);
> +	clocksource_register_hz(&cycle_counter_cs, cycles_per_sec);
>  
>  	/* Start up the tile-timer interrupt source on the boot cpu. */
>  	setup_tile_timer();
-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH 04/10] tile: convert to use clocksource_register_hz

[john stultz]

On Thu, 2010-11-11 at 16:21 -0500, Chris Metcalf wrote:
> On 11/1/2010 4:12 PM, John Stultz wrote:
> > Convert tile to use clocksource_register_hz.
> >
> > Untested. Help from maintainers would be appreciated.
> >
> > CC: Chris Metcalf <cmetcalf@tilera.com>
> > CC: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: John Stultz <johnstul@us.ibm.com>
> > ---
> >  arch/tile/kernel/time.c |    5 +----
> >  1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/tile/kernel/time.c b/arch/tile/kernel/time.c
> > index 6bed820..8c06cb2 100644
> > --- a/arch/tile/kernel/time.c
> > +++ b/arch/tile/kernel/time.c
> > @@ -76,7 +76,6 @@ static struct clocksource cycle_counter_cs = {
> >  	.rating = 300,
> >  	.read = clocksource_get_cycles,
> >  	.mask = CLOCKSOURCE_MASK(64),
> > -	.shift = 22,   /* typical value, e.g. x86 tsc uses this */
> >  	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
> >  };
> 
> We were using clocksource_calc_mult_shift() for a while to compute both
> this value and our sched_clock() value, and, like
> clocksource_register_hz(), it suggested that a shift value of 31 was best.
> 
> Perhaps unsurprisingly, we saw wraparound with sched_clock(), which is why
> we use a fixed shift value of "10" to avoid problems.  This is clearly
> required since sched_clock() values are fairly long-lived.

Heh. This is the third case *today* of issues cropping up due to
sched_clock utilizing the timekeeping clocksource. Interesting
confluence of events.

Obviously the interactions here need some clarification and cleanup. :)

> I admit that I don't really understand what the wraparound implications of
> using a shift of "31" for the clocksource is, but since it will cause the
> clocksource to wrap around fairly quickly, I wanted to make sure this shift
> value was OK for whatever uses that clocksource is put to.  (The underlying
> clock in question is a 64-bit cycle counter, so with "<< 31" at e.g. 1 GHz
> it will wrap negative after about four seconds.)

Its not so much an issue of wraparound of the clocksource, but
multiplication overflow when we are using long cycle deltas.

The shift calculation works backwards from an software defined max time
delta, converts it to a cycle delta, then reduces it if the cycle
counter's own wrap limit is smaller. It then calculates the largest
shift value possible where the resulting mult for the given freq won't
cause an overflow from that max cycle delta.

This gives us the best freq estimate, and finest grained adjustment
ability for that defined max time delta. That the max time delta needs
to be centrally managed, as it provides the trade-off between fine
grained ntp adjustments and the maximum NOHZ time, so we don't want to
chase issues surrounding these tradeoffs all over different
architectures.

Now, almost all of the clocksource shift calculation code has been done
with concern around how frequently we call update_wall_time, which
accumulates cycles from the clocksource (allowing the clocksource to
wrap safely without losing time). 

Not much thought on my part has gone into the sched_clock usage of the
clocksource. Folks saw a good way to access a counter and a way to
convert it to ns, so it seemed reasonable, but I don't think the proper
consideration was given to the requirements of the sched_clock behavior.

In many cases sched_clock used its own counter-extension hacks, or
per-cpu offset calculations to allow faster-non-timekeeping acceptable
clocksources (like the TSC on some machines) to be used. Its a bit more
of an arch specific thing, so there's more chances for it to go wrong
somewhere.

So some quick questions for Peter and Ingo to help clarify this:

1) How often is sched_clock guaranteed to be called? Once each tick, (so
the maximum time in nohz mode would be reasonable?)

2) What considerations for sched_clock wrapping is there in generic
code? I see some considerations in kernel/sched_clock.c, but its not
obvious the limits. On x86, the 64-bit TSC won't wrap (but might jump on
non-synced systems, or halt in idle modes). Do architectures that have
faster-wrapping counters need to handle the cycle accumulation
internally?

3) If there is generic code to handle counter wrapping, what is the
limit of the length of time that a old sched_clock value might be around
for before being updated? In other words, if there was a second granular
counter that wrapped every minute, you wouldn't want to compare two
values that were longer then a minute apart. So if that was scaled down
to counter frequencies, how long might it be between storing a
sched_clock reference and using it?


Chris: Thanks for pointing out this issue!

thanks
-john

Re: [PATCH 04/10] tile: convert to use clocksource_register_hz

[Peter Zijlstra]

On Thu, 2010-11-11 at 14:06 -0800, john stultz wrote:
> 1) How often is sched_clock guaranteed to be called? Once each tick, (so
> the maximum time in nohz mode would be reasonable?)

Never,.. sparc64 for example can stay in nohz mode for hours. We have a
nohz_exit hook for the kernel/sched_clock.c code though which resyncs us
against the GTOD.

> 2) What considerations for sched_clock wrapping is there in generic
> code? I see some considerations in kernel/sched_clock.c, but its not
> obvious the limits. On x86, the 64-bit TSC won't wrap (but might jump on
> non-synced systems, or halt in idle modes). Do architectures that have
> faster-wrapping counters need to handle the cycle accumulation
> internally? 

Basically all code assumes we wrap on the u64 boundary.

So the whole kernel/sched_clock.c machinery tries to make a crummy arch
sched_clock() usable, it syncs against the GTOD code (on tick, idle_exit
and nohz_exit) and only assumes the arch sched_clock() wraps at the u64
boundary, jumps, inter-cpu drift etc are all taken care of.

Re: [PATCH 04/10] tile: convert to use clocksource_register_hz

[john stultz]

On Thu, 2010-11-11 at 23:22 +0100, Peter Zijlstra wrote:
> On Thu, 2010-11-11 at 14:06 -0800, john stultz wrote:
> > 1) How often is sched_clock guaranteed to be called? Once each tick, (so
> > the maximum time in nohz mode would be reasonable?)
> 
> Never,.. sparc64 for example can stay in nohz mode for hours. We have a
> nohz_exit hook for the kernel/sched_clock.c code though which resyncs us
> against the GTOD.

Well, I suspect after hours in nohz, timekeeping might not be 100%
correct, unless a low enough shift value is used.

And that's part of the motivation for the clocksource_register_hz bits:
to consolidate assumptions about adjustment granularity and safe nohz
limits, so they can be tuned in a generic and clean fashion without
assumptions being made in the arch specific code.

> > 2) What considerations for sched_clock wrapping is there in generic
> > code? I see some considerations in kernel/sched_clock.c, but its not
> > obvious the limits. On x86, the 64-bit TSC won't wrap (but might jump on
> > non-synced systems, or halt in idle modes). Do architectures that have
> > faster-wrapping counters need to handle the cycle accumulation
> > internally? 
> 
> Basically all code assumes we wrap on the u64 boundary.
> 
> So the whole kernel/sched_clock.c machinery tries to make a crummy arch
> sched_clock() usable, it syncs against the GTOD code (on tick, idle_exit
> and nohz_exit) and only assumes the arch sched_clock() wraps at the u64
> boundary, jumps, inter-cpu drift etc are all taken care of.

So at some point it might be worth having a clocksource-like structure
to register for sched_clock and allowing generic code manage calculating
the cycles to ns conversion and accumulation method so we don't run into
arch specific issues.

That said, for now, I think it would be easiest to make sure the arch
specific sched_clock implementations don't mis-use the timekeeping logic
that is built for different assumptions.

It looks the tile folks have done the right thing, hopefully we can fix
the few other cases (like the lpj issue brought up earlier) fairly
easily in the arch code.

thanks
-john

Re: [PATCH 04/10] tile: convert to use clocksource_register_hz

[john stultz]

On Thu, 2010-11-11 at 16:21 -0500, Chris Metcalf wrote:
> On 11/1/2010 4:12 PM, John Stultz wrote:
> > Convert tile to use clocksource_register_hz.
> >
> > Untested. Help from maintainers would be appreciated.
> >
> > CC: Chris Metcalf <cmetcalf@tilera.com>
> > CC: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: John Stultz <johnstul@us.ibm.com>
> > ---
> >  arch/tile/kernel/time.c |    5 +----
> >  1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/tile/kernel/time.c b/arch/tile/kernel/time.c
> > index 6bed820..8c06cb2 100644
> > --- a/arch/tile/kernel/time.c
> > +++ b/arch/tile/kernel/time.c
> > @@ -76,7 +76,6 @@ static struct clocksource cycle_counter_cs = {
> >  	.rating = 300,
> >  	.read = clocksource_get_cycles,
> >  	.mask = CLOCKSOURCE_MASK(64),
> > -	.shift = 22,   /* typical value, e.g. x86 tsc uses this */
> >  	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
> >  };
> 
> We were using clocksource_calc_mult_shift() for a while to compute both
> this value and our sched_clock() value, and, like
> clocksource_register_hz(), it suggested that a shift value of 31 was best.
> 
> Perhaps unsurprisingly, we saw wraparound with sched_clock(), which is why
> we use a fixed shift value of "10" to avoid problems.  This is clearly
> required since sched_clock() values are fairly long-lived.

One more quick thing: I just reviewed the tile sched_clock code, and it
looks like its doing the right thing, calculating a mult/shift pair for
the sched_clock code itself, not directly using the timekeeping
clocksource's values.

So the clocksource_register_hz patch proposed here doesn't really affect
this issue and should still be fine to go in (assuming no other issues
are brought out by it :).

But Chris' point does still illustrate that we need better clarification
about what is expected from sched_clock and maybe some better reference
implementations to make sure arch maintainers don't trip on common
issues when implementing or modifying sched_clock.

thanks
-john

[PATCH 05/10] parisc: convert to clocksource_register_hz/khz

[John Stultz]

This converts the parisc clocksources to use clocksource_register_hz/khz

This is untested, so any assistance in testing would be appreciated!

CC: Kyle McMartin <kyle@mcmartin.ca>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/parisc/kernel/time.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 05511cc..8bfff4a 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -201,8 +201,6 @@ static struct clocksource clocksource_cr16 = {
 	.rating			= 300,
 	.read			= read_cr16,
 	.mask			= CLOCKSOURCE_MASK(BITS_PER_LONG),
-	.mult			= 0, /* to be set */
-	.shift			= 22,
 	.flags			= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -273,7 +271,5 @@ void __init time_init(void)
 
 	/* register at clocksource framework */
 	current_cr16_khz = PAGE0->mem_10msec/10;  /* kHz */
-	clocksource_cr16.mult = clocksource_khz2mult(current_cr16_khz,
-						clocksource_cr16.shift);
-	clocksource_register(&clocksource_cr16);
+	clocksource_register_khz(&clocksource_cr16, current_cr16_khz);
 }
-- 
1.7.3.2.146.gca209

[PATCH 06/10] microblaze: convert to clocksource_register_hz/khz

[John Stultz]

This converts the microblaze clocksources to use clocksource_register_hz/khz

This is untested, so any assistance in testing would be appreciated!

CC: Michal Simek <monstr@monstr.eu>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/microblaze/kernel/timer.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
index a5aa33d..68ec7d1 100644
--- a/arch/microblaze/kernel/timer.c
+++ b/arch/microblaze/kernel/timer.c
@@ -217,16 +217,12 @@ static struct clocksource clocksource_microblaze = {
 	.rating		= 300,
 	.read		= microblaze_read,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.shift		= 8, /* I can shift it */
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 static int __init microblaze_clocksource_init(void)
 {
-	clocksource_microblaze.mult =
-			clocksource_hz2mult(timer_clock_freq,
-						clocksource_microblaze.shift);
-	if (clocksource_register(&clocksource_microblaze))
+	if (clocksource_register_hz(&clocksource_microblaze, timer_clock_freq))
 		panic("failed to register clocksource");
 
 	/* stop timer1 */
-- 
1.7.3.2.146.gca209

Re: [PATCH 06/10] microblaze: convert to clocksource_register_hz/khz

[Michal Simek]

John Stultz wrote:
> This converts the microblaze clocksources to use clocksource_register_hz/khz
> 
> This is untested, so any assistance in testing would be appreciated!
> 
> CC: Michal Simek <monstr@monstr.eu>
> CC: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>
> ---
>  arch/microblaze/kernel/timer.c |    6 +-----
>  1 files changed, 1 insertions(+), 5 deletions(-)

I can't see any problem in it.

Tested-by: Michal Simek <monstr@monstr.eu>



> 
> diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
> index a5aa33d..68ec7d1 100644
> --- a/arch/microblaze/kernel/timer.c
> +++ b/arch/microblaze/kernel/timer.c
> @@ -217,16 +217,12 @@ static struct clocksource clocksource_microblaze = {
>  	.rating		= 300,
>  	.read		= microblaze_read,
>  	.mask		= CLOCKSOURCE_MASK(32),
> -	.shift		= 8, /* I can shift it */
>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
>  };
>  
>  static int __init microblaze_clocksource_init(void)
>  {
> -	clocksource_microblaze.mult =
> -			clocksource_hz2mult(timer_clock_freq,
> -						clocksource_microblaze.shift);
> -	if (clocksource_register(&clocksource_microblaze))
> +	if (clocksource_register_hz(&clocksource_microblaze, timer_clock_freq))
>  		panic("failed to register clocksource");
>  
>  	/* stop timer1 */


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

[PATCH 07/10] avr32: Convert to clocksource_register_hz

[John Stultz]

This converts the avr32 clocksource to use clocksource_register_hz.

This is untested, so any assistance in testing would be appreciated!

CC: Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/avr32/kernel/time.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/arch/avr32/kernel/time.c b/arch/avr32/kernel/time.c
index 668ed28..05ad291 100644
--- a/arch/avr32/kernel/time.c
+++ b/arch/avr32/kernel/time.c
@@ -35,7 +35,6 @@ static struct clocksource counter = {
 	.rating		= 50,
 	.read		= read_cycle_count,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.shift		= 16,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -123,9 +122,7 @@ void __init time_init(void)
 
 	/* figure rate for counter */
 	counter_hz = clk_get_rate(boot_cpu_data.clk);
-	counter.mult = clocksource_hz2mult(counter_hz, counter.shift);
-
-	ret = clocksource_register(&counter);
+	ret = clocksource_register_hz(&counter, counter_hz);
 	if (ret)
 		pr_debug("timer: could not register clocksource: %d\n", ret);
 
-- 
1.7.3.2.146.gca209

[PATCH 08/10] blackfin: convert to clocksource_register_hz

[John Stultz]

This converts the blackfin clocksource to use clocksource_register_hz.

This is untested, so any assistance in testing would be appreciated!

CC: Mike Frysinger <vapier@gentoo.org>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/blackfin/kernel/time-ts.c |   35 ++---------------------------------
 1 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/arch/blackfin/kernel/time-ts.c b/arch/blackfin/kernel/time-ts.c
index 8c9a43d..4a01371 100644
--- a/arch/blackfin/kernel/time-ts.c
+++ b/arch/blackfin/kernel/time-ts.c
@@ -23,29 +23,6 @@
 #include <asm/gptimers.h>
 #include <asm/nmi.h>
 
-/* Accelerators for sched_clock()
- * convert from cycles(64bits) => nanoseconds (64bits)
- *  basic equation:
- *		ns = cycles / (freq / ns_per_sec)
- *		ns = cycles * (ns_per_sec / freq)
- *		ns = cycles * (10^9 / (cpu_khz * 10^3))
- *		ns = cycles * (10^6 / cpu_khz)
- *
- *	Then we use scaling math (suggested by george@mvista.com) to get:
- *		ns = cycles * (10^6 * SC / cpu_khz) / SC
- *		ns = cycles * cyc2ns_scale / SC
- *
- *	And since SC is a constant power of two, we can convert the div
- *  into a shift.
- *
- *  We can use khz divisor instead of mhz to keep a better precision, since
- *  cyc2ns_scale is limited to 10^6 * 2^10, which fits in 32 bits.
- *  (mathieu.desnoyers@polymtl.ca)
- *
- *			-johnstul@us.ibm.com "math is hard, lets go shopping!"
- */
-
-#define CYC2NS_SCALE_FACTOR 10 /* 2^10, carefully chosen */
 
 #if defined(CONFIG_CYCLES_CLOCKSOURCE)
 
@@ -63,7 +40,6 @@ static struct clocksource bfin_cs_cycles = {
 	.rating		= 400,
 	.read		= bfin_read_cycles,
 	.mask		= CLOCKSOURCE_MASK(64),
-	.shift		= CYC2NS_SCALE_FACTOR,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -75,10 +51,7 @@ static inline unsigned long long bfin_cs_cycles_sched_clock(void)
 
 static int __init bfin_cs_cycles_init(void)
 {
-	bfin_cs_cycles.mult = \
-		clocksource_hz2mult(get_cclk(), bfin_cs_cycles.shift);
-
-	if (clocksource_register(&bfin_cs_cycles))
+	if (clocksource_register_hz(&bfin_cs_cycles, get_cclk()))
 		panic("failed to register clocksource");
 
 	return 0;
@@ -111,7 +84,6 @@ static struct clocksource bfin_cs_gptimer0 = {
 	.rating		= 350,
 	.read		= bfin_read_gptimer0,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.shift		= CYC2NS_SCALE_FACTOR,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
@@ -125,10 +97,7 @@ static int __init bfin_cs_gptimer0_init(void)
 {
 	setup_gptimer0();
 
-	bfin_cs_gptimer0.mult = \
-		clocksource_hz2mult(get_sclk(), bfin_cs_gptimer0.shift);
-
-	if (clocksource_register(&bfin_cs_gptimer0))
+	if (clocksource_register_hz(&bfin_cs_gptimer0, get_sclk()))
 		panic("failed to register clocksource");
 
 	return 0;
-- 
1.7.3.2.146.gca209

[PATCH 09/10] xtensa: convert to clocksource_register_hz/khz

[John Stultz]

This converts the xtensa clocksource to use clocksource_register_hz/khz

This is untested, so any assistance in testing would be appreciated!

CC: Chris Zankel <chris@zankel.net>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 arch/xtensa/kernel/time.c |   13 +------------
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/arch/xtensa/kernel/time.c b/arch/xtensa/kernel/time.c
index 19df764..fbca874 100644
--- a/arch/xtensa/kernel/time.c
+++ b/arch/xtensa/kernel/time.c
@@ -41,14 +41,6 @@ static struct clocksource ccount_clocksource = {
 	.rating = 200,
 	.read = ccount_read,
 	.mask = CLOCKSOURCE_MASK(32),
-	/*
-	 * With a shift of 22 the lower limit of the cpu clock is
-	 * 1MHz, where NSEC_PER_CCOUNT is 1000 or a bit less than
-	 * 2^10: Since we have 32 bits and the multiplicator can
-	 * already take up as much as 10 bits, this leaves us with
-	 * remaining upper 22 bits.
-	 */
-	.shift = 22,
 };
 
 static irqreturn_t timer_interrupt(int irq, void *dev_id);
@@ -66,10 +58,7 @@ void __init time_init(void)
 	printk("%d.%02d MHz\n", (int)ccount_per_jiffy/(1000000/HZ),
 			(int)(ccount_per_jiffy/(10000/HZ))%100);
 #endif
-	ccount_clocksource.mult =
-		clocksource_hz2mult(CCOUNT_PER_JIFFY * HZ,
-				ccount_clocksource.shift);
-	clocksource_register(&ccount_clocksource);
+	clocksource_register_hz(&ccount_clocksource, CCOUNT_PER_JIFFY * HZ);
 
 	/* Initialize the linux timer interrupt. */
 
-- 
1.7.3.2.146.gca209

[PATCH 10/10] sparc: convert to clocksource_register_hz/khz

[John Stultz]

This converts the sparc clocksources to use clocksource_register_hz/khz

This is untested, so any assistance in testing would be appreciated!

CC: "David S. Miller" <davem@davemloft.net>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
Acked-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc/kernel/time_64.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/sparc/kernel/time_64.c b/arch/sparc/kernel/time_64.c
index 3bc9c99..58aa27b 100644
--- a/arch/sparc/kernel/time_64.c
+++ b/arch/sparc/kernel/time_64.c
@@ -816,14 +816,12 @@ void __init time_init(void)
 		clocksource_hz2mult(freq, SPARC64_NSEC_PER_CYC_SHIFT);
 
 	clocksource_tick.name = tick_ops->name;
-	clocksource_calc_mult_shift(&clocksource_tick, freq, 4);
 	clocksource_tick.read = clocksource_tick_read;
 
+	clocksource_register_hz(&clocksource_tick, freq);
 	printk("clocksource: mult[%x] shift[%d]\n",
 	       clocksource_tick.mult, clocksource_tick.shift);
 
-	clocksource_register(&clocksource_tick);
-
 	sparc64_clockevent.name = tick_ops->name;
 	clockevents_calc_mult_shift(&sparc64_clockevent, freq, 4);
 
-- 
1.7.3.2.146.gca209