Re: [PATCH 02/13] perf hist: Save raw_data/size for tracepoint events

[Arnaldo Carvalho de Melo]

Em Thu, Dec 24, 2015 at 09:45:45AM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> On Wed, Dec 23, 2015 at 06:43:35PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Dec 23, 2015 at 02:06:59AM +0900, Namhyung Kim escreveu:
> > > The raw_data and raw_size fields are to provide tracepoint specific
> > > information.  They will be used by dynamic sort keys later.
> > > 
> > > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > > ---
> > >  tools/perf/util/hist.c | 4 ++++
> > >  tools/perf/util/sort.h | 2 ++
> > >  2 files changed, 6 insertions(+)
> > > 
> > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > > index 039bb91d0a92..c0c92a3daa69 100644
> > > --- a/tools/perf/util/hist.c
> > > +++ b/tools/perf/util/hist.c
> > > @@ -487,6 +487,8 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
> > >  		.branch_info = bi,
> > >  		.mem_info = mi,
> > >  		.transaction = sample->transaction,
> > > +		.raw_data = sample->raw_data,
> > > +		.raw_size = sample->raw_size,
> > 
> > So, sample->raw_data is just a pointer to perf_event_sample->array, that
> > may be overwritten, no?
> 
> I couldn't find where the array data is overwritten.  The
> __perf_session__process_events() mmaps with PROT_READ basically.  But
> the mmap can be munmapped on 32 bit systems.  I'll keep a copy then.

perf top, aka mmap with overwrite mode? 

I think we should always think first at how to make such new features to
work on 'perf top', where there is no such thing as a "file", but a
_ring_ buffer, where we reuse that buffer when we fill it up, so we
should not keep any pointers to past events, just use what in the
currently being processed.

Haven't looked, do we need to access it after we add the hist, or just
after the perf_sample is parsed?

> > Looking at the other patches.
> 
> Thanks,
> Namhyung
> 
> > 
> > - Arnaldo
> > 
> > >  	};
> > >  
> > >  	return hists__findnew_entry(hists, &entry, al, sample_self);
> > > @@ -801,6 +803,8 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter,
> > >  			.sym = al->sym,
> > >  		},
> > >  		.parent = iter->parent,
> > > +		.raw_data = sample->raw_data,
> > > +		.raw_size = sample->raw_size,
> > >  	};
> > >  	int i;
> > >  	struct callchain_cursor cursor;
> > > diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> > > index 86f05e7a5566..d29898708dbd 100644
> > > --- a/tools/perf/util/sort.h
> > > +++ b/tools/perf/util/sort.h
> > > @@ -122,6 +122,8 @@ struct hist_entry {
> > >  	struct branch_info	*branch_info;
> > >  	struct hists		*hists;
> > >  	struct mem_info		*mem_info;
> > > +	void			*raw_data;
> > > +	u32			raw_size;
> > >  	struct callchain_root	callchain[0]; /* must be last member */
> > >  };
> > >  
> > > -- 
> > > 2.6.4
--
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/