linux-kernel - Re: [PATCH] perf stat: Fix uncore event mixed metric with workload error issue

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] [day] [month] [year] [list]

Date:   Wed, 29 Apr 2020 19:50:48 +0800
From:   "Jin, Yao" <yao.jin@...ux.intel.com>
To:     Jiri Olsa <jolsa@...hat.com>, Andi Kleen <ak@...ux.intel.com>
Cc:     acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
        mingo@...hat.com, alexander.shishkin@...ux.intel.com,
        Linux-kernel@...r.kernel.org, kan.liang@...el.com,
        yao.jin@...el.com
Subject: Re: [PATCH] perf stat: Fix uncore event mixed metric with workload
 error issue

Hi Jiri,

On 4/29/2020 4:16 PM, Jiri Olsa wrote:
> On Tue, Apr 28, 2020 at 02:19:22PM -0700, Andi Kleen wrote:
>>> I wonder this would break some expectations.. would it be
>>> more safe to detect duration event and bypass it from the
>>> decission? but maybe the case I'm worried about is not a
>>> problem at all.. Andi?
>>
>> Don't see what it would break.
>>
>> Yes maybe we need to special case duration_time more, but that would
>> be a much bigger patch.
> 
> would below change work for you? if duration_time is the only
> case, I'd rather go with the special case for it
> 
> jirka
> 
> 

Just tested, for the case of DRAM_BW_Use, it can work.

root@...-csl-2sp5 ~# perf stat -M DRAM_BW_Use -- sleep 1

  Performance counter stats for 'system wide':

               9.54 MiB  uncore_imc/cas_count_read/
               3.26 MiB  uncore_imc/cas_count_write/
         1002109793 ns   duration_time

        1.002109793 seconds time elapsed

Thanks
Jin Yao

> ---
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 9207b6c45475..2518204cffd1 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1853,6 +1853,8 @@ static void setup_system_wide(int forks)
>   		struct evsel *counter;
>   
>   		evlist__for_each_entry(evsel_list, counter) {
> +			if (counter->tool_event == PERF_TOOL_DURATION_TIME)
> +				continue;
>   			if (!counter->core.system_wide)
>   				return;
>   		}
>