Re: [PATCH 1/1] Fix number of events displayed in header

[David Ahern]

On 3/20/12 1:42 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 20, 2012 at 02:23:16PM -0500, Ashay Rane escreveu:
> > From: Ashay Rane<ashay.rane@xxxxxxxxxxxxxxx>
>
> Too short a changeset comment, please elaborate.
>
> The change at least would have to be changed from
>
> "Events: %llu", total_period
>
> to
>
> "Total period: %llu", total_period
>
> To be consistent in what we call "Events", i.e. right now it is telling
> the number of PERF_RECORD_SAMPLE events, with your change it woul0d be
> showing the sum of the periods in all samples.

Does not seem appropriate to change from number of sample events to 
period for all event types. This change assumes the event is cycles, 
cpu-clock or task-clock.

David


> - Arnaldo
>
> > Signed-off-by: Ashay Rane<ashay.rane@xxxxxxxxxxxxxxx>
> > ---
> >   tools/perf/builtin-report.c |    6 ++----
> >   1 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 25d34d4..58a20cf 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -202,11 +202,9 @@ static size_t hists__fprintf_nr_sample_events(struct hists *self,
> >   					      const char *evname, FILE *fp)
> >   {
> >   	size_t ret;
> > -	char unit;
> > -	unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE];
> > +	u64 total_period = self->stats.total_period;
> >
> > -	nr_events = convert_unit(nr_events,&unit);
> > -	ret = fprintf(fp, "# Events: %lu%c", nr_events, unit);
> > +	ret = fprintf(fp, "# Events: %llu", total_period);
> >   	if (evname != NULL)
> >   		ret += fprintf(fp, " %s", evname);
> >   	return ret + fprintf(fp, "\n#\n");
> > --
> > 1.7.4.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
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/