[PATCH] oprofile, powerpc: Handle events that raise an exception without overflowing

[PATCH] oprofile, powerpc: Handle events that raise an exception without overflowing

[Eric B Munson]

Commit 0837e3242c73566fc1c0196b4ec61779c25ffc93 fixes a situation on POWER7
where events can roll back if a specualtive event doesn't actually complete.
This can raise a performance monitor exception.  We need to catch this to ensure
that we reset the PMC.  In all cases the PMC will be less than 256 cycles from
overflow.

This patch lifts Anton's fix for the problem in perf and applies it to oprofile
as well.

Signed-off-by: Eric B Munson <emunson@mgebm.net>
Cc: <stable@kernel.org> # as far back as it applies cleanly
---
 arch/powerpc/oprofile/op_model_power4.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/oprofile/op_model_power4.c b/arch/powerpc/oprofile/op_model_power4.c
index 8ee51a2..e6bec74 100644
--- a/arch/powerpc/oprofile/op_model_power4.c
+++ b/arch/powerpc/oprofile/op_model_power4.c
@@ -261,6 +261,28 @@ static int get_kernel(unsigned long pc, unsigned long mmcra)
 	return is_kernel;
 }
 
+static bool pmc_overflow(unsigned long val)
+{
+	if ((int)val < 0)
+		return true;
+
+	/*
+	 * Events on POWER7 can roll back if a speculative event doesn't
+	 * eventually complete. Unfortunately in some rare cases they will
+	 * raise a performance monitor exception. We need to catch this to
+	 * ensure we reset the PMC. In all cases the PMC will be 256 or less
+	 * cycles from overflow.
+	 *
+	 * We only do this if the first pass fails to find any overflowing
+	 * PMCs because a user might set a period of less than 256 and we
+	 * don't want to mistakenly reset them.
+	 */
+	if (__is_processor(PV_POWER7) && ((0x80000000 - val) <= 256))
+		return true;
+
+	return false;
+}
+
 static void power4_handle_interrupt(struct pt_regs *regs,
 				    struct op_counter_config *ctr)
 {
@@ -281,7 +303,7 @@ static void power4_handle_interrupt(struct pt_regs *regs,
 
 	for (i = 0; i < cur_cpu_spec->num_pmcs; ++i) {
 		val = classic_ctr_read(i);
-		if (val < 0) {
+		if (pmc_overflow(val)) {
 			if (oprofile_running && ctr[i].enabled) {
 				oprofile_add_ext_sample(pc, regs, i, is_kernel);
 				classic_ctr_write(i, reset_value[i]);
-- 
1.7.4.1

Re: [PATCH] oprofile, powerpc: Handle events that raise an exception without overflowing

[Eric B Munson]

[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]

On Mon, 23 May 2011, Eric B Munson wrote:

> Commit 0837e3242c73566fc1c0196b4ec61779c25ffc93 fixes a situation on POWER7
> where events can roll back if a specualtive event doesn't actually complete.
> This can raise a performance monitor exception.  We need to catch this to ensure
> that we reset the PMC.  In all cases the PMC will be less than 256 cycles from
> overflow.
> 
> This patch lifts Anton's fix for the problem in perf and applies it to oprofile
> as well.
> 
> Signed-off-by: Eric B Munson <emunson@mgebm.net>
> Cc: <stable@kernel.org> # as far back as it applies cleanly

I'd like to get this patch into mainline this merge window if at all possible.

> ---
>  arch/powerpc/oprofile/op_model_power4.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/oprofile/op_model_power4.c b/arch/powerpc/oprofile/op_model_power4.c
> index 8ee51a2..e6bec74 100644
> --- a/arch/powerpc/oprofile/op_model_power4.c
> +++ b/arch/powerpc/oprofile/op_model_power4.c
> @@ -261,6 +261,28 @@ static int get_kernel(unsigned long pc, unsigned long mmcra)
>  	return is_kernel;
>  }
>  
> +static bool pmc_overflow(unsigned long val)
> +{
> +	if ((int)val < 0)
> +		return true;
> +
> +	/*
> +	 * Events on POWER7 can roll back if a speculative event doesn't
> +	 * eventually complete. Unfortunately in some rare cases they will
> +	 * raise a performance monitor exception. We need to catch this to
> +	 * ensure we reset the PMC. In all cases the PMC will be 256 or less
> +	 * cycles from overflow.
> +	 *
> +	 * We only do this if the first pass fails to find any overflowing
> +	 * PMCs because a user might set a period of less than 256 and we
> +	 * don't want to mistakenly reset them.
> +	 */
> +	if (__is_processor(PV_POWER7) && ((0x80000000 - val) <= 256))
> +		return true;
> +
> +	return false;
> +}
> +
>  static void power4_handle_interrupt(struct pt_regs *regs,
>  				    struct op_counter_config *ctr)
>  {
> @@ -281,7 +303,7 @@ static void power4_handle_interrupt(struct pt_regs *regs,
>  
>  	for (i = 0; i < cur_cpu_spec->num_pmcs; ++i) {
>  		val = classic_ctr_read(i);
> -		if (val < 0) {
> +		if (pmc_overflow(val)) {
>  			if (oprofile_running && ctr[i].enabled) {
>  				oprofile_add_ext_sample(pc, regs, i, is_kernel);
>  				classic_ctr_write(i, reset_value[i]);
> -- 
> 1.7.4.1
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH] oprofile, powerpc: Handle events that raise an exception without overflowing

[Maynard Johnson]

Eric B Munson wrote:
> On Mon, 23 May 2011, Eric B Munson wrote:
> 
>> Commit 0837e3242c73566fc1c0196b4ec61779c25ffc93 fixes a situation on POWER7
>> where events can roll back if a specualtive event doesn't actually complete.
>> This can raise a performance monitor exception.  We need to catch this to ensure
>> that we reset the PMC.  In all cases the PMC will be less than 256 cycles from
>> overflow.
>>
>> This patch lifts Anton's fix for the problem in perf and applies it to oprofile
>> as well.
>>
>> Signed-off-by: Eric B Munson <emunson@mgebm.net>
>> Cc: <stable@kernel.org> # as far back as it applies cleanly
> 
> I'd like to get this patch into mainline this merge window if at all possible.
Ack.  I've been able to create a system hang profiling with speculative events on POWER7.  This patch fixes that problem.

-Maynard

> 
>> ---
>>  arch/powerpc/oprofile/op_model_power4.c |   24 +++++++++++++++++++++++-
>>  1 files changed, 23 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/oprofile/op_model_power4.c b/arch/powerpc/oprofile/op_model_power4.c
>> index 8ee51a2..e6bec74 100644
>> --- a/arch/powerpc/oprofile/op_model_power4.c
>> +++ b/arch/powerpc/oprofile/op_model_power4.c
>> @@ -261,6 +261,28 @@ static int get_kernel(unsigned long pc, unsigned long mmcra)
>>  	return is_kernel;
>>  }
>>  
>> +static bool pmc_overflow(unsigned long val)
>> +{
>> +	if ((int)val < 0)
>> +		return true;
>> +
>> +	/*
>> +	 * Events on POWER7 can roll back if a speculative event doesn't
>> +	 * eventually complete. Unfortunately in some rare cases they will
>> +	 * raise a performance monitor exception. We need to catch this to
>> +	 * ensure we reset the PMC. In all cases the PMC will be 256 or less
>> +	 * cycles from overflow.
>> +	 *
>> +	 * We only do this if the first pass fails to find any overflowing
>> +	 * PMCs because a user might set a period of less than 256 and we
>> +	 * don't want to mistakenly reset them.
>> +	 */
>> +	if (__is_processor(PV_POWER7) && ((0x80000000 - val) <= 256))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>>  static void power4_handle_interrupt(struct pt_regs *regs,
>>  				    struct op_counter_config *ctr)
>>  {
>> @@ -281,7 +303,7 @@ static void power4_handle_interrupt(struct pt_regs *regs,
>>  
>>  	for (i = 0; i < cur_cpu_spec->num_pmcs; ++i) {
>>  		val = classic_ctr_read(i);
>> -		if (val < 0) {
>> +		if (pmc_overflow(val)) {
>>  			if (oprofile_running && ctr[i].enabled) {
>>  				oprofile_add_ext_sample(pc, regs, i, is_kernel);
>>  				classic_ctr_write(i, reset_value[i]);
>> -- 
>> 1.7.4.1
>>
>>
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] oprofile, powerpc: Handle events that raise an exception without overflowing

[Robert Richter]

On 23.05.11 10:22:40, Eric B Munson wrote:
> Commit 0837e3242c73566fc1c0196b4ec61779c25ffc93 fixes a situation on POWER7
> where events can roll back if a specualtive event doesn't actually complete.
> This can raise a performance monitor exception.  We need to catch this to ensure
> that we reset the PMC.  In all cases the PMC will be less than 256 cycles from
> overflow.
> 
> This patch lifts Anton's fix for the problem in perf and applies it to oprofile
> as well.
> 
> Signed-off-by: Eric B Munson <emunson@mgebm.net>
> Cc: <stable@kernel.org> # as far back as it applies cleanly
> ---
>  arch/powerpc/oprofile/op_model_power4.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)

I applied the fix to oprofile/urgent.

Thanks Eric,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center