All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] updates for oprofile
@ 2010-04-23 15:40 Robert Richter
  2010-04-27  9:20 ` Ingo Molnar
  0 siblings, 1 reply; 19+ messages in thread
From: Robert Richter @ 2010-04-23 15:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list

Ingo,

please pull oprofile updates from:

  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

You might want to pull from 'for-next' instead that includes an
additional merge with tip/tracing/core to resolve conflicts with that
branch.

Thanks.

-Robert

--

commit b971f06187d83b5c03d2b597cccdfef421c0ca91
Merge: cb6e943 c1ab9ca
Author: Robert Richter <robert.richter@amd.com>
Date:   Fri Apr 23 16:47:51 2010 +0200

    Merge commit 'tip/tracing/core' into oprofile/core
    
    Conflicts:
        drivers/oprofile/cpu_buffer.c
    
    Signed-off-by: Robert Richter <robert.richter@amd.com>

commit cb6e943ccf19ab6d3189147e9d625a992e016084
Author: Andi Kleen <andi@firstfloor.org>
Date:   Thu Apr 1 03:17:25 2010 +0200

    oprofile: remove double ring buffering
    
    oprofile used a double buffer scheme for its cpu event buffer
    to avoid races on reading with the old locked ring buffer.
    
    But that is obsolete now with the new ring buffer, so simply
    use a single buffer. This greatly simplifies the code and avoids
    a lot of sample drops on large runs, especially with call graph.
    
    Based on suggestions from Steven Rostedt
    
    For stable kernels from v2.6.32, but not earlier.
    
    Signed-off-by: Andi Kleen <ak@linux.intel.com>
    Cc: Steven Rostedt <rostedt@goodmis.org>
    Cc: stable <stable@kernel.org>
    Signed-off-by: Robert Richter <robert.richter@amd.com>

commit a36bf32e9e8a86f291f746b7f8292e042ee04a46
Merge: bc078e4 01bf0b6
Author: Robert Richter <robert.richter@amd.com>
Date:   Fri Apr 23 14:30:22 2010 +0200

    Merge commit 'v2.6.34-rc5' into oprofile/core

commit bc078e4eab65f11bbaeed380593ab8151b30d703
Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
Date:   Tue Mar 2 16:01:10 2010 +0100

    oprofile: convert oprofile from timer_hook to hrtimer
    
    Oprofile is currently broken on systems running with NOHZ enabled.
    A maximum of 1 tick is accounted via the timer_hook if a cpu sleeps
    for a longer period of time. This does bad things to the percentages
    in the profiler output. To solve this problem convert oprofile to
    use a restarting hrtimer instead of the timer_hook.
    
    Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
    Signed-off-by: Robert Richter <robert.richter@amd.com>

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-04-23 15:40 Robert Richter
@ 2010-04-27  9:20 ` Ingo Molnar
  0 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2010-04-27  9:20 UTC (permalink / raw)
  To: Robert Richter
  Cc: LKML, oprofile-list, Steven Rostedt, Fr??d??ric Weisbecker,
	Peter Zijlstra, Arnaldo Carvalho de Melo


* Robert Richter <robert.richter@amd.com> wrote:

> Ingo,
> 
> please pull oprofile updates from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
> 
> You might want to pull from 'for-next' instead that includes an
> additional merge with tip/tracing/core to resolve conflicts with that
> branch.

Pulled, thanks Robert!

	Ingo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
@ 2010-04-27 15:25 Phil Carmody
  2010-04-27 17:40 ` Robert Richter
  0 siblings, 1 reply; 19+ messages in thread
From: Phil Carmody @ 2010-04-27 15:25 UTC (permalink / raw)
  To: robert.richter, schwidefsky, mingo; +Cc: linux-kernel

Ingo, et al., 

Regarding today's pulled request, containing:

commit bc078e4eab65f11bbaeed380593ab8151b30d703
Author: Martin Schwidefsky <schwidef...@de.ibm.com>
Date:   Tue Mar 2 16:01:10 2010 +0100

    oprofile: convert oprofile from timer_hook to hrtimer
    

Information is a touch scant, as I'm doing the investigation as I
write, but I believe that that patch can cause ooops regressions
via a null-pointer dereference in oprofile_add_sample().

That function declares:
"""
/**
 * Add a sample. This may be called from any context.
 */
void oprofile_add_sample(struct pt_regs * const regs, unsigned long event);
"""

And begins:
"""
void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
{
        int is_kernel = !user_mode(regs);
"""

Where on at least two major architectures (Arm, x86), user_mode()
unconditionally dereferences its parameter.

Now oprofile_add_sample() is called from this context:
"""
static enum hrtimer_restart oprofile_hrtimer_notify(struct hrtimer *hrtimer)
{
         oprofile_add_sample(get_irq_regs(), 0);
"""

And get_irq_regs() is NULL when not in an IRQ context.

Bang.

An example of this kind of thing kicking in has already been encountered 
last year:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg14069.html
(That thread got a little side-tracked onto OMAP specifics, but the 
original report is topical.)

Now would be a very good time for the "many eyes" principle to kick in.

I'm now looking into workarounds, but nothing that I'd necessarily
want to submit as a real fix.

Phil
cc:'d replies appreciated

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-04-27 15:25 Phil Carmody
@ 2010-04-27 17:40 ` Robert Richter
  2010-04-27 17:47   ` Siarhei Siamashka
  0 siblings, 1 reply; 19+ messages in thread
From: Robert Richter @ 2010-04-27 17:40 UTC (permalink / raw)
  To: Phil Carmody; +Cc: schwidefsky, mingo, linux-kernel, oprofile-list

(cc'ing oprofile-list)

On 27.04.10 18:25:44, Phil Carmody wrote:
> Ingo, et al., 
> 
> Regarding today's pulled request, containing:
> 
> commit bc078e4eab65f11bbaeed380593ab8151b30d703
> Author: Martin Schwidefsky <schwidef...@de.ibm.com>
> Date:   Tue Mar 2 16:01:10 2010 +0100
> 
>     oprofile: convert oprofile from timer_hook to hrtimer
>     
> 
> Information is a touch scant, as I'm doing the investigation as I
> write, but I believe that that patch can cause ooops regressions
> via a null-pointer dereference in oprofile_add_sample().
> 
> That function declares:
> """
> /**
>  * Add a sample. This may be called from any context.
>  */
> void oprofile_add_sample(struct pt_regs * const regs, unsigned long event);
> """
> 
> And begins:
> """
> void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
> {
>         int is_kernel = !user_mode(regs);
> """
> 
> Where on at least two major architectures (Arm, x86), user_mode()
> unconditionally dereferences its parameter.
> 
> Now oprofile_add_sample() is called from this context:
> """
> static enum hrtimer_restart oprofile_hrtimer_notify(struct hrtimer *hrtimer)
> {
>          oprofile_add_sample(get_irq_regs(), 0);
> """
> 
> And get_irq_regs() is NULL when not in an IRQ context.

Perf is simply dropping the sample in such cases, see:

 kernel/perf_event.c:perf_swevent_hrtimer()

So at quick fix would be to check for a null pointer also. But,
according to this:

 http://www.mail-archive.com/linux-omap@vger.kernel.org/msg14074.html

samples will be incorrect then since only interrupt context is
profiled. It seems there is no solution available right now.

-Robert

> 
> Bang.
> 
> An example of this kind of thing kicking in has already been encountered 
> last year:
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg14069.html
> (That thread got a little side-tracked onto OMAP specifics, but the 
> original report is topical.)
> 
> Now would be a very good time for the "many eyes" principle to kick in.
> 
> I'm now looking into workarounds, but nothing that I'd necessarily
> want to submit as a real fix.
> 
> Phil
> cc:'d replies appreciated
> 

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-04-27 17:40 ` Robert Richter
@ 2010-04-27 17:47   ` Siarhei Siamashka
  2010-04-28 16:59     ` Robert Richter
  0 siblings, 1 reply; 19+ messages in thread
