Re: Please pull arch perfmon update

Previous thread: [RFC PATCH 1/2] Shrink compat_ioctl.c by Matt Mackall on Monday, September 29, 2008 - 4:27 pm. (4 messages)

Next thread: Strange mtrrs in Aspire One by J.A. on Monday, September 29, 2008 - 4:57 pm. (8 messages)
From: Andi Kleen
Date: Monday, September 29, 2008 - 4:52 pm

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
--

From: Robert Richter
Date: Monday, October 13, 2008 - 11:35 am

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 = ...
From: Andi Kleen
Date: Monday, October 13, 2008 - 1:29 pm

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

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

-Andi

--

From: Robert Richter
Date: Tuesday, October 14, 2008 - 7:05 am

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

--

From: Andi Kleen
Date: Tuesday, October 14, 2008 - 7:15 am

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
--

From: Robert Richter
Date: Tuesday, October 14, 2008 - 7:35 am

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

--

From: Robert Richter
Date: Tuesday, October 14, 2008 - 9:01 am

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

--

Previous thread: [RFC PATCH 1/2] Shrink compat_ioctl.c by Matt Mackall on Monday, September 29, 2008 - 4:27 pm. (4 messages)

Next thread: Strange mtrrs in Aspire One by J.A. on Monday, September 29, 2008 - 4:57 pm. (8 messages)