Please pull arch perfmon update

Please pull arch perfmon update

[Andi Kleen]

Robert,

Here are the arch perfmon changes ported to your latest oprofile tree.
I didn't include the forcepmu=... change yet, that will come later.
Right now the force option is just removed. Please pull.

Thanks,
-Andi

The following changes since commit 22d87103484983035cf46891428819573ec7508d:
  Robert Richter (1):
        Merge branch 'oprofile/powerpc-for-paul' into oprofile/master

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc-2.6.git arch-perfmon2

Andi Kleen (4):
      oprofile: drop const in num counters field
      oprofile: Don't report Nehalem as core_2
      oprofile: Implement Intel architectural perfmon support
      oprofile: discover counters for op ppro too

 arch/x86/oprofile/nmi_int.c       |   26 ++++++---
 arch/x86/oprofile/op_model_ppro.c |  108 +++++++++++++++++++++++++++++--------
 arch/x86/oprofile/op_x86_model.h  |    9 ++-
 3 files changed, 108 insertions(+), 35 deletions(-)
-- 
ak@linux.intel.com

Re: Please pull arch perfmon update

[Robert Richter]

On 30.09.08 01:52:53, Andi Kleen wrote:
> Robert,
> 
> Here are the arch perfmon changes ported to your latest oprofile tree.
> I didn't include the forcepmu=... change yet, that will come later.
> Right now the force option is just removed. Please pull.
> 
> Thanks,
> -Andi
> 
> The following changes since commit 22d87103484983035cf46891428819573ec7508d:
>   Robert Richter (1):
>         Merge branch 'oprofile/powerpc-for-paul' into oprofile/master
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc-2.6.git arch-perfmon2
> 
> Andi Kleen (4):
>       oprofile: drop const in num counters field
>       oprofile: Don't report Nehalem as core_2
>       oprofile: Implement Intel architectural perfmon support
>       oprofile: discover counters for op ppro too

Thanks Andi,

I applied your patches to the x86-oprofile-for-tip branch of
git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git

I added a follow on patch (446223f) on branch arch-perfmon that
changes the initialization so that it uses the init function in
op_x86_model_spec (see below). Please give it a try, it is compile
tested only. If it works for you, I will add it to the tip branch too.

There is one question to patch "oprofile: discover counters for op
ppro too". arch_perfmon_setup_counters() is actually never called for
ppro, so there is no code that changes the numbers in
op_ppro_spec. The patch as it is has no effect (except then loading
additional module code).

Thanks,

-Robert

commit 446223f8d1454e647b54160584c5dec8f83fdddc
Author: Robert Richter <robert.richter@amd.com>
Date:   Sun Oct 12 15:12:34 2008 -0400

    x86/oprofile: moving arch perfmon counter setup to op_x86_model_spec.init
    
    Cc: Andi Kleen <ak@linux.intel.com>
    Signed-off-by: Robert Richter <robert.richter@amd.com>

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 12d6f85..fd902b8 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -435,7 +435,6 @@ static int __init arch_perfmon_init(char **cpu_type)
                return 0;
        *cpu_type = "i386/arch_perfmon";
        model = &op_arch_perfmon_spec;
-       arch_perfmon_setup_counters();
        return 1;
 }
 
diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
index f5a2268..3266795 100644
--- a/arch/x86/oprofile/op_model_ppro.c
+++ b/arch/x86/oprofile/op_model_ppro.c
@@ -220,7 +220,7 @@ struct op_x86_model_spec op_ppro_spec = {
  * the specific CPU.
  */
 
-void arch_perfmon_setup_counters(void)
+static int arch_perfmon_setup_counters(struct oprofile_operations *ignore)
 {
        union cpuid10_eax eax;
 
@@ -240,9 +240,12 @@ void arch_perfmon_setup_counters(void)
        op_arch_perfmon_spec.num_controls = num_counters;
        op_ppro_spec.num_counters = num_counters;
        op_ppro_spec.num_controls = num_counters;
+
+       return 0;
 }
 
 struct op_x86_model_spec op_arch_perfmon_spec = {
+       .init = &arch_perfmon_setup_counters,
        /* num_counters/num_controls filled in at runtime */
        .fill_in_addresses = &ppro_fill_in_addresses,
        /* user space does the cpuid check for available events */
diff --git a/arch/x86/oprofile/op_x86_model.h b/arch/x86/oprofile/op_x86_model.h
index 596de7a..418c2d0 100644
--- a/arch/x86/oprofile/op_x86_model.h
+++ b/arch/x86/oprofile/op_x86_model.h
@@ -51,6 +51,4 @@ extern struct op_x86_model_spec const op_p4_ht2_spec;
 extern struct op_x86_model_spec const op_amd_spec;
 extern struct op_x86_model_spec op_arch_perfmon_spec;
 
-extern void arch_perfmon_setup_counters(void);
-
 #endif /* OP_X86_MODEL_H */


-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: Please pull arch perfmon update

[Andi Kleen]

> 
> I applied your patches to the x86-oprofile-for-tip branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git

Will that still make 2.6.28?

> I added a follow on patch (446223f) on branch arch-perfmon that

I didn't do that intentionally because it's called too late.
The function really has to be called early, so that the fallback
works.

> There is one question to patch "oprofile: discover counters for op
> ppro too". arch_perfmon_setup_counters() is actually never called for
> ppro, so there is no code that changes the numbers in
> op_ppro_spec. The patch as it is has no effect (except then loading
> additional module code).

Ah true, i must have deleted one hunk too much while refactoring.
It's ok to drop this patch for now.

-Andi

Re: Please pull arch perfmon update

[Robert Richter]

On 13.10.08 22:29:51, Andi Kleen wrote:
> > I added a follow on patch (446223f) on branch arch-perfmon that
> 
> I didn't do that intentionally because it's called too late.
> The function really has to be called early, so that the fallback
> works.

The hook is in op_nmi_init() and directly called after
arch_perfmon_init() and before init_sysfs(). Only
register_cpu_notifier() and the setup of oprofile_operations are in
between. This should work.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: Please pull arch perfmon update

[Andi Kleen]

On Tue, Oct 14, 2008 at 04:05:50PM +0200, Robert Richter wrote:
> On 13.10.08 22:29:51, Andi Kleen wrote:
> > > I added a follow on patch (446223f) on branch arch-perfmon that
> > 
> > I didn't do that intentionally because it's called too late.
> > The function really has to be called early, so that the fallback
> > works.
> 
> The hook is in op_nmi_init() and directly called after
> arch_perfmon_init() and before init_sysfs(). Only
> register_cpu_notifier() and the setup of oprofile_operations are in
> between. This should work.

The problem is that arch perfmon init should only be called after
the other initialization function failed.

So you would need a chain of op_x86_model_spec for fallback.

It's simpler and cleaner to just write that out in explicit C.

-Andi

Re: Please pull arch perfmon update

[Robert Richter]

On 14.10.08 16:15:07, Andi Kleen wrote:
> On Tue, Oct 14, 2008 at 04:05:50PM +0200, Robert Richter wrote:
> > On 13.10.08 22:29:51, Andi Kleen wrote:
> > > > I added a follow on patch (446223f) on branch arch-perfmon that
> > > 
> > > I didn't do that intentionally because it's called too late.
> > > The function really has to be called early, so that the fallback
> > > works.
> > 
> > The hook is in op_nmi_init() and directly called after
> > arch_perfmon_init() and before init_sysfs(). Only
> > register_cpu_notifier() and the setup of oprofile_operations are in
> > between. This should work.
> 
> The problem is that arch perfmon init should only be called after
> the other initialization function failed.
> 
> So you would need a chain of op_x86_model_spec for fallback.
> 
> It's simpler and cleaner to just write that out in explicit C.

The patch makes arch_perfmon_setup_counters() static, so the init code
is bound directly to the model (it is model specific code). The
interface is much cleaner now since the delaration as an external is
not needed then.

The init function is only called, if this cpu type (i386/arch_perfmon)
is selected. And this type is only selected after all other init
funtions were failing.

Please test the patch, I don't see a reason why it should not work.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: Please pull arch perfmon update

[Robert Richter]

On 13.10.08 22:29:51, Andi Kleen wrote:
> > There is one question to patch "oprofile: discover counters for op
> > ppro too". arch_perfmon_setup_counters() is actually never called for
> > ppro, so there is no code that changes the numbers in
> > op_ppro_spec. The patch as it is has no effect (except then loading
> > additional module code).
> 
> Ah true, i must have deleted one hunk too much while refactoring.
> It's ok to drop this patch for now.

I sent it upstream anyway, please send an additional patch that fixes
this.

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com