Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-08 Thread Arjan van de Ven
On Sat, 8 Dec 2007 20:16:29 +0100
Ingo Molnar <[EMAIL PROTECTED]> wrote:

> 
> * Arjan van de Ven <[EMAIL PROTECTED]> wrote:
> 
> > > > > Firstly, we dont need the 'offset' anymore because
> > > > > cpu_clock() maintains offsets itself.
> > > > 
> > > > Yes, but a lower quality one. __update_rq_clock tries to 
> > > > compensate large jumping clocks with a jiffy resolution, while
> > > > my offset arranges for a very smooth frequency transition.
> > > 
> > > yes, but that would be easy to fix up via calling 
> > > sched_clock_idle_wakeup_event(0) when doing a frequency
> > > transition, without burdening the normal sched_clock() codepath
> > > with the offset. See the attached latest version.
> > 
> > can this deal with dual/quad core where the frequency of one core 
> > changes if the sofware changes the frequency of the other core?
> 
> doesnt the notifier still get run on the target CPU?
> 

 if and only if the BIOS actually gives correct information to the
OS.

In reality... that's not a very common thing on this field sadly

-- 
If you want to reach me at my work email, use [EMAIL PROTECTED]
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-08 Thread Ingo Molnar

* Arjan van de Ven <[EMAIL PROTECTED]> wrote:

> > > > Firstly, we dont need the 'offset' anymore because cpu_clock() 
> > > > maintains offsets itself.
> > > 
> > > Yes, but a lower quality one. __update_rq_clock tries to 
> > > compensate large jumping clocks with a jiffy resolution, while my 
> > > offset arranges for a very smooth frequency transition.
> > 
> > yes, but that would be easy to fix up via calling 
> > sched_clock_idle_wakeup_event(0) when doing a frequency transition, 
> > without burdening the normal sched_clock() codepath with the offset. 
> > See the attached latest version.
> 
> can this deal with dual/quad core where the frequency of one core 
> changes if the sofware changes the frequency of the other core?

doesnt the notifier still get run on the target CPU?

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-08 Thread Arjan van de Ven
On Fri, 7 Dec 2007 15:52:06 +0100
Ingo Molnar <[EMAIL PROTECTED]> wrote:

> 
> * Guillaume Chazarain <[EMAIL PROTECTED]> wrote:
> 
> > Le Fri, 7 Dec 2007 14:55:25 +0100,
> > Ingo Molnar <[EMAIL PROTECTED]> a ??crit :
> > 
> > > Firstly, we dont need the 'offset' anymore because cpu_clock() 
> > > maintains offsets itself.
> > 
> > Yes, but a lower quality one. __update_rq_clock tries to compensate 
> > large jumping clocks with a jiffy resolution, while my offset
> > arranges for a very smooth frequency transition.
> 
> yes, but that would be easy to fix up via calling 
> sched_clock_idle_wakeup_event(0) when doing a frequency transition, 
> without burdening the normal sched_clock() codepath with the offset.
> See the attached latest version.


can this deal with dual/quad core where the frequency of one core
changes if the sofware changes the frequency of the other core?


-- 
If you want to reach me at my work email, use [EMAIL PROTECTED]
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-08 Thread Arjan van de Ven
On Fri, 7 Dec 2007 15:52:06 +0100
Ingo Molnar [EMAIL PROTECTED] wrote:

 
 * Guillaume Chazarain [EMAIL PROTECTED] wrote:
 
  Le Fri, 7 Dec 2007 14:55:25 +0100,
  Ingo Molnar [EMAIL PROTECTED] a ??crit :
  
   Firstly, we dont need the 'offset' anymore because cpu_clock() 
   maintains offsets itself.
  
  Yes, but a lower quality one. __update_rq_clock tries to compensate 
  large jumping clocks with a jiffy resolution, while my offset
  arranges for a very smooth frequency transition.
 
 yes, but that would be easy to fix up via calling 
 sched_clock_idle_wakeup_event(0) when doing a frequency transition, 
 without burdening the normal sched_clock() codepath with the offset.
 See the attached latest version.


can this deal with dual/quad core where the frequency of one core
changes if the sofware changes the frequency of the other core?


-- 
If you want to reach me at my work email, use [EMAIL PROTECTED]
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-08 Thread Ingo Molnar

* Arjan van de Ven [EMAIL PROTECTED] wrote:

Firstly, we dont need the 'offset' anymore because cpu_clock() 
maintains offsets itself.
   
   Yes, but a lower quality one. __update_rq_clock tries to 
   compensate large jumping clocks with a jiffy resolution, while my 
   offset arranges for a very smooth frequency transition.
  
  yes, but that would be easy to fix up via calling 
  sched_clock_idle_wakeup_event(0) when doing a frequency transition, 
  without burdening the normal sched_clock() codepath with the offset. 
  See the attached latest version.
 
 can this deal with dual/quad core where the frequency of one core 
 changes if the sofware changes the frequency of the other core?

