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

From: Arnaldo Carvalho de Melo
Date: Wed Mar 02 2016 - 11:22:30 EST


Em Wed, Mar 02, 2016 at 09:16:48PM +0530, Ravi Bangoria escreveu:
> Thanks Arnaldo,
>
> Please find my comments.
>
> On Wednesday 02 March 2016 07:55 PM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, Feb 24, 2016 at 02:37:45PM +0530, Ravi Bangoria escreveu:
> >> 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)
> >{
> >}
>
> So you mean like this - Just implement only weak function and move code into
> it?
> ie. No strong implementation at this point of time.
>
> Like,
>
> __weak void arch__override_field_order(struct perf_evlist *evlist, const
> char **f_order)
> {
> if (!field_order &&
> is_perf_data_reorded_on_ppc(session->evlist) &&

Oh, I see, ugh, when running on x86_64 we wouldn't use this, so we need
to have per arch default field orders, now I have to recall why is it
that we need this per-arch field order :-\

- Arnaldo

> perf_guest_only())
> *field_order = "overhead,comm,dso,sym";
> }
>
> Then I can do that.
>
> But if you are proposing to implement a strong function and move this code
> into in, then we won't be able to enable cross arch reporting.
>
> >
> >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?
>
> Yes, I'll do this in next version.
>
> Regards,
> Ravi