linux-kernel - Re: [PATCH v1] perf top: Make zeroing histogram on refresh the default

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: <35b43fd3-54ed-49ba-b8c5-6b7d03c9bb8e@amd.com>
Date: Fri, 7 Jun 2024 09:26:14 +0530
From: Ravi Bangoria <ravi.bangoria@....com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Ingo Molnar <mingo@...hat.com>,
 Namhyung Kim <namhyung@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
 Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
 Kan Liang <kan.liang@...ux.intel.com>, Changbin Du <changbin.du@...wei.com>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, linux-perf-users@...r.kernel.org,
 linux-kernel@...r.kernel.org, Stephane Eranian <eranian@...gle.com>,
 ravi.bangoria@....com
Subject: Re: [PATCH v1] perf top: Make zeroing histogram on refresh the
 default

On 6/6/2024 7:41 PM, Arnaldo Carvalho de Melo wrote:
> On Thu, Jun 06, 2024 at 10:15:00AM +0530, Ravi Bangoria wrote:
>> On 5/17/2024 3:51 AM, Ian Rogers wrote:
>>> Instead of decaying histograms over time change it so that they are
>>> zero-ed on each perf top refresh. Previously the option '-z', or
>>> pressing 'z' in tui mode, would enable this behavior. Decaying samples
>>> is non-intuitive as it isn't how "top" works. Make zeroing on refresh
>>> the default and rename the command line options from 'z' to 'Z' and
>>> 'zero' to 'decay'.
> 
>> I've also felt `perf top` decay as non-intuitive. Esp. when system becomes
>> idle after some heavy workload, even decayed samples are far more compared
>> to samples from currently running processes and thus `perf top` keeps
>> showing already finished processes at the top, which is kind of confusing.
>> fwiw:
>  
>> Acked-by: Ravi Bangoria <ravi.bangoria@....com>
> 
> Thanks for voicing your opinion, that is really helpful.
> 
> Changing tool behaviour can have unintended consequences even when done
> with the best intentions and analysis, that is why I'm wary of doing it.
> 
> The --children case generated complaints when we made it the default, so
> we ended up with a ~/.perfconfig option to disable it:
> 
> root@...ber:~# perf config top.children=false
> root@...ber:~# perf top -g
> 
> Or enable explicitely:
> 
> root@...ber:~# perf config top.children=true
> root@...ber:~# perf top -g
> 
> Same thing with the build id cache, where one can disable it using 'perf
> config', etc.
> 
> So I'd do this initially with a 'perf config top.refresh=zero' instead
> of changing something so few people complained as not being intuitive
> after all those years of having that default.

Makes sense. Thanks for the clarification.

Ravi