doesnt the notifier still get run on the target CPU?

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-08 Thread Arjan van de Ven
On Sat, 8 Dec 2007 20:16:29 +0100
Ingo Molnar [EMAIL PROTECTED] wrote:

 
 * Arjan van de Ven [EMAIL PROTECTED] wrote:
 
 Firstly, we dont need the 'offset' anymore because
 cpu_clock() maintains offsets itself.

Yes, but a lower quality one. __update_rq_clock tries to 
compensate large jumping clocks with a jiffy resolution, while
my offset arranges for a very smooth frequency transition.
   
   yes, but that would be easy to fix up via calling 
   sched_clock_idle_wakeup_event(0) when doing a frequency
   transition, without burdening the normal sched_clock() codepath
   with the offset. See the attached latest version.
  
  can this deal with dual/quad core where the frequency of one core 
  changes if the sofware changes the frequency of the other core?
 
 doesnt the notifier still get run on the target CPU?
 

 if and only if the BIOS actually gives correct information to the
OS.

In reality... that's not a very common thing on this field sadly

-- 
If you want to reach me at my work email, use [EMAIL PROTECTED]
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-07 Thread Ingo Molnar

* Guillaume Chazarain <[EMAIL PROTECTED]> wrote:

> Le Fri, 7 Dec 2007 14:55:25 +0100,
> Ingo Molnar <[EMAIL PROTECTED]> a ??crit :
> 
> > Firstly, we dont need the 'offset' anymore because cpu_clock() 
> > maintains offsets itself.
> 
> Yes, but a lower quality one. __update_rq_clock tries to compensate 
> large jumping clocks with a jiffy resolution, while my offset arranges 
> for a very smooth frequency transition.

yes, but that would be easy to fix up via calling 
sched_clock_idle_wakeup_event(0) when doing a frequency transition, 
without burdening the normal sched_clock() codepath with the offset. See 
the attached latest version.

Ingo

--->
Subject: x86: scale cyc_2_nsec according to CPU frequency
From: "Guillaume Chazarain" <[EMAIL PROTECTED]>

scale the sched_clock() cyc_2_nsec scaling factor according to
CPU frequency changes.

[ [EMAIL PROTECTED]: simplified it and fixed it for SMP. ]

Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
Signed-off-by: Thomas Gleixner <[EMAIL PROTECTED]>
---
 arch/x86/kernel/tsc_32.c |   45 +++
 arch/x86/kernel/tsc_64.c |   59 +++
 include/asm-x86/timer.h  |   23 ++
 3 files changed, 106 insertions(+), 21 deletions(-)

Index: linux-x86.q/arch/x86/kernel/tsc_32.c
===
--- linux-x86.q.orig/arch/x86/kernel/tsc_32.c
+++ linux-x86.q/arch/x86/kernel/tsc_32.c
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -78,15 +79,35 @@ EXPORT_SYMBOL_GPL(check_tsc_unstable);
  *  cyc2ns_scale is limited to 10^6 * 2^10, which fits in 32 bits.
  *  ([EMAIL PROTECTED])
  *
+ *  ns += offset to avoid sched_clock jumps with cpufreq
+ *
  * [EMAIL PROTECTED] "math is hard, lets go shopping!"
  */
-unsigned long cyc2ns_scale __read_mostly;
 
-#define CYC2NS_SCALE_FACTOR 10 /* 2^10, carefully chosen */
+DEFINE_PER_CPU(unsigned long, cyc2ns);
 
