Re: [PATCH] perf report: Fix invalid memory accessing

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

Em Wed, Sep 09, 2015 at 01:06:40PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Sep 08, 2015 at 01:35:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Sep 08, 2015 at 01:13:59PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Tue, Sep 08, 2015 at 12:58:31PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Right now we don't have that much use for the other fields in
> > > > 'perf_env', just for the CPU topology information, that we will set in
> > > > addr_location for each sample, but we can have uses for that later,
> > > > think about a TUI interface for 'perf trace' where we will show what was
> > > > the command line, etc.
> 
> > > Argh, so in the patch introducing this al.socket thing it would first
> > > parse the value from the current system, reading sysfs, etc, then, in
> > > the 'report' case it would just throw this information away:
> 
> > > -       /* read socket id from perf.data for perf report */
> > > -       al.socket = env->cpu[al.cpu].socket_id;
> 
> > > We really should do this in perf_event__preprocess_sample() and read the
> > > topology information just once, probably using the same routine that
> > > creates the perf.data file env record.
>  
> > Lunch break, but I'll continue the work I started at
> > https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/core
> 
> Ok, I have them at my perf/env branch:
> 
> https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/env
> 
>     12  eefb0dbb1aa9 perf evsel: Remove forward declaration of 'struct perf_evlist'
>     11  8d9c09de8465 Revert "perf evsel: Add a backpointer to the evlist a evsel is in"
>     10  e3611f9dbced perf report: Do not blindly use env->cpu[al.cpu].socket_id
>      9  6f9ad1cd48e8 perf event: Use machine->env to find the cpu -> socket mapping
>      8  7b157d3eb7b4 perf machine: Add pointer to sample's environment
>      7  cc393318b0c0 perf hists browser: Fixup the "cpu" column width calculation
>      6  bc413472a275 perf top: Cache the cpu topology info when "-s socket" is used
>      5  908f1939ad52 perf sort: Set flag stating if the "socket" key is being used
>      4  81956a8be745 perf env: Introduce read_cpu_topology_map() method
>      3  7d76ec2a6f18 perf env: Adopt perf_header__set_cmdline
>      2  371495478108 perf env: Rename some leftovers from rename to perf_env
>      1  7b1a26d8cff7 perf env: Move perf_env out of header.h and session.c into separate object
> 
> Now I see that one more probably is needed, the one adding the ->env
> backpointer to struct evlist.

Ok, one more: 'perf top' needs --socket-filter, will do after lunch.

- Arnaldo
 
> The ones fixing the problem are #9 and #10, the rest is infrastructure needed
> for those fixes to work, and some are not strictly needed but as I was working
> on it, couldn't resist cleaning up.
> 
> I'll test this some more, add one or more cleanups and post for review, if all
> goes well, tomorrow I'll push it to Ingo.
> 
> - Arnaldo