linux-kernel - Re: [PATCH 01/26] perf tools: Introduce struct hist_entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874n09ii01.fsf@sejong.aot.lge.com>
Date:	Thu, 29 May 2014 08:46:38 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...nel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Namhyung Kim <namhyung.kim@....com>,
	LKML <linux-kernel@...r.kernel.org>,
	David Ahern <dsahern@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Andi Kleen <andi@...stfloor.org>, Arun Sharma <asharma@...com>,
	Rodrigo Campos <rodrigo@...g.com.ar>,
	Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 01/26] perf tools: Introduce struct hist_entry_iter

Hi Jiri,

On Mon, 26 May 2014 20:44:25 +0200, Jiri Olsa wrote:
> On Fri, May 23, 2014 at 07:03:58PM +0900, Namhyung Kim wrote:
>> There're some duplicate code when adding hist entries.  They are
>> different in that some have branch info or mem info but generally do
>> same thing.  So introduce new struct hist_entry_iter and add callbacks
>> to customize each case in general way.
>> 
>> The new perf_evsel__add_entry() function will look like:
>> 
>>   iter->prepare_entry();
>>   iter->add_single_entry();
>> 
>>   while (iter->next_entry())
>>     iter->add_next_entry();
>> 
>>   iter->finish_entry();
>> 
>> This will help further work like the cumulative callchain patchset.
>> 
>> Tested-by: Arun Sharma <asharma@...com>
>> Tested-by: Rodrigo Campos <rodrigo@...g.com.ar>
>> Acked-by: Jiri Olsa <jolsa@...hat.com>
>> Cc: David Ahern <dsahern@...il.com>
>> Cc: Stephane Eranian <eranian@...gle.com>
>> Cc: Frederic Weisbecker <fweisbec@...il.com>
>> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
>> ---
>>  tools/perf/builtin-report.c     | 194 +++----------------------
>>  tools/perf/tests/hists_filter.c |  18 +--
>>  tools/perf/tests/hists_output.c |  11 +-
>>  tools/perf/util/hist.c          | 303 ++++++++++++++++++++++++++++++++++++++++
>>  tools/perf/util/hist.h          |  33 +++++
>>  5 files changed, 371 insertions(+), 188 deletions(-)
>> 
>> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
>> index bc0eec1ce4be..d224f5961faa 100644
>> --- a/tools/perf/builtin-report.c
>> +++ b/tools/perf/builtin-report.c
>> @@ -76,165 +76,6 @@ static int report__config(const char *var, const char *value, void *cb)
>>  	return perf_default_config(var, value, cb);
>>  }
>>  
>> -static void report__inc_stats(struct report *rep, struct hist_entry *he)
>> -{
>> -	/*
>> -	 * The @he is either of a newly created one or an existing one
>> -	 * merging current sample.  We only want to count a new one so
>> -	 * checking ->nr_events being 1.
>> -	 */
>> -	if (he->stat.nr_events == 1)
>> -		rep->nr_entries++;
>> -
>
> hm.. above is report specific counter update
>
> while below code is repeated for each iterator in finish_entry,
> maybe we should separated this and put below into a function
> called from generic part

Right.  I'll separate the generic part into a function.

The problem of calling report__inc_stats() in process_sample_event() is
that the hist_entry is no longer available.  And each mode has slightly
different behavior when accounting entries so it's inaccurate at this
stage.  Once we have the callback mechanism later in this series, it'll
be solved.

Thanks,
Namhyung

>
>> -	/*
>> -	 * Only counts number of samples at this stage as it's more
>> -	 * natural to do it here and non-sample events are also
>> -	 * counted in perf_session_deliver_event().  The dump_trace
>> -	 * requires this info is ready before going to the output tree.
>> -	 */
>> -	hists__inc_nr_events(he->hists, PERF_RECORD_SAMPLE);
>> -	if (!he->filtered)
>> -		he->hists->stats.nr_non_filtered_samples++;
>> -}
>
> jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/