From: Siarhei Siamashka @ 2010-04-27 17:47 UTC (permalink / raw)
  To: oprofile-list
  Cc: ext Robert Richter, Carmody Phil.2 (EXT-Ixonos/Helsinki),
	schwidefsky@de.ibm.com, linux-kernel@vger.kernel.org

On Tuesday 27 April 2010 20:40:26 ext Robert Richter wrote:
> On 27.04.10 18:25:44, Phil Carmody wrote:
> > Now oprofile_add_sample() is called from this context:
> > """
> > static enum hrtimer_restart oprofile_hrtimer_notify(struct hrtimer
> > *hrtimer) {
> >          oprofile_add_sample(get_irq_regs(), 0);
> > """
> >
> > And get_irq_regs() is NULL when not in an IRQ context.
>
> Perf is simply dropping the sample in such cases, see:
>
>  kernel/perf_event.c:perf_swevent_hrtimer()
>
> So at quick fix would be to check for a null pointer also. But,
> according to this:
>
>  http://www.mail-archive.com/linux-omap@vger.kernel.org/msg14074.html
>
> samples will be incorrect then since only interrupt context is
> profiled. It seems there is no solution available right now.

Isn't hrtimer callback function supposed to be only called from IRQ context
after this cleanup: http://lwn.net/Articles/308545/ ?

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-04-27 17:47   ` Siarhei Siamashka
@ 2010-04-28 16:59     ` Robert Richter
  2010-04-28 17:09       ` Phil Carmody
  0 siblings, 1 reply; 19+ messages in thread
From: Robert Richter @ 2010-04-28 16:59 UTC (permalink / raw)
  To: Siarhei Siamashka
  Cc: oprofile-list, Carmody Phil.2 (EXT-Ixonos/Helsinki),
	schwidefsky@de.ibm.com, linux-kernel@vger.kernel.org

On 27.04.10 20:47:51, Siarhei Siamashka wrote:
> Isn't hrtimer callback function supposed to be only called from IRQ context
> after this cleanup: http://lwn.net/Articles/308545/ ?

Yes, the patch is upstream since v2.6.29. Thanks Siarhei.

I will add a null pointer check anyway.

-Robert

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-04-28 16:59     ` Robert Richter
@ 2010-04-28 17:09       ` Phil Carmody
  2010-04-28 21:14         ` Robert Richter
  2010-05-03 21:18         ` Robert Richter
  0 siblings, 2 replies; 19+ messages in thread
From: Phil Carmody @ 2010-04-28 17:09 UTC (permalink / raw)
  To: ext Robert Richter
  Cc: Siamashka Siarhei (Nokia-D/Helsinki),
	oprofile-list@lists.sourceforge.net, schwidefsky@de.ibm.com,
	linux-kernel@vger.kernel.org

On 28/04/10 18:59 +0200, ext Robert Richter wrote:
> On 27.04.10 20:47:51, Siarhei Siamashka wrote:
> > Isn't hrtimer callback function supposed to be only called from IRQ context
> > after this cleanup: http://lwn.net/Articles/308545/ ?
> 
> Yes, the patch is upstream since v2.6.29. Thanks Siarhei.
> 
> I will add a null pointer check anyway.

A few here thrashed around a couple of ideas, and the general consensus 
was that the following work for us, and is offered for consideration.

Phil


From: Phil Carmody <ext-phil.2.carmody@nokia.com>
Date: Tue, 27 Apr 2010 19:28:33 +0300
Subject: [PATCH v2 1/1] oprofile: HACK - protect from not being in an IRQ context

http://lkml.org/lkml/2010/4/27/285

Protect against dereferencing regs when it's NULL, and
force a magic number into pc to prevent too deep processing.
This approach permits the dropped samples to be tallied as
invalid Instruction Pointer events.

e.g. output from about 15mins at 10kHz sample rate:
Nr. samples received: 2565380
Nr. samples lost invalid pc: 4

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
---
 drivers/oprofile/cpu_buffer.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index a7aae24..f70f954 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -357,9 +357,15 @@ void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
 
 void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
 {
-	int is_kernel = !user_mode(regs);
-	unsigned long pc = profile_pc(regs);
-
+	int is_kernel;
+	unsigned long pc;
+	if (likely(regs)) {
+		is_kernel = !user_mode(regs);
+		pc = profile_pc(regs);
+	} else {
+		is_kernel = 0;    /* This value will not be used */
+		pc = ESCAPE_CODE; /* as this causes an early return. */
+	}
 	__oprofile_add_ext_sample(pc, regs, event, is_kernel);
 }
 
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-04-28 17:09       ` Phil Carmody
@ 2010-04-28 21:14         ` Robert Richter
  2010-05-03 21:18         ` Robert Richter
  1 sibling, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-04-28 21:14 UTC (permalink / raw)
  To: Phil Carmody
  Cc: Siamashka Siarhei (Nokia-D/Helsinki),
	oprofile-list@lists.sourceforge.net, schwidefsky@de.ibm.com,
	linux-kernel@vger.kernel.org

On 28.04.10 12:09:16, Phil Carmody wrote:
> A few here thrashed around a couple of ideas, and the general consensus 
> was that the following work for us, and is offered for consideration.

Phil,

I missed your patch. I will apply your version in favour of my patch
since it adds statistic data.

Thanks,

