Re: [RFC] [PATCH] Trace TLBIE's

[Balbir Singh <bsingharora@gmail.com>]

On 23/11/16 21:15, Michael Ellerman wrote:
> Balbir Singh <bsingharora@gmail.com> writes:
> 
>> Just a quick patch to trace tlbie(l)'s. The idea being that it can be
>> enabled when we suspect corruption or when we need to see if we are doing
>> the right thing during flush. I think the format can be enhanced to
>> make it nicer (expand the RB/RS/IS/L cases in more detail). For now I am
>> sharing the idea to get inputs
>>
>> A typical trace might look like this
>>
>>
>> <...>-5141  [062]  1354.486693: tlbie:                
>> 	tlbie with lpid 0, local 0, rb=7b5d0ff874f11f1, rs=0, ric=0 prs=0 r=0
>> systemd-udevd-2584  [018]  1354.486772: tlbie:
>> 	tlbie with lpid 0, local 0, rb=17be1f421adc10c1, rs=0, ric=0 prs=0 r=0
>> ...
>>
>> qemu-system-ppc-5371  [016]  1412.369519: tlbie:
>> 	tlbie with lpid 0, local 1, rb=67bd8900174c11c1, rs=0, ric=0 prs=0 r=0
>> qemu-system-ppc-5377  [056]  1421.687262: tlbie:
>> 	tlbie with lpid 1, local 0, rb=5f04edffa00c11c1, rs=1, ric=0 prs=0 r=0
> 
> My first reaction is "why the hell do we have so many open-coded
> tlbies". So step one might be to add a static inline helper, that way we
> don't have to add the trace_tlbie() in so many places.
> 

The problem is the variants

Hash uses the two operand variant and radix on p9 uses the 5 operand variant.
To make matters even more complex depending on the CPU_FTR_ARCH we also have
operand variants with L and a version where the immediate value was removed.
I've tried to encapsulate the traces with their variants, but that implied
having the traces at the various call sites

We also use tlbie* from assembly (like the one called by kvmppc_hv_entry),
which is not covered by these traces at the moment, but I guess we can
add them as well.

> Also in some of them you call trace_tlbie() before the
> eieio/tlbsync/ptesync. Which may not be wrong, but looks worrying at
> first glance.
> 

I'll try and make it consistent, but there are places like do_tlbies()
where we loop do the tlbie(l)'s and then do sync's. In those case
I've had to put the traces within the loop instead of queuing or looping
twice. We don't access any of the addresses flushed, just print the registers.

> But overall I guess it's OK. We'd want to do a quick benchmark to make
> sure it's not adding any overhead.

OK.. I'll try and find a benchmark and run it with traces disabled.

Thanks for the review
Balbir Singh