[PATCH v4 1/3] powerpc/perf: Use stack siar instead of mfspr

[PATCH v4 1/3] powerpc/perf: Use stack siar instead of mfspr

[Kajol Jain]

Minor optimization in the 'perf_instruction_pointer' function code by
making use of stack siar instead of mfspr.

Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
into perf_read_regs")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
 arch/powerpc/perf/core-book3s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index bb0ee716de91..1b464aad29c4 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
 		else
 			return regs->nip;
 	} else if (use_siar && siar_valid(regs))
-		return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
+		return siar + perf_ip_adjust(regs);
 	else if (use_siar)
 		return 0;		// no valid instruction pointer
 	else
-- 
2.26.2

[PATCH v4 2/3] powerpc/perf: Drop the case of returning 0 as instruction pointer

[Kajol Jain]

Drop the case of returning 0 as instruction pointer since kernel
never executes at 0 and userspace almost never does either.

Fixes: e6878835ac47 ("powerpc/perf: Sample only if SIAR-Valid
bit is set in P7+")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
 arch/powerpc/perf/core-book3s.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 1b464aad29c4..23ec89a59893 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2261,8 +2261,6 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
 			return regs->nip;
 	} else if (use_siar && siar_valid(regs))
 		return siar + perf_ip_adjust(regs);
-	else if (use_siar)
-		return 0;		// no valid instruction pointer
 	else
 		return regs->nip;
 }
-- 
2.26.2

[PATCH v4 3/3] powerpc/perf: Fix the check for SIAR value

[Kajol Jain]

Incase of random sampling, there can be scenarios where
Sample Instruction Address Register(SIAR) may not latch
to the sampled instruction and could result in
the value of 0. In these scenarios it is preferred to
return regs->nip. These corner cases are seen in the
previous generation (p9) also.

Patch adds the check for SIAR value along with regs_use_siar
and siar_valid checks so that the function will return
regs->nip incase SIAR is zero.

Patch drops the code under PPMU_P10_DD1 flag check
which handles SIAR 0 case only for Power10 DD1.

Fixes: 2ca13a4cc56c9 ("powerpc/perf: Use regs->nip when SIAR is zero")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---

Changelog:
v3 -> v4
- Remove use_siar variable and directly using regs_use_siar call as
  suggested by Christophe Leroy

v2 -> v3
- Drop adding new ternary condition to check siar value.
- Remove siar check specific for PPMU_P10_DD1 and add
  it along with common checks as suggested by Christophe Leroy
  and Michael Ellermen

 arch/powerpc/perf/core-book3s.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 23ec89a59893..b0a589409039 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2251,15 +2251,9 @@ unsigned long perf_misc_flags(struct pt_regs *regs)
  */
 unsigned long perf_instruction_pointer(struct pt_regs *regs)
 {
-	bool use_siar = regs_use_siar(regs);
 	unsigned long siar = mfspr(SPRN_SIAR);
 
-	if (ppmu && (ppmu->flags & PPMU_P10_DD1)) {
-		if (siar)
-			return siar;
-		else
-			return regs->nip;
-	} else if (use_siar && siar_valid(regs))
+	if (regs_use_siar(regs) && siar_valid(regs) && siar)
 		return siar + perf_ip_adjust(regs);
 	else
 		return regs->nip;
-- 
2.26.2

Re: [PATCH v4 1/3] powerpc/perf: Use stack siar instead of mfspr

[Nageswara Sastry]

On 18/08/21 10:45 pm, Kajol Jain wrote:
> Minor optimization in the 'perf_instruction_pointer' function code by
> making use of stack siar instead of mfspr.
> 
> Fixes: 75382aa72f06 ("powerpc/perf: Move code to select SIAR or pt_regs
> into perf_read_regs")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>


Tested this patch series, not seeing any '0' values.
Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>

example output:
# perf report -D | grep addr
0 26236879714 0x3dcc8 [0x38]: PERF_RECORD_SAMPLE(IP, 0x1): 1446/1446: 
0xc000000000113584 period: 1 addr: 0
0 26236882500 0x3dd00 [0x38]: PERF_RECORD_SAMPLE(IP, 0x1): 1446/1446: 
0xc000000000113584 period: 1 addr: 0
0 26236883436 0x3dd38 [0x38]: PERF_RECORD_SAMPLE(IP, 0x1): 1446/1446: 
0xc000000000113584 period: 10 addr: 0
...


> ---
>   arch/powerpc/perf/core-book3s.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index bb0ee716de91..1b464aad29c4 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2260,7 +2260,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
>   		else
>   			return regs->nip;
>   	} else if (use_siar && siar_valid(regs))
> -		return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
> +		return siar + perf_ip_adjust(regs);
>   	else if (use_siar)
>   		return 0;		// no valid instruction pointer
>   	else
> 

-- 
Thanks and Regards
R.Nageswara Sastry

Re: [PATCH v4 1/3] powerpc/perf: Use stack siar instead of mfspr

[Michael Ellerman]

On Wed, 18 Aug 2021 22:45:54 +0530, Kajol Jain wrote:
> Minor optimization in the 'perf_instruction_pointer' function code by
> making use of stack siar instead of mfspr.
> 
> 
> 
> 

Applied to powerpc/next.

[1/3] powerpc/perf: Use stack siar instead of mfspr
      https://git.kernel.org/powerpc/c/b1643084d164cea0c107a39bcdf0119fc52619af
[2/3] powerpc/perf: Drop the case of returning 0 as instruction pointer
      https://git.kernel.org/powerpc/c/cc90c6742ef5b438f4cb86029d7a794bd0a44a06
[3/3] powerpc/perf: Fix the check for SIAR value
      https://git.kernel.org/powerpc/c/3c69a5f22223fa3e312689ec218b5059784d49d7

cheers