-Robert

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-04-28 17:09       ` Phil Carmody
  2010-04-28 21:14         ` Robert Richter
@ 2010-05-03 21:18         ` Robert Richter
  1 sibling, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-03 21:18 UTC (permalink / raw)
  To: Phil Carmody
  Cc: Siamashka Siarhei (Nokia-D/Helsinki),
	oprofile-list@lists.sourceforge.net, schwidefsky@de.ibm.com,
	linux-kernel@vger.kernel.org

On 28.04.10 12:09:16, Phil Carmody wrote:
> On 28/04/10 18:59 +0200, ext Robert Richter wrote:
> > On 27.04.10 20:47:51, Siarhei Siamashka wrote:
> > > Isn't hrtimer callback function supposed to be only called from IRQ context
> > > after this cleanup: http://lwn.net/Articles/308545/ ?
> > 
> > Yes, the patch is upstream since v2.6.29. Thanks Siarhei.
> > 
> > I will add a null pointer check anyway.
> 
> A few here thrashed around a couple of ideas, and the general consensus 
> was that the following work for us, and is offered for consideration.
> 
> Phil
> 
> 
> From: Phil Carmody <ext-phil.2.carmody@nokia.com>
> Date: Tue, 27 Apr 2010 19:28:33 +0300
> Subject: [PATCH v2 1/1] oprofile: HACK - protect from not being in an IRQ context
> 
> http://lkml.org/lkml/2010/4/27/285
> 
> Protect against dereferencing regs when it's NULL, and
> force a magic number into pc to prevent too deep processing.
> This approach permits the dropped samples to be tallied as
> invalid Instruction Pointer events.
> 
> e.g. output from about 15mins at 10kHz sample rate:
> Nr. samples received: 2565380
> Nr. samples lost invalid pc: 4
> 
> Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>

Patch applied to:

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

Thanks Phil!

-Robert

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/7] updates for oprofile
@ 2010-05-04 10:44 Robert Richter
  2010-05-04 10:44 ` [PATCH 1/7] oprofile: update file list in MAINTAINERS file Robert Richter
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list

This patch set contains updates for oprofile, mainly with some reworks
of x86 code that improves the error handling.

-Robert



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/7] oprofile: update file list in MAINTAINERS file
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
@ 2010-05-04 10:44 ` Robert Richter
  2010-05-04 10:44 ` [PATCH 2/7] oprofile/x86: rework error handler in nmi_setup() Robert Richter
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list, Robert Richter

File list now catches:

 $ xargs | eval ls -d $(cat) | sort -u
 arch/*/include/asm/oprofile*.h
 arch/*/oprofile/
 drivers/oprofile/
 include/linux/oprofile.h

 arch/alpha/oprofile/
 arch/arm/oprofile/
 arch/avr32/oprofile/
 arch/blackfin/oprofile/
 arch/ia64/oprofile/
 arch/m32r/oprofile/
 arch/microblaze/oprofile/
 arch/mips/oprofile/
 arch/mn10300/oprofile/
 arch/parisc/oprofile/
 arch/powerpc/include/asm/oprofile_impl.h
 arch/powerpc/oprofile/
 arch/s390/oprofile/
 arch/sh/oprofile/
 arch/sparc/oprofile/
 arch/x86/oprofile/
 drivers/oprofile/
 include/linux/oprofile.h

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 MAINTAINERS |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a0e3c3a..31e8586 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4165,6 +4165,7 @@ OPROFILE
 M:	Robert Richter <robert.richter@amd.com>
 L:	oprofile-list@lists.sf.net
 S:	Maintained
+F:	arch/*/include/asm/oprofile*.h
 F:	arch/*/oprofile/
 F:	drivers/oprofile/
 F:	include/linux/oprofile.h
