Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230531203236.1602054-1-namhyung@kernel.org> <dfef9c42-5b84-6720-656f-5d3ac1d8ccf5@intel.com>
In-Reply-To: <dfef9c42-5b84-6720-656f-5d3ac1d8ccf5@intel.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Thu, 1 Jun 2023 09:14:50 -0700
Message-ID: <CAM9d7cjMLMExM_irC+ZXkDi65w95W6OpOihgrbB5X=jJgwyNeQ@mail.gmail.com>
Subject: Re: [PATCH] perf script: Increase PID/TID width for output
To:     Adrian Hunter <adrian.hunter@intel.com>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-perf-users@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Adrian,

On Wed, May 31, 2023 at 9:54 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 31/05/23 23:32, Namhyung Kim wrote:
> > On large systems, it's common that PID/TID is bigger than 5-digit and it
> > makes the output unaligned.  Let's increase the width to 7.
>
> Might be worth noting that currently the biggest PID_MAX_LIMIT
> is 2^22 so pids don't get bigger than 7 digits presently.

Nice, will add that.

>
> >
> > Before:
> >
> >   $ perf script
> >   ...
> >            swapper     0 [006] 1540823.803935:    1369324 cycles:P:  ffffffff9c755588 ktime_get+0x18 ([kernel.kallsyms])
> >        gvfsd-dnssd 95114 [004] 1540823.804164:    1643871 cycles:P:  ffffffff9cfdca5c __get_user_8+0x1c ([kernel.kallsyms])
> >          perf-exec 1558582 [000] 1540823.804209:    1018714 cycles:P:  ffffffff9c924ab9 __slab_free+0x9 ([kernel.kallsyms])
> >              nmcli 1558589 [007] 1540823.804384:    1859212 cycles:P:      7f70537a8ad8 __strchrnul_evex+0x18 (/usr/lib/x86_64-linux-gnu/libc.so.6>
> >              sleep 1558582 [000] 1540823.804456:     987425 cycles:P:      7fd35bb27b30 _dl_init+0x0 (/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2>
> >        dbus-daemon  3043 [003] 1540823.804575:    1564465 cycles:P:  ffffffff9cb2bb70 llist_add_batch+0x0 ([kernel.kallsyms])
> >              gdbus 1558592 [001] 1540823.804766:    1315219 cycles:P:  ffffffff9c797b2e audit_filter_syscall+0x9e ([kernel.kallsyms])
> >     NetworkManager  3452 [005] 1540823.805301:    1558782 cycles:P:      7fa957737748 g_bit_lock+0x58 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.5>
> >
> > After:
> >
> >   $ perf script
> >   ...
> >            swapper       0 [006] 1540823.803935:    1369324 cycles:P:  ffffffff9c755588 ktime_get+0x18 ([kernel.kallsyms])
> >        gvfsd-dnssd   95114 [004] 1540823.804164:    1643871 cycles:P:  ffffffff9cfdca5c __get_user_8+0x1c ([kernel.kallsyms])
> >          perf-exec 1558582 [000] 1540823.804209:    1018714 cycles:P:  ffffffff9c924ab9 __slab_free+0x9 ([kernel.kallsyms])
> >              nmcli 1558589 [007] 1540823.804384:    1859212 cycles:P:      7f70537a8ad8 __strchrnul_evex+0x18 (/usr/lib/x86_64-linux-gnu/libc.so.6>
> >              sleep 1558582 [000] 1540823.804456:     987425 cycles:P:      7fd35bb27b30 _dl_init+0x0 (/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2>
> >        dbus-daemon    3043 [003] 1540823.804575:    1564465 cycles:P:  ffffffff9cb2bb70 llist_add_batch+0x0 ([kernel.kallsyms])
> >              gdbus 1558592 [001] 1540823.804766:    1315219 cycles:P:  ffffffff9c797b2e audit_filter_syscall+0x9e ([kernel.kallsyms])
> >     NetworkManager    3452 [005] 1540823.805301:    1558782 cycles:P:      7fa957737748 g_bit_lock+0x58 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.5>
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks for your review!
Namhyung


>
> > ---
> >  tools/perf/builtin-script.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 029d5a597233..70549fc93b12 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -801,11 +801,11 @@ static int perf_sample__fprintf_start(struct perf_script *script,
> >       }
> >
> >       if (PRINT_FIELD(PID) && PRINT_FIELD(TID))
> > -             printed += fprintf(fp, "%5d/%-5d ", sample->pid, sample->tid);
> > +             printed += fprintf(fp, "%7d/%-7d ", sample->pid, sample->tid);
> >       else if (PRINT_FIELD(PID))
> > -             printed += fprintf(fp, "%5d ", sample->pid);
> > +             printed += fprintf(fp, "%7d ", sample->pid);
> >       else if (PRINT_FIELD(TID))
> > -             printed += fprintf(fp, "%5d ", sample->tid);
> > +             printed += fprintf(fp, "%7d ", sample->tid);
> >
> >       if (PRINT_FIELD(CPU)) {
> >               if (latency_format)
>