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

[Arnaldo Carvalho de Melo]

Em Tue, Sep 08, 2015 at 12:58:31PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Sep 08, 2015 at 12:49:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Sep 08, 2015 at 05:34:56PM +0200, Jiri Olsa escreveu:
> > > On Tue, Sep 08, 2015 at 12:18:13PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Humm, I think that we can have a pointer to the current perf_env, be it
> > > > from the current machine, or from the machine environment in the
> > > > perf.data file in struct machine, that way we don't need to change that
> > > > function prototype, I'm prototyping this now, will post a patch.
> > > 
> > > I was thinking of that.. but the perf_env is actualyl related to the
> > > perf.data not to the current machine.. I think it should be part of
> > > the session or perf_header
> > 
> > But what if I want to trace only events that take place in some specific
> > socket, i.e. what to do when perf_session is not used at all and we are
> > not dealing with any header, since there are no files involved?
> 
> So, this is the continuation of this patch:
> 
> commit ce80d3bef9ff97638ca57a5659ef6ad356f35047
> Author: Kan Liang <kan.liang@xxxxxxxxx>
> Date:   Fri Aug 28 05:48:04 2015 -0400
> 
>     perf tools: Rename perf_session_env to perf_env
>     
>     As it is not necessarily tied to a perf.data file and needs using in
>     places where a perf_session is not required.
>     
>     Suggested-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> 
> -----------------------------
> 
> perf_env not necessarily is related to a perf.data file, we need even to
> move it away from header.h.
> 
> I am looking now at where to populate perf_env and set it to
> machine->env when no perf.data files are being accessed.
> 
> I  should have seen the use cpu_map__get_socket_id() in
> perf_event__preprocess_sample(), that is unnaceptable, as it will parse
> that file for each sample, right ;-\
> 
> 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.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/