Re: [PATCH 5/6] perf tools: Add dynamic sort key for tracepoint events

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Dec 15, 2015 at 01:22:29PM +0100, Jiri Olsa wrote:
> On Tue, Dec 15, 2015 at 09:07:03PM +0900, Namhyung Kim wrote:
> > On Tue, Dec 15, 2015 at 09:53:09AM +0100, Jiri Olsa wrote:
> > > On Tue, Dec 15, 2015 at 12:46:12AM +0900, Namhyung Kim wrote:
> > > 
> > > SNIP
> > > 
> > > > 
> > > >   $ perf report -s comm,sched:sched_switch.next_pid,sched:sched_switch.next_comm --stdio
> > > >   ...
> > > >   # Overhead  Command            next_pid         next_comm
> > > >   # ........  ...............  ..........  ................
> > > >   #
> > > >       20.86%  swapper               17773   transmission-gt
> > > >        9.64%  transmission-gt           0         swapper/0
> > > >        9.16%  transmission-gt           0         swapper/2
> > > >        5.25%  swapper                 109      kworker/0:1H
> > > >        5.21%  kworker/0:1H              0         swapper/0
> > > >        2.14%  netctl-auto               0         swapper/2
> > > >        1.98%  netctl-auto               0         swapper/0
> > > >        1.98%  swapper                6524            Xephyr
> > > >        1.98%  swapper               27478       netctl-auto
> > > >        1.78%  transmission-gt           0         swapper/3
> > > >        1.53%  Xephyr                    0         swapper/0
> > > >        1.29%  netctl-auto               0         swapper/1
> > > >        1.29%  swapper               27476       netctl-auto
> > > >        1.21%  netctl-auto               0         swapper/3
> > > >        1.17%  swapper                 233    irq/33-iwlwifi
> > > > 
> > > > Note that pid 0 exists for each cpu so have comm of 'swapper/N'.
> > > 
> > > could we also add by default all tracepoint fields in case none
> > > is specified and the event to display is tracepoint?
> > 
> > Seems like a good suggestion.  We can check if there's only one
> > tracepoint event, then use dynamic sort keys for all fields.  But I
> > think we should skip common fields in that case.
> > 
> > > 
> > > also an extra field that would hold/show the 'print fmt' display ? 
> > 
> > Do you want a single extra field per event or per field?
> 
> hm, so the 'print fmt' defines the intended output from the tracepoint,
> like for sched_switch:
> 
>   print fmt: "prev_comm=%s prev_pid=%d prev_prio=%d prev_state=%s%s ==> next_comm=%s next_pid=%d next_prio=%d", REC->prev_comm, REC->prev_pid, REC->prev_prio, REC->prev_state & (2048-1) ? __print_flags(REC->prev_state & (2048-1), "|", { 1, "S"} , { 2, "D" }, { 4, "T" }, { 8, "t" }, { 16, "Z" }, { 32, "X" }, { 64, "x" }, { 128, "K" }, { 256, "W" }, { 512, "P" }, { 1024, "N" }) : "R", REC->prev_state & 2048 ? "+" : "", REC->next_comm, REC->next_pid, REC->next_prio
> 
> gets you (perf script can already do that):
> 
>   perf:21226 [120] S ==> swapper/0:0 [120]
> 
> 
> so maybe have a option or ahve a special field like 'fmt'
> that would carry/display this translation
> 
>   perf report -s comm,fmt
> 
> could be combined with other fields if needed..

OK.  I'll try to implement the 'fmt' sort key and use it for
tracepoint events by default.

Thanks,
Namhyung