-- 
1.7.0.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/7] oprofile/x86: rework error handler in nmi_setup()
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
  2010-05-04 10:44 ` [PATCH 1/7] oprofile: update file list in MAINTAINERS file Robert Richter
@ 2010-05-04 10:44 ` Robert Richter
  2010-05-04 10:44 ` [PATCH 3/7] oprofile/x86: reserve counter msrs pairwise Robert Richter
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list, Robert Richter

This patch improves the error handler in nmi_setup(). Most parts of
the code are moved to allocate_msrs(). In case of an error
allocate_msrs() also frees already allocated memory. nmi_setup()
becomes easier and better extendable.

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 2c505ee..c0c21f2 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -295,6 +295,7 @@ static void free_msrs(void)
 		kfree(per_cpu(cpu_msrs, i).controls);
 		per_cpu(cpu_msrs, i).controls = NULL;
 	}
+	nmi_shutdown_mux();
 }
 
 static int allocate_msrs(void)
@@ -307,14 +308,21 @@ static int allocate_msrs(void)
 		per_cpu(cpu_msrs, i).counters = kzalloc(counters_size,
 							GFP_KERNEL);
 		if (!per_cpu(cpu_msrs, i).counters)
-			return 0;
+			goto fail;
 		per_cpu(cpu_msrs, i).controls = kzalloc(controls_size,
 							GFP_KERNEL);
 		if (!per_cpu(cpu_msrs, i).controls)
-			return 0;
+			goto fail;
 	}
 
+	if (!nmi_setup_mux())
+		goto fail;
+
 	return 1;
+
+fail:
+	free_msrs();
+	return 0;
 }
 
 static void nmi_cpu_setup(void *dummy)
@@ -342,17 +350,7 @@ static int nmi_setup(void)
 	int cpu;
 
 	if (!allocate_msrs())
-		err = -ENOMEM;
-	else if (!nmi_setup_mux())
-		err = -ENOMEM;
-	else
-		err = register_die_notifier(&profile_exceptions_nb);
-
-	if (err) {
-		free_msrs();
-		nmi_shutdown_mux();
-		return err;
-	}
+		return -ENOMEM;
 
 	/* We need to serialize save and setup for HT because the subset
 	 * of msrs are distinct for save and setup operations
@@ -374,9 +372,17 @@ static int nmi_setup(void)
 
 		mux_clone(cpu);
 	}
+
+	err = register_die_notifier(&profile_exceptions_nb);
+	if (err)
+		goto fail;
+
 	on_each_cpu(nmi_cpu_setup, NULL, 1);
 	nmi_enabled = 1;
 	return 0;
+fail:
+	free_msrs();
+	return err;
 }
 
 static void nmi_cpu_restore_registers(struct op_msrs *msrs)
@@ -421,7 +427,6 @@ static void nmi_shutdown(void)
 	nmi_enabled = 0;
 	on_each_cpu(nmi_cpu_shutdown, NULL, 1);
 	unregister_die_notifier(&profile_exceptions_nb);
-	nmi_shutdown_mux();
 	msrs = &get_cpu_var(cpu_msrs);
 	model->shutdown(msrs);
 	free_msrs();
-- 
1.7.0.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/7] oprofile/x86: reserve counter msrs pairwise
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
  2010-05-04 10:44 ` [PATCH 1/7] oprofile: update file list in MAINTAINERS file Robert Richter
  2010-05-04 10:44 ` [PATCH 2/7] oprofile/x86: rework error handler in nmi_setup() Robert Richter
@ 2010-05-04 10:44 ` Robert Richter
  2010-05-04 10:44 ` [PATCH 4/7] oprofile/x86: moving shutdown functions Robert Richter
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list, Robert Richter

For AMD's and Intel's P6 generic performance counters have pairwise
counter and control msrs. This patch changes the counter reservation
in a way that both msrs must be registered. It joins some counter
loops and also removes the unnecessary NUM_CONTROLS macro in the AMD
implementation.

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_amd.c  |   43 ++++++++++++++++--------------------
 arch/x86/oprofile/op_model_ppro.c |   36 ++++++++++++++----------------
 2 files changed, 36 insertions(+), 43 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index 090cbbe..2e2bc90 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -30,13 +30,10 @@
 #include "op_counter.h"
 
 #define NUM_COUNTERS 4
-#define NUM_CONTROLS 4
 #ifdef CONFIG_OPROFILE_EVENT_MULTIPLEX
 #define NUM_VIRT_COUNTERS 32
-#define NUM_VIRT_CONTROLS 32
 #else
 #define NUM_VIRT_COUNTERS NUM_COUNTERS
-#define NUM_VIRT_CONTROLS NUM_CONTROLS
 #endif
 
 #define OP_EVENT_MASK			0x0FFF
@@ -134,13 +131,15 @@ static void op_amd_fill_in_addresses(struct op_msrs * const msrs)
 	int i;
 
 	for (i = 0; i < NUM_COUNTERS; i++) {
-		if (reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i))
-			msrs->counters[i].addr = MSR_K7_PERFCTR0 + i;
-	}
-
-	for (i = 0; i < NUM_CONTROLS; i++) {
-		if (reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i))
-			msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i;
+		if (!reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i))
+			continue;
+		if (!reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i)) {
+			release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
+			continue;
+		}
+		/* both registers must be reserved */
+		msrs->counters[i].addr = MSR_K7_PERFCTR0 + i;
+		msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i;
 	}
 }
 
@@ -160,7 +159,7 @@ static void op_amd_setup_ctrs(struct op_x86_model_spec const *model,
 	}
 
 	/* clear all counters */
-	for (i = 0; i < NUM_CONTROLS; ++i) {
+	for (i = 0; i < NUM_COUNTERS; ++i) {
 		if (unlikely(!msrs->controls[i].addr)) {
 			if (counter_config[i].enabled && !smp_processor_id())
 				/*
@@ -175,12 +174,10 @@ static void op_amd_setup_ctrs(struct op_x86_model_spec const *model,
 			op_x86_warn_in_use(i);
 		val &= model->reserved;
 		wrmsrl(msrs->controls[i].addr, val);
-	}
-
-	/* avoid a false detection of ctr overflows in NMI handler */
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if (unlikely(!msrs->counters[i].addr))
-			continue;
+		/*
+		 * avoid a false detection of ctr overflows in NMI
+		 * handler
+		 */
 		wrmsrl(msrs->counters[i].addr, -1LL);
 	}
 
@@ -430,12 +427,10 @@ static void op_amd_shutdown(struct op_msrs const * const msrs)
 	int i;
 
 	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if (msrs->counters[i].addr)
-			release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
-	}
-	for (i = 0; i < NUM_CONTROLS; ++i) {
-		if (msrs->controls[i].addr)
-			release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
+		if (!msrs->counters[i].addr)
+			continue;
+		release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
+		release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
 	}
 }
 
@@ -583,7 +578,7 @@ static void op_amd_exit(void)
 
 struct op_x86_model_spec op_amd_spec = {
 	.num_counters		= NUM_COUNTERS,
-	.num_controls		= NUM_CONTROLS,
+	.num_controls		= NUM_COUNTERS,
 	.num_virt_counters	= NUM_VIRT_COUNTERS,
 	.reserved		= MSR_AMD_EVENTSEL_RESERVED,
 	.event_mask		= OP_EVENT_MASK,
diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
index 2bf90fa..f8e268e 100644
--- a/arch/x86/oprofile/op_model_ppro.c
+++ b/arch/x86/oprofile/op_model_ppro.c
@@ -35,13 +35,15 @@ static void ppro_fill_in_addresses(struct op_msrs * const msrs)
 	int i;
 
 	for (i = 0; i < num_counters; i++) {
-		if (reserve_perfctr_nmi(MSR_P6_PERFCTR0 + i))
-			msrs->counters[i].addr = MSR_P6_PERFCTR0 + i;
-	}
-
-	for (i = 0; i < num_counters; i++) {
-		if (reserve_evntsel_nmi(MSR_P6_EVNTSEL0 + i))
-			msrs->controls[i].addr = MSR_P6_EVNTSEL0 + i;
+		if (!reserve_perfctr_nmi(MSR_P6_PERFCTR0 + i))
+			continue;
+		if (!reserve_evntsel_nmi(MSR_P6_EVNTSEL0 + i)) {
+			release_perfctr_nmi(MSR_P6_PERFCTR0 + i);
+			continue;
+		}
+		/* both registers must be reserved */
+		msrs->counters[i].addr = MSR_P6_PERFCTR0 + i;
+		msrs->controls[i].addr = MSR_P6_EVNTSEL0 + i;
 	}
 }
 
@@ -92,12 +94,10 @@ static void ppro_setup_ctrs(struct op_x86_model_spec const *model,
 			op_x86_warn_in_use(i);
 		val &= model->reserved;
 		wrmsrl(msrs->controls[i].addr, val);
-	}
-
-	/* avoid a false detection of ctr overflows in NMI handler */
-	for (i = 0; i < num_counters; ++i) {
-		if (unlikely(!msrs->counters[i].addr))
-			continue;
+		/*
+		 * avoid a false detection of ctr overflows in NMI *
+		 * handler
+		 */
 		wrmsrl(msrs->counters[i].addr, -1LL);
 	}
 
@@ -194,12 +194,10 @@ static void ppro_shutdown(struct op_msrs const * const msrs)
 	int i;
 
 	for (i = 0; i < num_counters; ++i) {
-		if (msrs->counters[i].addr)
-			release_perfctr_nmi(MSR_P6_PERFCTR0 + i);
-	}
-	for (i = 0; i < num_counters; ++i) {
-		if (msrs->controls[i].addr)
-			release_evntsel_nmi(MSR_P6_EVNTSEL0 + i);
+		if (!msrs->counters[i].addr)
+			continue;
+		release_perfctr_nmi(MSR_P6_PERFCTR0 + i);
+		release_evntsel_nmi(MSR_P6_EVNTSEL0 + i);
 	}
 	if (reset_value) {
 		kfree(reset_value);
-- 
1.7.0.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/7] oprofile/x86: moving shutdown functions
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
                   ` (2 preceding siblings ...)
  2010-05-04 10:44 ` [PATCH 3/7] oprofile/x86: reserve counter msrs pairwise Robert Richter
@ 2010-05-04 10:44 ` Robert Richter
  2010-05-04 10:44 ` [PATCH 5/7] oprofile/x86: return -EBUSY if counters are already reserved Robert Richter
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list, Robert Richter

Moving some code in preparation of the next patch.

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_amd.c  |   24 +++++++++++-----------
 arch/x86/oprofile/op_model_p4.c   |   38 +++++++++++++++++-------------------
 arch/x86/oprofile/op_model_ppro.c |   33 +++++++++++++++----------------
 3 files changed, 46 insertions(+), 49 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index 2e2bc90..7e5886d 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -126,6 +126,18 @@ static void op_mux_switch_ctrl(struct op_x86_model_spec const *model,
 
 /* functions for op_amd_spec */
 
+static void op_amd_shutdown(struct op_msrs const * const msrs)
+{
+	int i;
+
+	for (i = 0; i < NUM_COUNTERS; ++i) {
+		if (!msrs->counters[i].addr)
+			continue;
+		release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
+		release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
+	}
+}
+
 static void op_amd_fill_in_addresses(struct op_msrs * const msrs)
 {
 	int i;
@@ -422,18 +434,6 @@ static void op_amd_stop(struct op_msrs const * const msrs)
 	op_amd_stop_ibs();
 }
 
-static void op_amd_shutdown(struct op_msrs const * const msrs)
-{
-	int i;
-
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if (!msrs->counters[i].addr)
-			continue;
-		release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
-		release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
-	}
-}
-
 static u8 ibs_eilvt_off;
 
 static inline void apic_init_ibs_nmi_per_cpu(void *arg)
diff --git a/arch/x86/oprofile/op_model_p4.c b/arch/x86/oprofile/op_model_p4.c
index e6a160a..7cc80df 100644
--- a/arch/x86/oprofile/op_model_p4.c
+++ b/arch/x86/oprofile/op_model_p4.c
@@ -385,6 +385,24 @@ static unsigned int get_stagger(void)
 
 static unsigned long reset_value[NUM_COUNTERS_NON_HT];
 
+static void p4_shutdown(struct op_msrs const * const msrs)
+{
+	int i;
+
+	for (i = 0; i < num_counters; ++i) {
+		if (msrs->counters[i].addr)
+			release_perfctr_nmi(msrs->counters[i].addr);
+	}
+	/*
+	 * some of the control registers are specially reserved in
+	 * conjunction with the counter registers (hence the starting offset).
+	 * This saves a few bits.
+	 */
+	for (i = num_counters; i < num_controls; ++i) {
+		if (msrs->controls[i].addr)
+			release_evntsel_nmi(msrs->controls[i].addr);
+	}
+}
 
 static void p4_fill_in_addresses(struct op_msrs * const msrs)
 {
@@ -668,26 +686,6 @@ static void p4_stop(struct op_msrs const * const msrs)
 	}
 }
 
-static void p4_shutdown(struct op_msrs const * const msrs)
-{
-	int i;
-
-	for (i = 0; i < num_counters; ++i) {
-		if (msrs->counters[i].addr)
-			release_perfctr_nmi(msrs->counters[i].addr);
-	}
-	/*
-	 * some of the control registers are specially reserved in
-	 * conjunction with the counter registers (hence the starting offset).
-	 * This saves a few bits.
-	 */
-	for (i = num_counters; i < num_controls; ++i) {
-		if (msrs->controls[i].addr)
-			release_evntsel_nmi(msrs->controls[i].addr);
-	}
-}
-
-
 #ifdef CONFIG_SMP
 struct op_x86_model_spec op_p4_ht2_spec = {
 	.num_counters		= NUM_COUNTERS_HT2,
diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
index f8e268e..b07d25a 100644
--- a/arch/x86/oprofile/op_model_ppro.c
+++ b/arch/x86/oprofile/op_model_ppro.c
@@ -30,6 +30,22 @@ static int counter_width = 32;
 
 static u64 *reset_value;
 
+static void ppro_shutdown(struct op_msrs const * const msrs)
+{
+	int i;
+
+	for (i = 0; i < num_counters; ++i) {
+		if (!msrs->counters[i].addr)
+			continue;
+		release_perfctr_nmi(MSR_P6_PERFCTR0 + i);
+		release_evntsel_nmi(MSR_P6_EVNTSEL0 + i);
+	}
+	if (reset_value) {
+		kfree(reset_value);
+		reset_value = NULL;
+	}
+}
+
 static void ppro_fill_in_addresses(struct op_msrs * const msrs)
 {
 	int i;
@@ -189,23 +205,6 @@ static void ppro_stop(struct op_msrs const * const msrs)
 	}
 }
 
-static void ppro_shutdown(struct op_msrs const * const msrs)
-{
-	int i;
-
-	for (i = 0; i < num_counters; ++i) {
-		if (!msrs->counters[i].addr)
-			continue;
-		release_perfctr_nmi(MSR_P6_PERFCTR0 + i);
-		release_evntsel_nmi(MSR_P6_EVNTSEL0 + i);
-	}
-	if (reset_value) {
-		kfree(reset_value);
-		reset_value = NULL;
-	}
-}
-
-
 struct op_x86_model_spec op_ppro_spec = {
 	.num_counters		= 2,
 	.num_controls		= 2,
-- 
1.7.0.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/7] oprofile/x86: return -EBUSY if counters are already reserved
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
                   ` (3 preceding siblings ...)
  2010-05-04 10:44 ` [PATCH 4/7] oprofile/x86: moving shutdown functions Robert Richter
@ 2010-05-04 10:44 ` Robert Richter
  2010-05-04 10:44 ` [PATCH 6/7] oprofile/x86: move IBS code Robert Richter
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list, Robert Richter

In case a counter is already reserved by the watchdog or perf_event
subsystem, oprofile ignored this counters silently. This case is
handled now and oprofile_setup() now reports an error.

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c       |    5 ++++-
 arch/x86/oprofile/op_model_amd.c  |   24 +++++++++++++-----------
 arch/x86/oprofile/op_model_p4.c   |   14 +++++++++++++-
 arch/x86/oprofile/op_model_ppro.c |   24 +++++++++++++-----------
 arch/x86/oprofile/op_x86_model.h  |    2 +-
 5 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index c0c21f2..9f001d9 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -357,7 +357,10 @@ static int nmi_setup(void)
 	 */
 
 	/* Assume saved/restored counters are the same on all CPUs */
-	model->fill_in_addresses(&per_cpu(cpu_msrs, 0));
+	err = model->fill_in_addresses(&per_cpu(cpu_msrs, 0));
+	if (err)
+		goto fail;
+
 	for_each_possible_cpu(cpu) {
 		if (!cpu)
 			continue;
diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index 7e5886d..536d0b0 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -138,21 +138,30 @@ static void op_amd_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
-static void op_amd_fill_in_addresses(struct op_msrs * const msrs)
+static int op_amd_fill_in_addresses(struct op_msrs * const msrs)
 {
 	int i;
 
 	for (i = 0; i < NUM_COUNTERS; i++) {
 		if (!reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i))
-			continue;
+			goto fail;
 		if (!reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i)) {
 			release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
-			continue;
+			goto fail;
 		}
 		/* both registers must be reserved */
 		msrs->counters[i].addr = MSR_K7_PERFCTR0 + i;
 		msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i;
+		continue;
+	fail:
+		if (!counter_config[i].enabled)
+			continue;
+		op_x86_warn_reserved(i);
+		op_amd_shutdown(msrs);
+		return -EBUSY;
 	}
+
+	return 0;
 }
 
 static void op_amd_setup_ctrs(struct op_x86_model_spec const *model,
@@ -172,15 +181,8 @@ static void op_amd_setup_ctrs(struct op_x86_model_spec const *model,
 
 	/* clear all counters */
 	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if (unlikely(!msrs->controls[i].addr)) {
-			if (counter_config[i].enabled && !smp_processor_id())
-				/*
-				 * counter is reserved, this is on all
-				 * cpus, so report only for cpu #0
-				 */
-				op_x86_warn_reserved(i);
+		if (!msrs->controls[i].addr)
 			continue;
-		}
 		rdmsrl(msrs->controls[i].addr, val);
 		if (val & ARCH_PERFMON_EVENTSEL_ENABLE)
 			op_x86_warn_in_use(i);
diff --git a/arch/x86/oprofile/op_model_p4.c b/arch/x86/oprofile/op_model_p4.c
index 7cc80df..182558d 100644
--- a/arch/x86/oprofile/op_model_p4.c
+++ b/arch/x86/oprofile/op_model_p4.c
@@ -404,7 +404,7 @@ static void p4_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
-static void p4_fill_in_addresses(struct op_msrs * const msrs)
+static int p4_fill_in_addresses(struct op_msrs * const msrs)
 {
 	unsigned int i;
 	unsigned int addr, cccraddr, stag;
@@ -486,6 +486,18 @@ static void p4_fill_in_addresses(struct op_msrs * const msrs)
 			msrs->controls[i++].addr = MSR_P4_CRU_ESCR5;
 		}
 	}
+
+	for (i = 0; i < num_counters; ++i) {
+		if (!counter_config[i].enabled)
+			continue;
+		if (msrs->controls[i].addr)
+			continue;
+		op_x86_warn_reserved(i);
+		p4_shutdown(msrs);
+		return -EBUSY;
+	}
+
+	return 0;
 }
 
 
diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
index b07d25a..1fd17cf 100644
--- a/arch/x86/oprofile/op_model_ppro.c
+++ b/arch/x86/oprofile/op_model_ppro.c
@@ -46,21 +46,30 @@ static void ppro_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
-static void ppro_fill_in_addresses(struct op_msrs * const msrs)
+static int ppro_fill_in_addresses(struct op_msrs * const msrs)
 {
 	int i;
 
 	for (i = 0; i < num_counters; i++) {
 		if (!reserve_perfctr_nmi(MSR_P6_PERFCTR0 + i))
-			continue;
+			goto fail;
 		if (!reserve_evntsel_nmi(MSR_P6_EVNTSEL0 + i)) {
 			release_perfctr_nmi(MSR_P6_PERFCTR0 + i);
-			continue;
+			goto fail;
 		}
 		/* both registers must be reserved */
 		msrs->counters[i].addr = MSR_P6_PERFCTR0 + i;
 		msrs->controls[i].addr = MSR_P6_EVNTSEL0 + i;
+		continue;
+	fail:
+		if (!counter_config[i].enabled)
+			continue;
+		op_x86_warn_reserved(i);
+		ppro_shutdown(msrs);
+		return -EBUSY;
 	}
+
+	return 0;
 }
 
 
@@ -96,15 +105,8 @@ static void ppro_setup_ctrs(struct op_x86_model_spec const *model,
 
 	/* clear all counters */
 	for (i = 0; i < num_counters; ++i) {
-		if (unlikely(!msrs->controls[i].addr)) {
-			if (counter_config[i].enabled && !smp_processor_id())
-				/*
-				 * counter is reserved, this is on all
-				 * cpus, so report only for cpu #0
-				 */
-				op_x86_warn_reserved(i);
+		if (!msrs->controls[i].addr)
 			continue;
-		}
 		rdmsrl(msrs->controls[i].addr, val);
 		if (val & ARCH_PERFMON_EVENTSEL_ENABLE)
 			op_x86_warn_in_use(i);
diff --git a/arch/x86/oprofile/op_x86_model.h b/arch/x86/oprofile/op_x86_model.h
index ff82a75..5514013 100644
--- a/arch/x86/oprofile/op_x86_model.h
+++ b/arch/x86/oprofile/op_x86_model.h
@@ -41,7 +41,7 @@ struct op_x86_model_spec {
 	u16		event_mask;
 	int		(*init)(struct oprofile_operations *ops);
 	void		(*exit)(void);
-	void		(*fill_in_addresses)(struct op_msrs * const msrs);
+	int		(*fill_in_addresses)(struct op_msrs * const msrs);
 	void		(*setup_ctrs)(struct op_x86_model_spec const *model,
 				      struct op_msrs const * const msrs);
 	int		(*check_ctrs)(struct pt_regs * const regs,
-- 
1.7.0.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/7] oprofile/x86: move IBS code
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
                   ` (4 preceding siblings ...)
  2010-05-04 10:44 ` [PATCH 5/7] oprofile/x86: return -EBUSY if counters are already reserved Robert Richter
@ 2010-05-04 10:44 ` Robert Richter
  2010-05-04 10:44 ` [PATCH 7/7] oprofile/x86: remove duplicate IBS capability check Robert Richter
  2010-05-06 13:03 ` [GIT PULL] updates for oprofile Robert Richter
  7 siblings, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list, Robert Richter

