Re: [RFC 4/4] perf kvm: Fix output fields instead of 'trace' for perf kvm report on powerpc

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Feb 24, 2016 at 02:37:45PM +0530, Ravi Bangoria escreveu:
> commit d49dadea7862 ("perf tools: Make 'trace' or 'trace_fields' sort key
> default for tracepoint events") makes 'trace' sort key as a default
> while displaying report for tracepoint.
> 
> Because tracepoint(kvm_hv:kvm_guest_exit) is used as a default event,
> perf kvm report will display output as a list of tracepoint hits and
> not with a normal report columns.
> 
> This patch will replace 'trace' field with 'overhead,comm,dso,sym' while
> displaying perf kvm report of powerpc.
> 
> Before applying patch:
> 
>   $ ./perf kvm --guestkallsyms=guest.kallsyms --guestmodules=guest.modules report --stdio
>   # To display the perf.data header info, please use --header/--header-only options.
>   #
>   #
>   # Total Lost Samples: 0
>   #
>   # Samples: 181K of event 'kvm_hv:kvm_guest_exit'
>   # Event count (approx.): 181061
>   #
>   # Overhead  Trace output
>   # ........  .................................................................................
>   #
>        0.02%  VCPU 8: trap=HV_DECREMENTER pc=0xc000000000091924 msr=0x8000000000009032, ceded=0
>        0.00%  VCPU 0: trap=HV_DECREMENTER pc=0xc000000000091924 msr=0x8000000000009032, ceded=0
>        0.00%  VCPU 8: trap=HV_DECREMENTER pc=0x10005c7c msr=0x800000000280f032, ceded=0
>        0.00%  VCPU 8: trap=HV_DECREMENTER pc=0x1001ef14 msr=0x800000000280f032, ceded=0
>        0.00%  VCPU 8: trap=HV_DECREMENTER pc=0x3fff83398830 msr=0x800000000280f032, ceded=0
>        0.00%  VCPU 8: trap=HV_DECREMENTER pc=0x3fff833a6fe4 msr=0x800000000280f032, ceded=0
>        0.00%  VCPU 8: trap=HV_DECREMENTER pc=0x3fff833a7a64 msr=0x800000000280f032, ceded=0
> 
> After applying patch:
> 
>   $ ./perf kvm --guestkallsyms=guest.kallsyms --guestmodules=guest.modules report --stdio
>   # To display the perf.data header info, please use --header/--header-only options.
>   #
>   #
>   # Total Lost Samples: 0
>   #
>   # Samples: 181K of event 'kvm_hv:kvm_guest_exit'
>   # Event count (approx.): 181061
>   #
>   # Overhead  Command  Shared Object            Symbol
>   # ........  .......  .......................  ..............................
>   #
>        0.02%  :57276   [guest.kernel.kallsyms]  [g] .plpar_hcall_norets
>        0.00%  :57274   [guest.kernel.kallsyms]  [g] .plpar_hcall_norets
>        0.00%  :57276   [guest.kernel.kallsyms]  [g] .__copy_tofrom_user_power7
>        0.00%  :57276   [guest.kernel.kallsyms]  [g] ._atomic_dec_and_lock
>        0.00%  :57276   [guest.kernel.kallsyms]  [g] ._raw_spin_lock
>        0.00%  :57276   [guest.kernel.kallsyms]  [g] ._switch
>        0.00%  :57276   [guest.kernel.kallsyms]  [g] .bio_add_page
>        0.00%  :57276   [guest.kernel.kallsyms]  [g] .kmem_cache_alloc
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/builtin-report.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 31ec4ba..5d96882 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -930,6 +930,11 @@ repeat:
>  	else
>  		use_browser = 0;
>  
> +	if (!field_order &&
> +	    is_perf_data_reorded_on_ppc(session->evlist) &&
> +	    perf_guest_only())
> +		field_order = "overhead,comm,dso,sym";
> +

Can you please do it as:

__weak void arch__override_field_order(struct perf_evlist *evlist, const char **field_order)
{
}

This way we don't see any arch specific stuff in the tool, also I
haven't seen any doc update, are you sure nothing needs to be added to
tools/perf/Documentaton/ for any of these patches?

I think this needs to be documented further, probably in
tools/perf/design.txt too?

>  	if (setup_sorting(session->evlist) < 0) {
>  		if (sort_order)
>  			parse_options_usage(report_usage, options, "s", 1);
> -- 
> 2.1.4