-static inline void set_cyc2ns_scale(unsigned long cpu_khz)
+static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
 {
-   cyc2ns_scale = (100 << CYC2NS_SCALE_FACTOR)/cpu_khz;
+   unsigned long flags, prev_scale, *scale;
+   unsigned long long tsc_now, ns_now;
+
+   local_irq_save(flags);
+   sched_clock_idle_sleep_event();
+
+   scale = _cpu(cyc2ns, cpu);
+
+   rdtscll(tsc_now);
+   ns_now = __cycles_2_ns(tsc_now);
+
+   prev_scale = *scale;
+   if (cpu_khz)
+   *scale = (NSEC_PER_MSEC << CYC2NS_SCALE_FACTOR)/cpu_khz;
+
+   /*
+* Start smoothly with the new frequency:
+*/
+   sched_clock_idle_wakeup_event(0);
+   local_irq_restore(flags);
 }
 
 /*
@@ -239,7 +260,9 @@ time_cpufreq_notifier(struct notifier_bl
ref_freq, freq->new);
if (!(freq->flags & CPUFREQ_CONST_LOOPS)) {
tsc_khz = cpu_khz;
-   set_cyc2ns_scale(cpu_khz);
+   preempt_disable();
+   set_cyc2ns_scale(cpu_khz, smp_processor_id());
+   preempt_enable();
/*
 * TSC based sched_clock turns
 * to junk w/ cpufreq
@@ -367,6 +390,8 @@ static inline void check_geode_tsc_relia
 
 void __init tsc_init(void)
 {
+   int cpu;
+
if (!cpu_has_tsc || tsc_disable)
goto out_no_tsc;
 
@@ -380,7 +405,15 @@ void __init tsc_init(void)
(unsigned long)cpu_khz / 1000,
(unsigned long)cpu_khz % 1000);
 
-   set_cyc2ns_scale(cpu_khz);
+   /*
+* Secondary CPUs do not run through tsc_init(), so set up
+* all the scale factors for all CPUs, assuming the same
+* speed as the bootup CPU. (cpufreq notifiers will fix this
+* up if their speed diverges)
+*/
+   for_each_possible_cpu(cpu)
+   set_cyc2ns_scale(cpu_khz, cpu);
+
use_tsc_delay();
 
/* Check and install the TSC clocksource */
Index: linux-x86.q/arch/x86/kernel/tsc_64.c
===
--- linux-x86.q.orig/arch/x86/kernel/tsc_64.c
+++ linux-x86.q/arch/x86/kernel/tsc_64.c
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 
 static int notsc __initdata = 0;
 
@@ -18,16 +19,50 @@ EXPORT_SYMBOL(cpu_khz);
 unsigned int tsc_khz;
 EXPORT_SYMBOL(tsc_khz);
 
-static unsigned int cyc2ns_scale __read_mostly;
+/* 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)
+ *
+ * 

Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-07 Thread Guillaume Chazarain
Le Fri, 7 Dec 2007 14:55:25 +0100,
Ingo Molnar <[EMAIL PROTECTED]> a écrit :

> Firstly, we dont need the 'offset' anymore because cpu_clock() maintains 
> offsets itself.

Yes, but a lower quality one. __update_rq_clock tries to compensate
large jumping clocks with a jiffy resolution, while my offset arranges
for a very smooth frequency transition.

I agree with keeping a single offset, but I liked the fact that with my
patch on frequency change, the clock had no jump at all.

> + *  ns += offset to avoid sched_clock jumps with cpufreq

I guess this needs to go away if I don't make my point :-(

> + printk("CPU#%d: changed cyc2ns scale from %ld to %ld\n",
> + cpu, prev_scale, *scale);

Pointing it out just to be sure it does not end in the final version ;-)

Thanks for cleaning up my mess ;-)

-- 
Guillaume
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-07 Thread Guillaume Chazarain
Le Fri, 7 Dec 2007 14:55:25 +0100,
Ingo Molnar [EMAIL PROTECTED] a écrit :

 Firstly, we dont need the 'offset' anymore because cpu_clock() maintains 
 offsets itself.

Yes, but a lower quality one. __update_rq_clock tries to compensate
large jumping clocks with a jiffy resolution, while my offset arranges
for a very smooth frequency transition.

I agree with keeping a single offset, but I liked the fact that with my
patch on frequency change, the clock had no jump at all.

 + *  ns += offset to avoid sched_clock jumps with cpufreq

I guess this needs to go away if I don't make my point :-(

 + printk(CPU#%d: changed cyc2ns scale from %ld to %ld\n,
 + cpu, prev_scale, *scale);

Pointing it out just to be sure it does not end in the final version ;-)

Thanks for cleaning up my mess ;-)

-- 
Guillaume
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] x86: scale cyc_2_nsec according to CPU frequency

2007-12-07 Thread Ingo Molnar

* Guillaume Chazarain [EMAIL PROTECTED] wrote:

 Le Fri, 7 Dec 2007 14:55:25 +0100,
 Ingo Molnar [EMAIL PROTECTED] a ??crit :
 
  Firstly, we dont need the 'offset' anymore because cpu_clock() 
  maintains offsets itself.
 
 Yes, but a lower quality one. __update_rq_clock tries to compensate 
 large jumping clocks with a jiffy resolution, while my offset arranges 
 for a very smooth frequency transition.

yes, but that would be easy to fix up via calling 
sched_clock_idle_wakeup_event(0) when doing a frequency transition, 
without burdening the normal sched_clock() codepath with the offset. See 
the attached latest version.

Ingo

---
Subject: x86: scale cyc_2_nsec according to CPU frequency
From: Guillaume Chazarain [EMAIL PROTECTED]

scale the sched_clock() cyc_2_nsec scaling factor according to
CPU frequency changes.

[ [EMAIL PROTECTED]: simplified it and fixed it for SMP. ]

Signed-off-by: Ingo Molnar [EMAIL PROTECTED]
Signed-off-by: Thomas Gleixner [EMAIL PROTECTED]
---
 arch/x86/kernel/tsc_32.c |   45 +++
 arch/x86/kernel/tsc_64.c |   59 +++
 include/asm-x86/timer.h  |   23 ++
 3 files changed, 106 insertions(+), 21 deletions(-)

Index: linux-x86.q/arch/x86/kernel/tsc_32.c
===
--- linux-x86.q.orig/arch/x86/kernel/tsc_32.c
+++ linux-x86.q/arch/x86/kernel/tsc_32.c
@@ -5,6 +5,7 @@
 #include linux/jiffies.h
 #include linux/init.h
 #include linux/dmi.h
+#include linux/percpu.h
 
 #include asm/delay.h
 #include asm/tsc.h
@@ -78,15 +79,35 @@ EXPORT_SYMBOL_GPL(check_tsc_unstable);
  *  cyc2ns_scale is limited to 10^6 * 2^10, which fits in 32 bits.
  *  ([EMAIL PROTECTED])
  *
+ *  ns += offset to avoid sched_clock jumps with cpufreq
+ *
  * [EMAIL PROTECTED] math is hard, lets go shopping!
  */