Moving code to make future changes easier. This groups all IBS code
together.

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_amd.c |  220 +++++++++++++++++++-------------------
 1 files changed, 110 insertions(+), 110 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index 536d0b0..e159254 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -102,116 +102,6 @@ static u32 get_ibs_caps(void)
 	return ibs_caps;
 }
 
-#ifdef CONFIG_OPROFILE_EVENT_MULTIPLEX
-
-static void op_mux_switch_ctrl(struct op_x86_model_spec const *model,
-			       struct op_msrs const * const msrs)
-{
-	u64 val;
-	int i;
-
-	/* enable active counters */
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		int virt = op_x86_phys_to_virt(i);
-		if (!reset_value[virt])
-			continue;
-		rdmsrl(msrs->controls[i].addr, val);
-		val &= model->reserved;
-		val |= op_x86_get_ctrl(model, &counter_config[virt]);
-		wrmsrl(msrs->controls[i].addr, val);
-	}
-}
-
-#endif
-
-/* functions for op_amd_spec */
-
-static void op_amd_shutdown(struct op_msrs const * const msrs)
-{
-	int i;
-
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if (!msrs->counters[i].addr)
-			continue;
-		release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
-		release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
-	}
-}
-
-static int op_amd_fill_in_addresses(struct op_msrs * const msrs)
-{
-	int i;
-
-	for (i = 0; i < NUM_COUNTERS; i++) {
-		if (!reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i))
-			goto fail;
-		if (!reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i)) {
-			release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
-			goto fail;
-		}
-		/* both registers must be reserved */
-		msrs->counters[i].addr = MSR_K7_PERFCTR0 + i;
-		msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i;
-		continue;
-	fail:
-		if (!counter_config[i].enabled)
-			continue;
-		op_x86_warn_reserved(i);
-		op_amd_shutdown(msrs);
-		return -EBUSY;
-	}
-
-	return 0;
-}
-
-static void op_amd_setup_ctrs(struct op_x86_model_spec const *model,
-			      struct op_msrs const * const msrs)
-{
-	u64 val;
-	int i;
-
-	/* setup reset_value */
-	for (i = 0; i < NUM_VIRT_COUNTERS; ++i) {
-		if (counter_config[i].enabled
-		    && msrs->counters[op_x86_virt_to_phys(i)].addr)
-			reset_value[i] = counter_config[i].count;
-		else
-			reset_value[i] = 0;
-	}
-
-	/* clear all counters */
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if (!msrs->controls[i].addr)
-			continue;
-		rdmsrl(msrs->controls[i].addr, val);
-		if (val & ARCH_PERFMON_EVENTSEL_ENABLE)
-			op_x86_warn_in_use(i);
-		val &= model->reserved;
-		wrmsrl(msrs->controls[i].addr, val);
-		/*
-		 * avoid a false detection of ctr overflows in NMI
-		 * handler
-		 */
-		wrmsrl(msrs->counters[i].addr, -1LL);
-	}
-
-	/* enable active counters */
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		int virt = op_x86_phys_to_virt(i);
-		if (!reset_value[virt])
-			continue;
-
-		/* setup counter registers */
-		wrmsrl(msrs->counters[i].addr, -(u64)reset_value[virt]);
-
-		/* setup control registers */
-		rdmsrl(msrs->controls[i].addr, val);
-		val &= model->reserved;
-		val |= op_x86_get_ctrl(model, &counter_config[virt]);
-		wrmsrl(msrs->controls[i].addr, val);
-	}
-}
-
 /*
  * 16-bit Linear Feedback Shift Register (LFSR)
  *
@@ -376,6 +266,116 @@ static void op_amd_stop_ibs(void)
 		wrmsrl(MSR_AMD64_IBSOPCTL, 0);
 }
 
+#ifdef CONFIG_OPROFILE_EVENT_MULTIPLEX
+
+static void op_mux_switch_ctrl(struct op_x86_model_spec const *model,
+			       struct op_msrs const * const msrs)
+{
+	u64 val;
+	int i;
+
+	/* enable active counters */
+	for (i = 0; i < NUM_COUNTERS; ++i) {
+		int virt = op_x86_phys_to_virt(i);
+		if (!reset_value[virt])
+			continue;
+		rdmsrl(msrs->controls[i].addr, val);
+		val &= model->reserved;
+		val |= op_x86_get_ctrl(model, &counter_config[virt]);
+		wrmsrl(msrs->controls[i].addr, val);
+	}
+}
+
+#endif
+
+/* functions for op_amd_spec */
+
+static void op_amd_shutdown(struct op_msrs const * const msrs)
+{
+	int i;
+
+	for (i = 0; i < NUM_COUNTERS; ++i) {
+		if (!msrs->counters[i].addr)
+			continue;
+		release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
+		release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
+	}
+}
+
+static int op_amd_fill_in_addresses(struct op_msrs * const msrs)
+{
+	int i;
+
+	for (i = 0; i < NUM_COUNTERS; i++) {
+		if (!reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i))
+			goto fail;
+		if (!reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i)) {
+			release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
+			goto fail;
+		}
+		/* both registers must be reserved */
+		msrs->counters[i].addr = MSR_K7_PERFCTR0 + i;
+		msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i;
+		continue;
+	fail:
+		if (!counter_config[i].enabled)
+			continue;
+		op_x86_warn_reserved(i);
+		op_amd_shutdown(msrs);
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static void op_amd_setup_ctrs(struct op_x86_model_spec const *model,
+			      struct op_msrs const * const msrs)
+{
+	u64 val;
+	int i;
+
+	/* setup reset_value */
+	for (i = 0; i < NUM_VIRT_COUNTERS; ++i) {
+		if (counter_config[i].enabled
+		    && msrs->counters[op_x86_virt_to_phys(i)].addr)
+			reset_value[i] = counter_config[i].count;
+		else
+			reset_value[i] = 0;
+	}
+
+	/* clear all counters */
+	for (i = 0; i < NUM_COUNTERS; ++i) {
+		if (!msrs->controls[i].addr)
+			continue;
+		rdmsrl(msrs->controls[i].addr, val);
+		if (val & ARCH_PERFMON_EVENTSEL_ENABLE)
+			op_x86_warn_in_use(i);
+		val &= model->reserved;
+		wrmsrl(msrs->controls[i].addr, val);
+		/*
+		 * avoid a false detection of ctr overflows in NMI
+		 * handler
+		 */
+		wrmsrl(msrs->counters[i].addr, -1LL);
+	}
+
+	/* enable active counters */
+	for (i = 0; i < NUM_COUNTERS; ++i) {
+		int virt = op_x86_phys_to_virt(i);
+		if (!reset_value[virt])
+			continue;
+
+		/* setup counter registers */
+		wrmsrl(msrs->counters[i].addr, -(u64)reset_value[virt]);
+
+		/* setup control registers */
+		rdmsrl(msrs->controls[i].addr, val);
+		val &= model->reserved;
+		val |= op_x86_get_ctrl(model, &counter_config[virt]);
+		wrmsrl(msrs->controls[i].addr, val);
+	}
+}
+
 static int op_amd_check_ctrs(struct pt_regs * const regs,
 			     struct op_msrs const * const msrs)
 {
-- 
1.7.0.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/7] oprofile/x86: remove duplicate IBS capability check
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
                   ` (5 preceding siblings ...)
  2010-05-04 10:44 ` [PATCH 6/7] oprofile/x86: move IBS code Robert Richter
