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

[Tom Zanussi]

On Thu, 2022-09-01 at 08:02 +0900, Masami Hiramatsu wrote:
> On Wed, 31 Aug 2022 16:35:25 -0500
> Tom Zanussi <zanussi@xxxxxxxxxx> 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/

Yeah, makes sense to me.

Thanks,

Tom

> 
> > 
> > Anyway, for the other ones, patches 2-4,
> > 
> >   Signed-off-by: Tom Zanussi <zanussi@xxxxxxxxxx>
> >   Tested-by: Tom Zanussi <zanussi@xxxxxxxxxx
> 
> 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@xxxxxxxxxx>
> > 
> 
>