-unsigned long cyc2ns_scale __read_mostly;
 
-#define CYC2NS_SCALE_FACTOR 10 /* 2^10, carefully chosen */
+DEFINE_PER_CPU(unsigned long, cyc2ns);
 
-static inline void set_cyc2ns_scale(unsigned long cpu_khz)
+static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
 {
-   cyc2ns_scale = (100  CYC2NS_SCALE_FACTOR)/cpu_khz;
+   unsigned long flags, prev_scale, *scale;
+   unsigned long long tsc_now, ns_now;
+
+   local_irq_save(flags);
+   sched_clock_idle_sleep_event();
+
+   scale = per_cpu(cyc2ns, cpu);
+
+   rdtscll(tsc_now);
+   ns_now = __cycles_2_ns(tsc_now);
+
+   prev_scale = *scale;
+   if (cpu_khz)
+   *scale = (NSEC_PER_MSEC  CYC2NS_SCALE_FACTOR)/cpu_khz;
+
+   /*
+* Start smoothly with the new frequency:
+*/
+   sched_clock_idle_wakeup_event(0);
+   local_irq_restore(flags);
 }
 
 /*
@@ -239,7 +260,9 @@ time_cpufreq_notifier(struct notifier_bl
ref_freq, freq-new);
if (!(freq-flags  CPUFREQ_CONST_LOOPS)) {
tsc_khz = cpu_khz;
-   set_cyc2ns_scale(cpu_khz);
+   preempt_disable();
+   set_cyc2ns_scale(cpu_khz, smp_processor_id());
+   preempt_enable();
/*
 * TSC based sched_clock turns
 * to junk w/ cpufreq
@@ -367,6 +390,8 @@ static inline void check_geode_tsc_relia
 
 void __init tsc_init(void)
 {
+   int cpu;
+
if (!cpu_has_tsc || tsc_disable)
goto out_no_tsc;
 
@@ -380,7 +405,15 @@ void __init tsc_init(void)
(unsigned long)cpu_khz / 1000,
(unsigned long)cpu_khz % 1000);
 
-   set_cyc2ns_scale(cpu_khz);
+   /*
+* Secondary CPUs do not run through tsc_init(), so set up
+* all the scale factors for all CPUs, assuming the same
+* speed as the bootup CPU. (cpufreq notifiers will fix this
+* up if their speed diverges)
+*/
+   for_each_possible_cpu(cpu)
+   set_cyc2ns_scale(cpu_khz, cpu);
+
use_tsc_delay();
 
/* Check and install the TSC clocksource */
Index: linux-x86.q/arch/x86/kernel/tsc_64.c
===
--- linux-x86.q.orig/arch/x86/kernel/tsc_64.c
+++ linux-x86.q/arch/x86/kernel/tsc_64.c
@@ -10,6 +10,7 @@
 
 #include asm/hpet.h
 #include asm/timex.h
+#include asm/timer.h
 
 static int notsc __initdata = 0;
 
@@ -18,16 +19,50 @@ EXPORT_SYMBOL(cpu_khz);
 unsigned int tsc_khz;
 EXPORT_SYMBOL(tsc_khz);
 
-static unsigned int cyc2ns_scale __read_mostly;
+/* 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 /