@ 2010-05-04 10:44 ` Robert Richter
  2010-05-06 13:03 ` [GIT PULL] updates for oprofile Robert Richter
  7 siblings, 0 replies; 19+ messages in thread
From: Robert Richter @ 2010-05-04 10:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list, Robert Richter

The check is already done in ibs_exit().

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_amd.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index e159254..384c524 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -490,8 +490,7 @@ static int init_ibs_nmi(void)
 /* uninitialize the APIC for the IBS interrupts if needed */
 static void clear_ibs_nmi(void)
 {
-	if (ibs_caps)
-		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
+	on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
 }
 
 /* initialize the APIC for the IBS interrupts if available */
-- 
1.7.0.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [GIT PULL] updates for oprofile
  2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
                   ` (6 preceding siblings ...)
  2010-05-04 10:44 ` [PATCH 7/7] oprofile/x86: remove duplicate IBS capability check Robert Richter
@ 2010-05-06 13:03 ` Robert Richter
  2010-05-06 14:21   ` Ingo Molnar
  7 siblings, 1 reply; 19+ messages in thread
From: Robert Richter @ 2010-05-06 13:03 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML, oprofile-list

Ingo,

please pull oprofile updates for tip from

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

Thanks,

-Robert

-- 

The following changes since commit 9414e99672271adcc661f3c160a30b374179b92f:
  Phil Carmody (1):
        oprofile: protect from not being in an IRQ context

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

