| Message ID | 20210818131949.32008-3-kjain@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v3,1/3] powerpc/perf: Use stack siar instead of mfspr | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 25 jobs. |
Le 18/08/2021 à 15:19, Kajol Jain a écrit : > 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 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: > - 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 | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c > index 23ec89a59893..55efbba7572b 100644 > --- a/arch/powerpc/perf/core-book3s.c > +++ b/arch/powerpc/perf/core-book3s.c > @@ -2254,12 +2254,7 @@ 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 (use_siar && siar_valid(regs) && siar) You can probably now do + if (regs_use_siar(regs) && siar_valid(regs) && siar) and remove use_siar > return siar + perf_ip_adjust(regs); > else > return regs->nip; >
On 8/18/21 6:58 PM, Christophe Leroy wrote: > > > Le 18/08/2021 à 15:19, Kajol Jain a écrit : >> 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 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: >> - 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 | 7 +------ >> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c >> index 23ec89a59893..55efbba7572b 100644 >> --- a/arch/powerpc/perf/core-book3s.c >> +++ b/arch/powerpc/perf/core-book3s.c >> @@ -2254,12 +2254,7 @@ 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 (use_siar && siar_valid(regs) && siar) > > You can probably now do > > + if (regs_use_siar(regs) && siar_valid(regs) && siar) > > and remove use_siar Hi Christophe, I will update it. Thanks for pointing it. Thanks, Kajol Jain > >> return siar + perf_ip_adjust(regs); >> else >> return regs->nip; >>
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c index 23ec89a59893..55efbba7572b 100644 --- a/arch/powerpc/perf/core-book3s.c +++ b/arch/powerpc/perf/core-book3s.c @@ -2254,12 +2254,7 @@ 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 (use_siar && siar_valid(regs) && siar) return siar + perf_ip_adjust(regs); else return regs->nip;
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 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: - 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 | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)