2009-09-03 13:23:34

by Peter Zijlstra

[permalink] [raw]
Subject: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

Move some of the aperf/mperf code out from the cpufreq driver thingy
so that other people can enjoy it too.

Signed-off-by: Peter Zijlstra <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Venkatesh Pallipadi <[email protected]>
Cc: Yanmin <[email protected]>
Cc: Dave Jones <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: [email protected]
---
arch/x86/include/asm/processor.h | 12 ++++++++
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c | 41 +++++++++--------------------
2 files changed, 26 insertions(+), 27 deletions(-)

Index: linux-2.6/arch/x86/include/asm/processor.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/processor.h
+++ linux-2.6/arch/x86/include/asm/processor.h
@@ -1000,4 +1000,16 @@ extern void start_thread(struct pt_regs
extern int get_tsc_mode(unsigned long adr);
extern int set_tsc_mode(unsigned int val);

+struct aperfmperf {
+ u64 aperf, mperf;
+};
+
+static inline void get_aperfmperf(struct aperfmperf *am)
+{
+ WARN_ON_ONCE(!boot_cpu_has(X86_FEATURE_APERFMPERF));
+
+ rdmsrl(MSR_IA32_APERF, am->aperf);
+ rdmsrl(MSR_IA32_MPERF, am->mperf);
+}
+
#endif /* _ASM_X86_PROCESSOR_H */
Index: linux-2.6/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ linux-2.6/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -243,23 +243,12 @@ static u32 get_cur_val(const struct cpum
return cmd.val;
}

-struct perf_pair {
- union {
- struct {
- u32 lo;
- u32 hi;
- } split;
- u64 whole;
- } aperf, mperf;
-};
-
/* Called via smp_call_function_single(), on the target CPU */
static void read_measured_perf_ctrs(void *_cur)
{
- struct perf_pair *cur = _cur;
+ struct aperfmperf *am = _cur;

- rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
- rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
+ get_aperfmperf(am);
}

/*
@@ -278,19 +267,17 @@ static void read_measured_perf_ctrs(void
static unsigned int get_measured_perf(struct cpufreq_policy *policy,
unsigned int cpu)
{
- struct perf_pair readin, cur;
+ struct aperfmperf readin, cur;
unsigned int perf_percent;
unsigned int retval;

if (smp_call_function_single(cpu, read_measured_perf_ctrs, &readin, 1))
return 0;

- cur.aperf.whole = readin.aperf.whole -
- per_cpu(msr_data, cpu).saved_aperf;
- cur.mperf.whole = readin.mperf.whole -
- per_cpu(msr_data, cpu).saved_mperf;
- per_cpu(msr_data, cpu).saved_aperf = readin.aperf.whole;
- per_cpu(msr_data, cpu).saved_mperf = readin.mperf.whole;
+ cur.aperf = readin.aperf - per_cpu(msr_data, cpu).saved_aperf;
+ cur.mperf = readin.mperf - per_cpu(msr_data, cpu).saved_mperf;
+ per_cpu(msr_data, cpu).saved_aperf = readin.aperf;
+ per_cpu(msr_data, cpu).saved_mperf = readin.mperf;

#ifdef __i386__
/*
@@ -305,8 +292,8 @@ static unsigned int get_measured_perf(st
h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
shift_count = fls(h);

- cur.aperf.whole >>= shift_count;
- cur.mperf.whole >>= shift_count;
+ cur.aperf >>= shift_count;
+ cur.mperf >>= shift_count;
}

if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
@@ -321,14 +308,14 @@ static unsigned int get_measured_perf(st
perf_percent = 0;

#else
- if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
+ if (unlikely(((unsigned long)(-1) / 100) < cur.aperf)) {
int shift_count = 7;
- cur.aperf.whole >>= shift_count;
- cur.mperf.whole >>= shift_count;
+ cur.aperf >>= shift_count;
+ cur.mperf >>= shift_count;
}

- if (cur.aperf.whole && cur.mperf.whole)
- perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
+ if (cur.aperf && cur.mperf)
+ perf_percent = (cur.aperf * 100) / cur.mperf;
else
perf_percent = 0;


--


2009-09-04 09:19:11

by Thomas Renninger

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

On Thursday 03 September 2009 15:21:55 Peter Zijlstra wrote:
> Move some of the aperf/mperf code out from the cpufreq driver thingy
> so that other people can enjoy it too.
>
> Signed-off-by: Peter Zijlstra <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Venkatesh Pallipadi <[email protected]>
> Cc: Yanmin <[email protected]>
> Cc: Dave Jones <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Cc: [email protected]
> ---
> arch/x86/include/asm/processor.h | 12 ++++++++
> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c | 41
+++++++++--------------------
> 2 files changed, 26 insertions(+), 27 deletions(-)
>
> Index: linux-2.6/arch/x86/include/asm/processor.h
> ===================================================================
> --- linux-2.6.orig/arch/x86/include/asm/processor.h
> +++ linux-2.6/arch/x86/include/asm/processor.h
> @@ -1000,4 +1000,16 @@ extern void start_thread(struct pt_regs
> extern int get_tsc_mode(unsigned long adr);
> extern int set_tsc_mode(unsigned int val);
>
> +struct aperfmperf {
> + u64 aperf, mperf;
> +};
> +
> +static inline void get_aperfmperf(struct aperfmperf *am)
> +{
> + WARN_ON_ONCE(!boot_cpu_has(X86_FEATURE_APERFMPERF));
> +
> + rdmsrl(MSR_IA32_APERF, am->aperf);
> + rdmsrl(MSR_IA32_MPERF, am->mperf);
> +}
> +
> #endif /* _ASM_X86_PROCESSOR_H */
> Index: linux-2.6/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ linux-2.6/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -243,23 +243,12 @@ static u32 get_cur_val(const struct cpum
> return cmd.val;
> }
>
> -struct perf_pair {
> - union {
> - struct {
> - u32 lo;
> - u32 hi;
> - } split;
> - u64 whole;
> - } aperf, mperf;
> -};
> -
> /* Called via smp_call_function_single(), on the target CPU */
> static void read_measured_perf_ctrs(void *_cur)
> {
> - struct perf_pair *cur = _cur;
> + struct aperfmperf *am = _cur;
>
> - rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
> - rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
> + get_aperfmperf(am);
> }
>
> /*
> @@ -278,19 +267,17 @@ static void read_measured_perf_ctrs(void
> static unsigned int get_measured_perf(struct cpufreq_policy *policy,
> unsigned int cpu)
> {
> - struct perf_pair readin, cur;
> + struct aperfmperf readin, cur;
> unsigned int perf_percent;
> unsigned int retval;
>
> if (smp_call_function_single(cpu, read_measured_perf_ctrs, &readin,
1))
> return 0;
>
> - cur.aperf.whole = readin.aperf.whole -
> - per_cpu(msr_data, cpu).saved_aperf;
> - cur.mperf.whole = readin.mperf.whole -
> - per_cpu(msr_data, cpu).saved_mperf;
> - per_cpu(msr_data, cpu).saved_aperf = readin.aperf.whole;
> - per_cpu(msr_data, cpu).saved_mperf = readin.mperf.whole;
> + cur.aperf = readin.aperf - per_cpu(msr_data, cpu).saved_aperf;
> + cur.mperf = readin.mperf - per_cpu(msr_data, cpu).saved_mperf;
> + per_cpu(msr_data, cpu).saved_aperf = readin.aperf;
> + per_cpu(msr_data, cpu).saved_mperf = readin.mperf;
>
> #ifdef __i386__
> /*
> @@ -305,8 +292,8 @@ static unsigned int get_measured_perf(st
> h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
You still use struct perf_pair split/hi/lo members in #ifdef __i386__
case which you deleted above.
> shift_count = fls(h);
>
> - cur.aperf.whole >>= shift_count;
> - cur.mperf.whole >>= shift_count;
> + cur.aperf >>= shift_count;
> + cur.mperf >>= shift_count;
> }
>
> if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
Same here, possibly still elsewhere.
Is this only x86_64 compile tested?

Thomas

> @@ -321,14 +308,14 @@ static unsigned int get_measured_perf(st
> perf_percent = 0;
>
> #else
> - if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
> + if (unlikely(((unsigned long)(-1) / 100) < cur.aperf)) {
> int shift_count = 7;
> - cur.aperf.whole >>= shift_count;
> - cur.mperf.whole >>= shift_count;
> + cur.aperf >>= shift_count;
> + cur.mperf >>= shift_count;
> }
>
> - if (cur.aperf.whole && cur.mperf.whole)
> - perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
> + if (cur.aperf && cur.mperf)
> + perf_percent = (cur.aperf * 100) / cur.mperf;
> else
> perf_percent = 0;
>
>
> --
>
> --
> To unsubscribe from this list: send the line "unsubscribe cpufreq" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2009-09-04 09:25:20

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

On Fri, 2009-09-04 at 11:19 +0200, Thomas Renninger wrote:
> You still use struct perf_pair split/hi/lo members in #ifdef __i386__
> case which you deleted above.

> > shift_count = fls(h);
> >
> > - cur.aperf.whole >>= shift_count;
> > - cur.mperf.whole >>= shift_count;
> > + cur.aperf >>= shift_count;
> > + cur.mperf >>= shift_count;
> > }
> >
> > if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> Same here, possibly still elsewhere.
> Is this only x86_64 compile tested?

Of course, who still has 32bit only hardware anyway ;-)

Will fix, thanks for spotting that.

2009-09-04 09:27:36

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

On Fri, 2009-09-04 at 11:25 +0200, Peter Zijlstra wrote:
> On Fri, 2009-09-04 at 11:19 +0200, Thomas Renninger wrote:
> > You still use struct perf_pair split/hi/lo members in #ifdef __i386__
> > case which you deleted above.
>
> > > shift_count = fls(h);
> > >
> > > - cur.aperf.whole >>= shift_count;
> > > - cur.mperf.whole >>= shift_count;
> > > + cur.aperf >>= shift_count;
> > > + cur.mperf >>= shift_count;
> > > }
> > >
> > > if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> > Same here, possibly still elsewhere.
> > Is this only x86_64 compile tested?
>
> Of course, who still has 32bit only hardware anyway ;-)
>
> Will fix, thanks for spotting that.

Hrmm, on that, does it really make sense to maintain the i386 code path?

How frequently is that code called and what i386 only chips support
aperf/mperf, atom?

2009-09-04 09:34:46

by Thomas Renninger

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

On Friday 04 September 2009 11:27:19 Peter Zijlstra wrote:
> On Fri, 2009-09-04 at 11:25 +0200, Peter Zijlstra wrote:
> > On Fri, 2009-09-04 at 11:19 +0200, Thomas Renninger wrote:
> > > You still use struct perf_pair split/hi/lo members in #ifdef
__i386__
> > > case which you deleted above.
> >
> > > > shift_count = fls(h);
> > > >
> > > > - cur.aperf.whole >>= shift_count;
> > > > - cur.mperf.whole >>= shift_count;
> > > > + cur.aperf >>= shift_count;
> > > > + cur.mperf >>= shift_count;
> > > > }
> > > >
> > > > if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> > > Same here, possibly still elsewhere.
> > > Is this only x86_64 compile tested?
> >
> > Of course, who still has 32bit only hardware anyway ;-)
> >
> > Will fix, thanks for spotting that.
>
> Hrmm, on that, does it really make sense to maintain the i386 code
> path?
>
> How frequently is that code called and what i386 only chips support
> aperf/mperf, atom?
Venki should know best.

Thomas

2009-09-04 14:22:46

by Dave Jones

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

On Fri, Sep 04, 2009 at 11:27:19AM +0200, Peter Zijlstra wrote:
> On Fri, 2009-09-04 at 11:25 +0200, Peter Zijlstra wrote:
> > On Fri, 2009-09-04 at 11:19 +0200, Thomas Renninger wrote:
> > > You still use struct perf_pair split/hi/lo members in #ifdef __i386__
> > > case which you deleted above.
> >
> > > > shift_count = fls(h);
> > > >
> > > > - cur.aperf.whole >>= shift_count;
> > > > - cur.mperf.whole >>= shift_count;
> > > > + cur.aperf >>= shift_count;
> > > > + cur.mperf >>= shift_count;
> > > > }
> > > >
> > > > if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> > > Same here, possibly still elsewhere.
> > > Is this only x86_64 compile tested?
> >
> > Of course, who still has 32bit only hardware anyway ;-)
> >
> > Will fix, thanks for spotting that.
>
> Hrmm, on that, does it really make sense to maintain the i386 code path?
>
> How frequently is that code called and what i386 only chips support
> aperf/mperf, atom?

any 64-bit cpu that supports it can have a 32bit kernel installed on it.
(and a significant number of users actually do this).

Dave

2009-09-04 14:42:21

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

On Fri, 2009-09-04 at 10:22 -0400, Dave Jones wrote:
> On Fri, Sep 04, 2009 at 11:27:19AM +0200, Peter Zijlstra wrote:
> > On Fri, 2009-09-04 at 11:25 +0200, Peter Zijlstra wrote:
> > > On Fri, 2009-09-04 at 11:19 +0200, Thomas Renninger wrote:
> > > > You still use struct perf_pair split/hi/lo members in #ifdef __i386__
> > > > case which you deleted above.
> > >
> > > > > shift_count = fls(h);
> > > > >
> > > > > - cur.aperf.whole >>= shift_count;
> > > > > - cur.mperf.whole >>= shift_count;
> > > > > + cur.aperf >>= shift_count;
> > > > > + cur.mperf >>= shift_count;
> > > > > }
> > > > >
> > > > > if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> > > > Same here, possibly still elsewhere.
> > > > Is this only x86_64 compile tested?
> > >
> > > Of course, who still has 32bit only hardware anyway ;-)
> > >
> > > Will fix, thanks for spotting that.
> >
> > Hrmm, on that, does it really make sense to maintain the i386 code path?
> >
> > How frequently is that code called and what i386 only chips support
> > aperf/mperf, atom?
>
> any 64-bit cpu that supports it can have a 32bit kernel installed on it.
> (and a significant number of users actually do this).

1) we really should be pushing those people to run 64bit kernels

[ I'm still hoping distros will start shipping 64bit kernels and have
the bootloader pick the 64bit one when the hardware supports lm ]

2) those cpus aren't real bad at 64bit divisions :-)

2009-09-04 17:49:45

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/14] x86: generic aperf/mperf code.

On 09/04/2009 07:42 AM, Peter Zijlstra wrote:
>>
>> any 64-bit cpu that supports it can have a 32bit kernel installed on it.
>> (and a significant number of users actually do this).
>
> 1) we really should be pushing those people to run 64bit kernels
>
> [ I'm still hoping distros will start shipping 64bit kernels and have
> the bootloader pick the 64bit one when the hardware supports lm ]
>

FWIW, there is already support in Syslinux to do that.

If what distros want is a single kernel image that can boot into either
mode, that might be an interesting enhancement to wraplinux.

-hpa