Robert Richter (7):
      oprofile: update file list in MAINTAINERS file
      oprofile/x86: rework error handler in nmi_setup()
      oprofile/x86: reserve counter msrs pairwise
      oprofile/x86: moving shutdown functions
      oprofile/x86: return -EBUSY if counters are already reserved
      oprofile/x86: move IBS code
      oprofile/x86: remove duplicate IBS capability check

 MAINTAINERS                       |    1 +
 arch/x86/oprofile/nmi_int.c       |   38 ++++---
 arch/x86/oprofile/op_model_amd.c  |  228 ++++++++++++++++++-------------------
 arch/x86/oprofile/op_model_p4.c   |   52 +++++----
 arch/x86/oprofile/op_model_ppro.c |   77 ++++++-------
 arch/x86/oprofile/op_x86_model.h  |    2 +-
 6 files changed, 206 insertions(+), 192 deletions(-)

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [GIT PULL] updates for oprofile
  2010-05-06 13:03 ` [GIT PULL] updates for oprofile Robert Richter
@ 2010-05-06 14:21   ` Ingo Molnar
  0 siblings, 0 replies; 19+ messages in thread
From: Ingo Molnar @ 2010-05-06 14:21 UTC (permalink / raw)
  To: Robert Richter; +Cc: LKML, oprofile-list


