[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070525074316.GA16821@elte.hu>
Date: Fri, 25 May 2007 09:43:16 +0200
From: Ingo Molnar <mingo@...e.hu>
To: Andi Kleen <andi@...stfloor.org>
Cc: Satyam Sharma <satyam.sharma@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [patch] sched_clock(): cleanups
* Ingo Molnar <mingo@...e.hu> wrote:
> > Admittedly the second test could be inside a block of the first, but
> > then it would make the code more ugly and this code is not
> > performance critical.
>
> moving it inside the block makes the code _cleaner_ in fact :-)
updated version of the cleanup patch below, renamed r_s_c to
resync_freq, and changed 'f' to 'freq'.
Ingo
---------------->
Subject: [patch] sched_clock(): cleanups
From: Ingo Molnar <mingo@...e.hu>
clean up sched-clock.c - mostly comment style fixes.
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
arch/i386/kernel/sched-clock.c | 108 +++++++++++++++++++++++++----------------
1 file changed, 68 insertions(+), 40 deletions(-)
Index: linux-cfs-2.6.22-rc2-mm1.q/arch/i386/kernel/sched-clock.c
===================================================================
--- linux-cfs-2.6.22-rc2-mm1.q.orig/arch/i386/kernel/sched-clock.c
+++ linux-cfs-2.6.22-rc2-mm1.q/arch/i386/kernel/sched-clock.c
@@ -60,18 +60,20 @@ DEFINE_PER_CPU(struct sc_data, sc_data)
*/
unsigned long long native_sched_clock(void)
{
- unsigned long long r;
struct sc_data *sc = &get_cpu_var(sc_data);
+ unsigned long long r;
+ unsigned long flags;
if (sc->unstable) {
- unsigned long flags;
r = (jiffies_64 - sc->sync_base) * (1000000000 / HZ);
r += sc->ns_base;
local_irq_save(flags);
- /* last_val is used to avoid non monotonity on a
- stable->unstable transition. Make sure the time
- never goes to before the last value returned by
- the TSC clock */
+ /*
+ * last_val is used to avoid non monotonity on a
+ * stable->unstable transition. Make sure the time
+ * never goes to before the last value returned by
+ * the TSC clock
+ */
if (r <= sc->last_val)
r = sc->last_val + 1;
sc->last_val = r;
@@ -87,8 +89,10 @@ unsigned long long native_sched_clock(vo
return r;
}
-/* We need to define a real function for sched_clock, to override the
- weak default version */
+/*
+ * We need to define a real function for sched_clock, to override the
+ * weak default version
+ */
#ifdef CONFIG_PARAVIRT
unsigned long long sched_clock(void)
{
@@ -101,9 +105,11 @@ unsigned long long sched_clock(void)
static int no_sc_for_printk;
-/* printk clock: when it is known the sc results are very non monotonic
- fall back to jiffies for printk. Other sched_clock users are supposed
- to handle this. */
+/*
+ * printk clock: when it is known the sc results are very non monotonic
+ * fall back to jiffies for printk. Other sched_clock users are supposed
+ * to handle this.
+ */
unsigned long long printk_clock(void)
{
if (unlikely(no_sc_for_printk))
@@ -119,35 +125,46 @@ static void resync_sc_freq(struct sc_dat
sc->unstable = 1;
return;
}
- /* Handle nesting, but when we're zero multiple calls in a row
- are ok too and not a bug */
+ /*
+ * Handle nesting, but when we're zero multiple calls in a row
+ * are ok too and not a bug
+ */
if (sc->unstable > 0)
sc->unstable--;
if (sc->unstable)
return;
- /* RED-PEN protect with seqlock? I hope that's not needed
- because sched_clock callers should be able to tolerate small
- errors. */
+ /*
+ * RED-PEN protect with seqlock? I hope that's not needed
+ * because sched_clock callers should be able to tolerate small
+ * errors.
+ */
sc->ns_base = ktime_to_ns(ktime_get());
rdtscll(sc->sync_base);
sc->cyc2ns_scale = (1000000 << CYC2NS_SCALE_FACTOR) / newfreq;
}
-static void call_r_s_f(void *arg)
+static void __resync_freq(void *arg)
{
struct cpufreq_freqs *freq = arg;
- unsigned f = freq->new;
- if (!f)
- f = cpufreq_get(freq->cpu);
- if (!f)
- f = tsc_khz;
- resync_sc_freq(&per_cpu(sc_data, freq->cpu), f);
+ unsigned freq = freq->new;
+
+ if (!freq) {
+ freq = cpufreq_get(freq->cpu);
+ /*
+ * Still no frequency? Then fall back to tsc_khz:
+ */
+ if (!freq)
+ freq = tsc_khz;
+ }
+ resync_sc_freq(&per_cpu(sc_data, freq->cpu), freq);
}
-static void call_r_s_f_here(void *arg)
+static void resync_freq(void *arg)
{
- struct cpufreq_freqs f = { .cpu = get_cpu(), .new = 0 };
- call_r_s_f(&f);
+ struct cpufreq_freqs f = { .new = 0 };
+
+ f.cpu = get_cpu();
+ __resync_freq(&f);
put_cpu();
}
@@ -155,9 +172,11 @@ static int sc_freq_event(struct notifier
void *data)
{
struct cpufreq_freqs *freq = data;
- int cpu = get_cpu();
- struct sc_data *sc = &per_cpu(sc_data, cpu);
+ struct sc_data *sc;
+ int cpu;
+ cpu = get_cpu();
+ sc = &per_cpu(sc_data, cpu);
if (cpu_has(&cpu_data[cpu], X86_FEATURE_CONSTANT_TSC))
goto out;
if (freq->old == freq->new)
@@ -167,25 +186,30 @@ static int sc_freq_event(struct notifier
case CPUFREQ_SUSPENDCHANGE:
/* Mark TSC unstable during suspend/resume */
case CPUFREQ_PRECHANGE:
- /* Mark TSC as unstable until cpu frequency change is done
- because we don't know when exactly it will change.
- unstable in used as a counter to guard against races
- between the cpu frequency notifiers and normal resyncs */
+ /*
+ * Mark TSC as unstable until cpu frequency change is done
+ * because we don't know when exactly it will change.
+ * unstable in used as a counter to guard against races
+ * between the cpu frequency notifiers and normal resyncs
+ */
sc->unstable++;
/* FALL THROUGH */
case CPUFREQ_RESUMECHANGE:
case CPUFREQ_POSTCHANGE:
- /* Frequency change or resume is done -- update everything and
- mark TSC as stable again. */
+ /*
+ * Frequency change or resume is done -- update everything
+ * and mark TSC as stable again.
+ */
if (cpu == freq->cpu)
resync_sc_freq(sc, freq->new);
else
- smp_call_function_single(freq->cpu, call_r_s_f,
+ smp_call_function_single(freq->cpu, __resync_freq,
freq, 0, 1);
break;
}
out:
put_cpu();
+
return NOTIFY_DONE;
}
@@ -197,9 +221,11 @@ static int __cpuinit
sc_cpu_event(struct notifier_block *self, unsigned long event, void *hcpu)
{
long cpu = (long)hcpu;
+
if (event == CPU_ONLINE) {
struct cpufreq_freqs f = { .cpu = cpu, .new = 0 };
- smp_call_function_single(cpu, call_r_s_f, &f, 0, 1);
+
+ smp_call_function_single(cpu, __resync_freq, &f, 0, 1);
}
return NOTIFY_DONE;
}
@@ -209,13 +235,15 @@ static __init int init_sched_clock(void)
if (unsynchronized_tsc())
no_sc_for_printk = 1;
- /* On a race between the various events the initialization might be
- done multiple times, but code is tolerant to this */
+ /*
+ * On a race between the various events the initialization might be
+ * done multiple times, but code is tolerant to this
+ */
cpufreq_register_notifier(&sc_freq_notifier,
CPUFREQ_TRANSITION_NOTIFIER);
hotcpu_notifier(sc_cpu_event, 0);
- on_each_cpu(call_r_s_f_here, NULL, 0, 0);
+ on_each_cpu(resync_freq, NULL, 0, 0);
+
return 0;
}
core_initcall(init_sched_clock);
-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists