Re: [PATCH v2 2/3] perf kvm: enable record|report feature on powerpc

From: Ravi Bangoria
Date: Tue Feb 02 2016 - 04:07:06 EST


HI acme,

On Tuesday 02 February 2016 02:36 AM, Arnaldo Carvalho de Melo wrote:
Em Fri, Jan 22, 2016 at 11:28:11AM +0530, Ravi Bangoria escreveu:
+ return event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
+}
This hunk and the next should be on the previous patch, that is not even
compiling...

You have to compile patch by patch, we can't just test at the end of a
patchkit like this, this destroys bisection ;-\

Didn't aware about that. Will take care of compiling each patch
separately next time onwards.

Also you first need to put in place a way to override how to obtain the
cpumode, then you should use it.

Also this mode doesn't look feasible at all, think about processing
perf.data files generated in !powerpc systems being analysed in a
powerpc system. This has to be dependend on the architecture of the
machine where the perf.data file was recorded, not on the archictecture
of the machine the binary was built for.

Valid point.

I'll re-think about approach in this case.

It is only when you do live analysis, like with 'perf trace' and 'perf
top' that its guaranteed to be all on the same machine.

IIRC in one of the patches in this series you introduce and use a
library function on the same patch, please break it into two patches as
well, lemme see what is the name...

Yeah, it is also in this patch:

perf_evlist__arch_add_default(struct perf_evlist *evlist)

Please add this in a separate patch, stating in the changeset comment
why it is needed and how architectures can override it.

Will do that. Thanks for reviewing.

Regards,
Ravi