* Robert Richter <robert.richter@amd.com> wrote:

> Ingo,
> 
> please pull oprofile updates for tip from
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
> 
> Thanks,
> 
> -Robert
> 
> -- 
> 
> The following changes since commit 9414e99672271adcc661f3c160a30b374179b92f:
>   Phil Carmody (1):
>         oprofile: protect from not being in an IRQ context
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
> 
> Robert Richter (7):
>       oprofile: update file list in MAINTAINERS file
>       oprofile/x86: rework error handler in nmi_setup()
>       oprofile/x86: reserve counter msrs pairwise
>       oprofile/x86: moving shutdown functions
>       oprofile/x86: return -EBUSY if counters are already reserved
>       oprofile/x86: move IBS code
>       oprofile/x86: remove duplicate IBS capability check
> 
>  MAINTAINERS                       |    1 +
>  arch/x86/oprofile/nmi_int.c       |   38 ++++---
>  arch/x86/oprofile/op_model_amd.c  |  228 ++++++++++++++++++-------------------
>  arch/x86/oprofile/op_model_p4.c   |   52 +++++----
>  arch/x86/oprofile/op_model_ppro.c |   77 ++++++-------
>  arch/x86/oprofile/op_x86_model.h  |    2 +-
>  6 files changed, 206 insertions(+), 192 deletions(-)

Pulled, thanks Robert!

	Ingo

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2010-05-06 14:21 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-04 10:44 [PATCH 0/7] updates for oprofile Robert Richter
2010-05-04 10:44 ` [PATCH 1/7] oprofile: update file list in MAINTAINERS file Robert Richter
2010-05-04 10:44 ` [PATCH 2/7] oprofile/x86: rework error handler in nmi_setup() Robert Richter
2010-05-04 10:44 ` [PATCH 3/7] oprofile/x86: reserve counter msrs pairwise Robert Richter
2010-05-04 10:44 ` [PATCH 4/7] oprofile/x86: moving shutdown functions Robert Richter
2010-05-04 10:44 ` [PATCH 5/7] oprofile/x86: return -EBUSY if counters are already reserved Robert Richter
2010-05-04 10:44 ` [PATCH 6/7] oprofile/x86: move IBS code Robert Richter
2010-05-04 10:44 ` [PATCH 7/7] oprofile/x86: remove duplicate IBS capability check Robert Richter
2010-05-06 13:03 ` [GIT PULL] updates for oprofile Robert Richter
2010-05-06 14:21   ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2010-04-27 15:25 Phil Carmody
2010-04-27 17:40 ` Robert Richter
2010-04-27 17:47   ` Siarhei Siamashka
2010-04-28 16:59     ` Robert Richter
2010-04-28 17:09       ` Phil Carmody
2010-04-28 21:14         ` Robert Richter
2010-05-03 21:18         ` Robert Richter
2010-04-23 15:40 Robert Richter
2010-04-27  9:20 ` Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.