Re: [patch] x86: scale cyc_2_nsec according to CPU frequency
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
* 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
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
* 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 = &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
#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
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/
[patch] x86: scale cyc_2_nsec according to CPU frequency
* Guillaume Chazarain <[EMAIL PROTECTED]> wrote:
> > > Hmrpf. sched_clock() is used for the time stamp of the printks. We
> > > need to find some better solution other than killing off the tsc
> > > access completely.
> >
> > Something like http://lkml.org/lkml/2007/3/16/291 that would need
> > some refresh?
>
> And here is a refreshed one just for testing with 2.6-git. The 64 bit
> part is a shamelessly untested copy/paste as I cannot test it.
Guillaume, i've updated your patch with a handful of changes - see the
result below.
Firstly, we dont need the 'offset' anymore because cpu_clock() maintains
offsets itself. This simplifies the math and speeds up the sched_clock()
common case.
Secondly, with PER_CPU variables we need to update them for all possible
CPUs - otherwise they might end up with a zero scaling factor which is
not good. (not all CPUs are cpufreq capable)
Thirdly, we can do a bit smarter and faster by using the fact that
local_irq_disable() is preempt-safe - so we can use per_cpu() instead of
get_cpu_var().
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 | 41 +++-
arch/x86/kernel/tsc_64.c | 59 +++
include/asm-x86/timer.h | 23 ++
3 files changed, 102 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,31 @@ 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);
+ 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;
+
+ printk("CPU#%d: changed cyc2ns scale from %ld to %ld\n",
+ cpu, prev_scale, *scale);
+ local_irq_restore(flags);
}
/*
@@ -239,7 +256,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 +386,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 +401,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
