[PATCH v2 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0

[Michael Ellerman]

Christophe Leroy <christophe.leroy at csgroup.eu> writes:
> Le 13/08/2021 à 10:24, Kajol Jain a écrit :
>> Incase of random sampling, there can be scenarios where SIAR is not
>> latching sample address and results in 0 value. Since current code
>> directly returning the siar value, we could see multiple instruction
>> pointer values as 0 in perf report.

Can you please give more detail on that? What scenarios? On what CPUs?

>> Patch resolves this issue by adding a ternary condition to return
>> regs->nip incase SIAR is 0.
>
> Your description seems rather similar to 
> https://github.com/linuxppc/linux/commit/2ca13a4cc56c920a6c9fc8ee45d02bccacd7f46c
>
> Does it mean that the problem occurs on more than the power10 DD1 ?
>
> In that case, can the solution be common instead of doing something for power10 DD1 and something 
> for others ?

Agreed.

This change would seem to make that P10 DD1 logic superfluous.

Also we already have a fallback to regs->nip in the else case of the if,
so we should just use that rather than adding a ternary condition.

eg.

	if (use_siar && siar_valid(regs) && siar)
		return siar + perf_ip_adjust(regs);
	else if (use_siar)
		return 0;		// no valid instruction pointer
	else
		return regs->nip;


I'm also not sure why we have that return 0 case, I can't think of why
we'd ever want to do that rather than using nip. So maybe we should do
another patch to drop that case.

cheers