From: "Jin, Yao" <yao.jin@linux.intel.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
mingo@redhat.com, alexander.shishkin@linux.intel.com,
Linux-kernel@vger.kernel.org, ak@linux.intel.com,
kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v4 3/4] perf stat: Copy counts from prev_raw_counts to evsel->counts
Date: Thu, 14 May 2020 13:42:39 +0800 [thread overview]
Message-ID: <7df7c652-fb34-66c0-e2f2-6a5585afd621@linux.intel.com> (raw)
In-Reply-To: <20200513153133.GN3158213@krava>
Hi Jiri,
On 5/13/2020 11:31 PM, Jiri Olsa wrote:
> On Fri, May 08, 2020 at 03:58:16PM +0800, Jin Yao wrote:
>> It would be useful to support the overall statistics for perf-stat
>> interval mode. For example, report the summary at the end of
>> "perf-stat -I" output.
>>
>> But since perf-stat can support many aggregation modes, such as
>> --per-thread, --per-socket, -M and etc, we need a solution which
>> doesn't bring much complexity.
>>
>> The idea is to use 'evsel->prev_raw_counts' which is updated in
>> each interval and it's saved with the latest counts. Before reporting
>> the summary, we copy the counts from evsel->prev_raw_counts to
>> evsel->counts, and next we just follow non-interval processing.
>>
>> In evsel__compute_deltas, this patch saves counts to the member
>> [cpu0,thread0] of perf_counts for AGGR_GLOBAL.
>>
>> That's because after copying evsel->prev_raw_counts to evsel->counts,
>> perf_counts(evsel->counts, cpu, thread) are all 0 for AGGR_GLOBAL.
>> Once we go to process_counter_maps again, all members of perf_counts
>> are 0.
>>
>> So this patch uses a trick that saves the previous aggr value to
>> the member [cpu0,thread0] of perf_counts, then aggr calculation
>> in process_counter_values can work correctly.
>>
>> v4:
>> ---
>> Change the commit message.
>> No functional change.
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>> tools/perf/util/evsel.c | 1 +
>> tools/perf/util/stat.c | 24 ++++++++++++++++++++++++
>> tools/perf/util/stat.h | 1 +
>> 3 files changed, 26 insertions(+)
>>
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index 28683b0eb738..6fae1ec28886 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -1283,6 +1283,7 @@ void evsel__compute_deltas(struct evsel *evsel, int cpu, int thread,
>> if (cpu == -1) {
>> tmp = evsel->prev_raw_counts->aggr;
>> evsel->prev_raw_counts->aggr = *count;
>> + *perf_counts(evsel->prev_raw_counts, 0, 0) = *count;
>
> ok, I think I understand that now.. it's only for AGGR_GLOBAL mode,
> because the perf_stat_process_counter will create aggr values from
> per cpu values
>
> but why do we need to do that all the time? can't we just set it up
> before you zero prev_raw_counts in next patch?
>
>
> if (interval) {
> stat_config.interval = 0;
> stat_config.summary = true;
> perf_evlist__copy_prev_raw_counts(evsel_list);
>
> -> for AGGR_GLOBAL set the counts[0,0] to prev_raw_counts->aggr
>
> perf_evlist__reset_prev_raw_counts(evsel_list);
> runtime_stat_reset(&stat_config);
> perf_stat__reset_shadow_per_stat(&rt_stat);
> }
>
Yes, I think that's a good idea.
Now in v5, I create a new patch "perf stat: Save aggr value to first member of
prev_raw_counts" to save aggr value to first member of prev_raw_counts for
AGGR_GLOBAL. Then next, perf_stat_process_counter can create aggr values from
per cpu values successfully.
Thanks
Jin Yao
>
> thanks,
> jirka
>
next prev parent reply other threads:[~2020-05-14 5:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-08 7:58 [PATCH v4 0/4] perf stat: Support overall statistics for interval mode Jin Yao
2020-05-08 7:58 ` [PATCH v4 1/4] perf stat: Fix wrong per-thread runtime stat " Jin Yao
2020-05-08 7:58 ` [PATCH v4 2/4] perf counts: Reset prev_raw_counts counts Jin Yao
2020-05-08 7:58 ` [PATCH v4 3/4] perf stat: Copy counts from prev_raw_counts to evsel->counts Jin Yao
2020-05-13 15:31 ` Jiri Olsa
2020-05-14 5:42 ` Jin, Yao [this message]
2020-05-08 7:58 ` [PATCH v4 4/4] perf stat: Report summary for interval mode Jin Yao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7df7c652-fb34-66c0-e2f2-6a5585afd621@linux.intel.com \
--to=yao.jin@linux.intel.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=kan.liang@intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yao.jin@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.