2008-01-29 22:17:39

by Guillaume Chazarain

Subject: [PATCH] x86: remove unused code in set_cyc2ns_scale()

This should be fold into:
4f95bd6e2b21a8c724357463f8341502d47aba13
x86: scale cyc_2_nsec according to CPU frequency

Signed-off-by: Guillaume Chazarain <[email protected]>
---
arch/x86/kernel/tsc_32.c | 14 +++++---------
arch/x86/kernel/tsc_64.c | 14 +++++---------
2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/tsc_32.c b/arch/x86/kernel/tsc_32.c
index 43517e3..e05e221 100644
--- a/arch/x86/kernel/tsc_32.c
+++ b/arch/x86/kernel/tsc_32.c
@@ -83,20 +83,16 @@ DEFINE_PER_CPU(unsigned long, cyc2ns);

static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
{
- unsigned long flags, prev_scale, *scale;
- unsigned long long tsc_now, ns_now;
+ unsigned long flags, *scale;
+
+ if (!cpu_khz)
+ return;

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;
+ *scale = (NSEC_PER_MSEC << CYC2NS_SCALE_FACTOR)/cpu_khz;

/*
* Start smoothly with the new frequency:
diff --git a/arch/x86/kernel/tsc_64.c b/arch/x86/kernel/tsc_64.c
index 947554d..e0e9d4f 100644
--- a/arch/x86/kernel/tsc_64.c
+++ b/arch/x86/kernel/tsc_64.c
@@ -44,20 +44,16 @@ DEFINE_PER_CPU(unsigned long, cyc2ns);

static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
{
- unsigned long flags, prev_scale, *scale;
- unsigned long long tsc_now, ns_now;
+ unsigned long flags, *scale;
+
+ if (!cpu_khz)
+ return;

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;
+ *scale = (NSEC_PER_MSEC << CYC2NS_SCALE_FACTOR)/cpu_khz;

sched_clock_idle_wakeup_event(0);
local_irq_restore(flags);
--
1.5.3.7

2008-01-31 15:22:20

by Ingo Molnar

Subject: Re: [PATCH] x86: remove unused code in set_cyc2ns_scale()

* Guillaume Chazarain <[email protected]> wrote:

> arch/x86/kernel/tsc_32.c | 14 +++++---------
> arch/x86/kernel/tsc_64.c | 14 +++++---------
> 2 files changed, 10 insertions(+), 18 deletions(-)

> static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
> {
> - unsigned long flags, prev_scale, *scale;
> - unsigned long long tsc_now, ns_now;
> + unsigned long flags, *scale;
> +
> + if (!cpu_khz)
> + return;
>
> 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;
> + *scale = (NSEC_PER_MSEC << CYC2NS_SCALE_FACTOR)/cpu_khz;

hm, this is not a pure elimination of dead code, this will change
behavior. For example we wont call sched_clock_idle_sleep_event() on
!cpu_khz now. Hm?

Ingo

2008-01-31 19:18:51

by Guillaume Chazarain

Subject: Re: [PATCH] x86: remove unused code in set_cyc2ns_scale()

On 1/31/08, Ingo Molnar <[email protected]> wrote:
> hm, this is not a pure elimination of dead code, this will change
> behavior. For example we wont call sched_clock_idle_sleep_event() on
> !cpu_khz now. Hm?

Oops, indeed I overlooked that. OTOH, I can't see how it can happen
(in 32 bit at least), and even if it happens it should not have any
effect. But I'll keep this check to avoid making this case illegal.

Thanks for the review.

--
Guillaume