Re: [PATCH] perf: implement recording/reporting per-cpu samples

[Arnaldo Carvalho de Melo <acme@ghostprotocols.net>]

Em Wed, May 05, 2010 at 11:16:12AM -0700, Arun Sharma escreveu:
> On Tue, May 04, 2010 at 11:16:38AM +0200, Peter Zijlstra wrote:
> > > In a shared multi-core environment, users want to analyze why their
> > > program was slow. In particular, if the code ran slower only on
> > > certain CPUs due to interference from other programs or kernel
> > > threads, they want to know that.
> > 
> > But for that you use perf record -a, right? So you record all cpus
> > allways -- otherwise there is no telling what was happening to make it
> > go slow.
> 
> The updated patch records the CPU only in the system_wide mode.

I think this should be done only if you'll actually need it, as in,
"cpu" is one of the sort keys, but that can be done as a followup patch,
but there is another thing I think you need to change, see below.

> > > But that's just our use case. The patch is mostly about --sort cpu
> > > option. If you want to drop the part that enables PERF_SAMPLE_CPU by
> > > default, that's fine by me.
> > 
> > Right, it would be very nice if we can avoid growing the default sample
> > size. Also, your changelog needs work, please explain the full usecase
> > that goes with this feature.
> > 
> > Explain the thing you're wanting to measure, explain the implementation
> > and maybe give a short example.
> 
> Updated changelog as well.
> 
>  -Arun
> 

<SNIP>

> +	if (session->sample_type & PERF_SAMPLE_CPU) {
> +		dump_printf("... cpu: %d\n", data.cpu);
> +		al.cpu = data.cpu;
> +	} else {
> +		al.cpu = -1;
> +	}
> +

This should be in event__parse_sample() that will set a field in
sample_data, just like the others optional fields.

I'll cook up a patch with these changes and post here for your review.

- Arnaldo