linux-kernel - Re: [PATCH v4 0/5] tracing/hist: Add percentage histogram suffixes

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: <20220901080235.20db3793534a64475a3db7be@kernel.org>
Date:   Thu, 1 Sep 2022 08:02:35 +0900
From:   Masami Hiramatsu (Google) <mhiramat@...nel.org>
To:     Tom Zanussi <zanussi@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 0/5] tracing/hist: Add percentage histogram suffixes

On Wed, 31 Aug 2022 16:35:25 -0500
Tom Zanussi <zanussi@...nel.org> wrote:

> Hi Masami,
> 
> On Sat, 2022-08-27 at 13:03 +0900, Masami Hiramatsu (Google) wrote:
> > Hi,
> > 
> > Here is the 4th version of .percent and .graph suffixes for histogram
> > trigger to show the value in percentage and in bar-graph
> > respectively.
> > 
> > I've rebased on Tom's hitcount patch[1/5] on the series and added a
> > patch
> > for supressing display of hitcount[5/5] in this version.
> 
> This is a very nice patchset overall - the only question I have
> concerns patch 5 for suppressing the hitcount.  I actually think the
> patch is fine and does what it says nicely (and probably should have
> been done that way to begin with) but it looks like it would cause
> problems for anyone already doing postprocessing and whose scripts
> would be expecting the hitcount to be there.  So changing the default
> behavior would require their scripts to change, and also now that I
> look at it, the example output in Documentation/ as well.

Good catch! Yeah, this type of change will need to update the docs.
I missed that.

> 
> How about adding an option like 'nohitcount' and having that patch do
> what it does but only if that option is set?

Agreed. So something like this?

echo hist:keys=pid:vals=runtime.graph:nohitcount (or NOHC for short)

Maybe we can also add an option under <tracefs/>options/

> 
> Anyway, for the other ones, patches 2-4,
> 
>   Signed-off-by: Tom Zanussi <zanussi@...nel.org>
>   Tested-by: Tom Zanussi <zanussi@...nel.org

Thank you!

> 
> Thanks!
> 
> Tom
> 
> > 
> > This will help us to check the trend of the histogram instantly
> > without any post processing tool.
> > 
> > Here shows an example of the percentage and the bar graph of
> > the hitcount of the running tasks.
> > 
> >   # cd /sys/kernel/debug/tracing/
> >   # echo hist:keys=pid:vals=hitcount.percent,hitcount.graph:sort=pid
> > > \
> >         events/sched/sched_stat_runtime/trigger
> >   # sleep 10
> >   # cat events/sched/sched_stat_runtime/hist
> >  # event histogram
> >  #
> >  # trigger info:
> > hist:keys=pid:vals=hitcount.percent,hitcount.graph:sort=pid:size=2048
> > [active]
> >  #
> > 
> >  { pid:         14 } hitcount (%):   4.68  hitcount:
> > ###                 
> >  { pid:         16 } hitcount (%):  17.18  hitcount:
> > ###########         
> >  { pid:         57 } hitcount (%):   7.81  hitcount:
> > #####               
> >  { pid:         61 } hitcount (%):  31.25  hitcount:
> > ####################
> >  { pid:         70 } hitcount (%):   4.68  hitcount:
> > ###                 
> >  { pid:         77 } hitcount (%):   1.56  hitcount:
> > #                   
> >  { pid:        145 } hitcount (%):  18.75  hitcount:
> > ############        
> >  { pid:        151 } hitcount (%):   9.37  hitcount:
> > ######              
> >  { pid:        152 } hitcount (%):   4.68  hitcount:
> > ###                 
> > 
> >  Totals:
> >      Hits: 64
> >      Entries: 9
> >      Dropped: 0
> > 
> > Of course if you explicitly specify the hitcount, it can show the
> > hitcount as below;
> > 
> >   # cd /sys/kernel/debug/tracing/
> >   # echo hist:keys=pid:vals=hitcount,runtime:sort=pid > \
> >         events/sched/sched_stat_runtime/trigger
> >   # sleep 10
> >   # cat events/sched/sched_stat_runtime/hist
> >  # event histogram
> >  #
> >  # trigger info:
> > hist:keys=pid:vals=hitcount,runtime:sort=pid:size=2048 [active]
> >  #
> > 
> >  { pid:         14 } hitcount:          2  runtime:     304876
> >  { pid:         16 } hitcount:          8  runtime:     300574
> >  { pid:         26 } hitcount:          2  runtime:      15578
> >  { pid:         32 } hitcount:          2  runtime:     219186
> >  { pid:         57 } hitcount:          3  runtime:     506003
> >  { pid:         61 } hitcount:         20  runtime:    1681473
> >  { pid:         69 } hitcount:          3  runtime:     201785
> >  { pid:         70 } hitcount:          4  runtime:     360608
> >  { pid:         77 } hitcount:          8  runtime:    4146935
> >  { pid:        145 } hitcount:         13  runtime:    7537994
> >  { pid:        155 } hitcount:          4  runtime:    2511937
> >  { pid:        156 } hitcount:          2  runtime:    1398886
> > 
> >  Totals:
> >      Hits: 71
> >      Entries: 12
> >      Dropped: 0
> > 
> > 
> > Thank you,
> > 
> > ---
> > 
> > Masami Hiramatsu (Google) (4):
> >       tracing: Fix to check event_mutex is held while accessing
> > trigger list
> >       tracing: Add .percent suffix option to histogram values
> >       tracing: Add .graph suffix option to histogram value
> >       tracing: Show hitcount value only when specified
> > 
> > Tom Zanussi (1):
> >       tracing: Allow multiple hitcount values in histograms
> > 
> > 
> >  kernel/trace/trace.c                |    3 -
> >  kernel/trace/trace_events_hist.c    |  176
> > ++++++++++++++++++++++++++++++-----
> >  kernel/trace/trace_events_trigger.c |    3 -
> >  3 files changed, 156 insertions(+), 26 deletions(-)
> > 
> > --
> > Masami Hiramatsu (Google) <mhiramat@...nel.org>
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>