Commit 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
added support for printing cgroup path in perf script output.
It was okay if you didn't want any stacks:
$ sudo perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup
jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
With stacks it gets messier as cgroup is printed after the stack:
$ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
jpegtran:23f4bf 3321915 [013] 404718.587488:
5c554 compress_output
570d9 jpeg_finish_compress
3476e jpegtran_main
330ee jpegtran::main
326e2 core::ops::function::FnOnce::call_once (inlined)
326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
/idle.slice/polish.service
jpegtran:23f4bf 3321915 [031] 404718.592073:
8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
55af68e62fff [unknown]
/idle.slice/polish.service
Let's instead print cgroup on the same line as comm:
$ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
5c554 compress_output
570d9 jpeg_finish_compress
3476e jpegtran_main
330ee jpegtran::main
326e2 core::ops::function::FnOnce::call_once (inlined)
326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
55af68e62fff [unknown]
Signed-off-by: Ivan Babrou <[email protected]>
Fixes: 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
---
tools/perf/builtin-script.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 200b3e7ea8da..517bf25750c8 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2199,6 +2199,17 @@ static void process_event(struct perf_script *script,
if (PRINT_FIELD(RETIRE_LAT))
fprintf(fp, "%16" PRIu16, sample->retire_lat);
+ if (PRINT_FIELD(CGROUP)) {
+ const char *cgrp_name;
+ struct cgroup *cgrp = cgroup__find(machine->env,
+ sample->cgroup);
+ if (cgrp != NULL)
+ cgrp_name = cgrp->name;
+ else
+ cgrp_name = "unknown";
+ fprintf(fp, " %s", cgrp_name);
+ }
+
if (PRINT_FIELD(IP)) {
struct callchain_cursor *cursor = NULL;
@@ -2243,17 +2254,6 @@ static void process_event(struct perf_script *script,
if (PRINT_FIELD(CODE_PAGE_SIZE))
fprintf(fp, " %s", get_page_size_name(sample->code_page_size, str));
- if (PRINT_FIELD(CGROUP)) {
- const char *cgrp_name;
- struct cgroup *cgrp = cgroup__find(machine->env,
- sample->cgroup);
- if (cgrp != NULL)
- cgrp_name = cgrp->name;
- else
- cgrp_name = "unknown";
- fprintf(fp, " %s", cgrp_name);
- }
-
perf_sample__fprintf_ipc(sample, attr, fp);
fprintf(fp, "\n");
--
2.41.0
On Mon, Jul 17, 2023 at 5:07 PM Ivan Babrou <[email protected]> wrote:
>
> Commit 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> added support for printing cgroup path in perf script output.
>
> It was okay if you didn't want any stacks:
>
> $ sudo perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup
> jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
>
> With stacks it gets messier as cgroup is printed after the stack:
>
> $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> jpegtran:23f4bf 3321915 [013] 404718.587488:
> 5c554 compress_output
> 570d9 jpeg_finish_compress
> 3476e jpegtran_main
> 330ee jpegtran::main
> 326e2 core::ops::function::FnOnce::call_once (inlined)
> 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> /idle.slice/polish.service
> jpegtran:23f4bf 3321915 [031] 404718.592073:
> 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> 55af68e62fff [unknown]
> /idle.slice/polish.service
>
> Let's instead print cgroup on the same line as comm:
>
> $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> 5c554 compress_output
> 570d9 jpeg_finish_compress
> 3476e jpegtran_main
> 330ee jpegtran::main
> 326e2 core::ops::function::FnOnce::call_once (inlined)
> 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
>
> jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> 55af68e62fff [unknown]
>
> Signed-off-by: Ivan Babrou <[email protected]>
> Fixes: 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> ---
> tools/perf/builtin-script.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 200b3e7ea8da..517bf25750c8 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2199,6 +2199,17 @@ static void process_event(struct perf_script *script,
> if (PRINT_FIELD(RETIRE_LAT))
> fprintf(fp, "%16" PRIu16, sample->retire_lat);
>
> + if (PRINT_FIELD(CGROUP)) {
> + const char *cgrp_name;
> + struct cgroup *cgrp = cgroup__find(machine->env,
> + sample->cgroup);
> + if (cgrp != NULL)
> + cgrp_name = cgrp->name;
> + else
> + cgrp_name = "unknown";
> + fprintf(fp, " %s", cgrp_name);
> + }
> +
> if (PRINT_FIELD(IP)) {
> struct callchain_cursor *cursor = NULL;
>
> @@ -2243,17 +2254,6 @@ static void process_event(struct perf_script *script,
> if (PRINT_FIELD(CODE_PAGE_SIZE))
> fprintf(fp, " %s", get_page_size_name(sample->code_page_size, str));
>
> - if (PRINT_FIELD(CGROUP)) {
> - const char *cgrp_name;
> - struct cgroup *cgrp = cgroup__find(machine->env,
> - sample->cgroup);
> - if (cgrp != NULL)
> - cgrp_name = cgrp->name;
> - else
> - cgrp_name = "unknown";
> - fprintf(fp, " %s", cgrp_name);
> - }
> -
> perf_sample__fprintf_ipc(sample, attr, fp);
>
> fprintf(fp, "\n");
> --
> 2.41.0
>
A friendly bump in case this slipped through the cracks.
On Fri, Jul 28, 2023 at 10:42 AM Ivan Babrou <[email protected]> wrote:
>
> On Mon, Jul 17, 2023 at 5:07 PM Ivan Babrou <[email protected]> wrote:
> >
> > Commit 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> > added support for printing cgroup path in perf script output.
> >
> > It was okay if you didn't want any stacks:
> >
> > $ sudo perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup
> > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> >
> > With stacks it gets messier as cgroup is printed after the stack:
> >
> > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > jpegtran:23f4bf 3321915 [013] 404718.587488:
> > 5c554 compress_output
> > 570d9 jpeg_finish_compress
> > 3476e jpegtran_main
> > 330ee jpegtran::main
> > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > /idle.slice/polish.service
> > jpegtran:23f4bf 3321915 [031] 404718.592073:
> > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > 55af68e62fff [unknown]
> > /idle.slice/polish.service
> >
> > Let's instead print cgroup on the same line as comm:
> >
> > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > 5c554 compress_output
> > 570d9 jpeg_finish_compress
> > 3476e jpegtran_main
> > 330ee jpegtran::main
> > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> >
> > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > 55af68e62fff [unknown]
> >
> > Signed-off-by: Ivan Babrou <[email protected]>
> > Fixes: 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
This change makes sense to me. Namhyung, wdyt?
Thanks,
Ian
> > ---
> > tools/perf/builtin-script.c | 22 +++++++++++-----------
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 200b3e7ea8da..517bf25750c8 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -2199,6 +2199,17 @@ static void process_event(struct perf_script *script,
> > if (PRINT_FIELD(RETIRE_LAT))
> > fprintf(fp, "%16" PRIu16, sample->retire_lat);
> >
> > + if (PRINT_FIELD(CGROUP)) {
> > + const char *cgrp_name;
> > + struct cgroup *cgrp = cgroup__find(machine->env,
> > + sample->cgroup);
> > + if (cgrp != NULL)
> > + cgrp_name = cgrp->name;
> > + else
> > + cgrp_name = "unknown";
> > + fprintf(fp, " %s", cgrp_name);
> > + }
> > +
> > if (PRINT_FIELD(IP)) {
> > struct callchain_cursor *cursor = NULL;
> >
> > @@ -2243,17 +2254,6 @@ static void process_event(struct perf_script *script,
> > if (PRINT_FIELD(CODE_PAGE_SIZE))
> > fprintf(fp, " %s", get_page_size_name(sample->code_page_size, str));
> >
> > - if (PRINT_FIELD(CGROUP)) {
> > - const char *cgrp_name;
> > - struct cgroup *cgrp = cgroup__find(machine->env,
> > - sample->cgroup);
> > - if (cgrp != NULL)
> > - cgrp_name = cgrp->name;
> > - else
> > - cgrp_name = "unknown";
> > - fprintf(fp, " %s", cgrp_name);
> > - }
> > -
> > perf_sample__fprintf_ipc(sample, attr, fp);
> >
> > fprintf(fp, "\n");
> > --
> > 2.41.0
> >
>
> A friendly bump in case this slipped through the cracks.
On Fri, Jul 28, 2023 at 10:57 AM Ian Rogers <[email protected]> wrote:
>
> On Fri, Jul 28, 2023 at 10:42 AM Ivan Babrou <[email protected]> wrote:
> >
> > On Mon, Jul 17, 2023 at 5:07 PM Ivan Babrou <[email protected]> wrote:
> > >
> > > Commit 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> > > added support for printing cgroup path in perf script output.
> > >
> > > It was okay if you didn't want any stacks:
> > >
> > > $ sudo perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup
> > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> > >
> > > With stacks it gets messier as cgroup is printed after the stack:
> > >
> > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > jpegtran:23f4bf 3321915 [013] 404718.587488:
> > > 5c554 compress_output
> > > 570d9 jpeg_finish_compress
> > > 3476e jpegtran_main
> > > 330ee jpegtran::main
> > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > > /idle.slice/polish.service
> > > jpegtran:23f4bf 3321915 [031] 404718.592073:
> > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > 55af68e62fff [unknown]
> > > /idle.slice/polish.service
> > >
> > > Let's instead print cgroup on the same line as comm:
> > >
> > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > 5c554 compress_output
> > > 570d9 jpeg_finish_compress
> > > 3476e jpegtran_main
> > > 330ee jpegtran::main
> > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > >
> > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > 55af68e62fff [unknown]
> > >
> > > Signed-off-by: Ivan Babrou <[email protected]>
> > > Fixes: 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
>
> This change makes sense to me. Namhyung, wdyt?
>
> Thanks,
> Ian
Hi Namhyung,
This is a really trivial patch and it would be good to get a word from you.
Thanks.
> > > ---
> > > tools/perf/builtin-script.c | 22 +++++++++++-----------
> > > 1 file changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > > index 200b3e7ea8da..517bf25750c8 100644
> > > --- a/tools/perf/builtin-script.c
> > > +++ b/tools/perf/builtin-script.c
> > > @@ -2199,6 +2199,17 @@ static void process_event(struct perf_script *script,
> > > if (PRINT_FIELD(RETIRE_LAT))
> > > fprintf(fp, "%16" PRIu16, sample->retire_lat);
> > >
> > > + if (PRINT_FIELD(CGROUP)) {
> > > + const char *cgrp_name;
> > > + struct cgroup *cgrp = cgroup__find(machine->env,
> > > + sample->cgroup);
> > > + if (cgrp != NULL)
> > > + cgrp_name = cgrp->name;
> > > + else
> > > + cgrp_name = "unknown";
> > > + fprintf(fp, " %s", cgrp_name);
> > > + }
> > > +
> > > if (PRINT_FIELD(IP)) {
> > > struct callchain_cursor *cursor = NULL;
> > >
> > > @@ -2243,17 +2254,6 @@ static void process_event(struct perf_script *script,
> > > if (PRINT_FIELD(CODE_PAGE_SIZE))
> > > fprintf(fp, " %s", get_page_size_name(sample->code_page_size, str));
> > >
> > > - if (PRINT_FIELD(CGROUP)) {
> > > - const char *cgrp_name;
> > > - struct cgroup *cgrp = cgroup__find(machine->env,
> > > - sample->cgroup);
> > > - if (cgrp != NULL)
> > > - cgrp_name = cgrp->name;
> > > - else
> > > - cgrp_name = "unknown";
> > > - fprintf(fp, " %s", cgrp_name);
> > > - }
> > > -
> > > perf_sample__fprintf_ipc(sample, attr, fp);
> > >
> > > fprintf(fp, "\n");
> > > --
> > > 2.41.0
> > >
> >
> > A friendly bump in case this slipped through the cracks.
Em Mon, Aug 07, 2023 at 11:02:01AM -0700, Ivan Babrou escreveu:
> On Fri, Jul 28, 2023 at 10:57 AM Ian Rogers <[email protected]> wrote:
> > On Fri, Jul 28, 2023 at 10:42 AM Ivan Babrou <[email protected]> wrote:
> > > On Mon, Jul 17, 2023 at 5:07 PM Ivan Babrou <[email protected]> wrote:
> > > > Commit 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> > > > added support for printing cgroup path in perf script output.
> > > > It was okay if you didn't want any stacks:
> > > > $ sudo perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup
> > > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> > > > With stacks it gets messier as cgroup is printed after the stack:
> > > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > > jpegtran:23f4bf 3321915 [013] 404718.587488:
> > > > 5c554 compress_output
> > > > 570d9 jpeg_finish_compress
> > > > 3476e jpegtran_main
> > > > 330ee jpegtran::main
> > > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > > > /idle.slice/polish.service
> > > > jpegtran:23f4bf 3321915 [031] 404718.592073:
> > > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > > 55af68e62fff [unknown]
> > > > /idle.slice/polish.service
> > > >
> > > > Let's instead print cgroup on the same line as comm:
> > > >
> > > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > > 5c554 compress_output
> > > > 570d9 jpeg_finish_compress
> > > > 3476e jpegtran_main
> > > > 330ee jpegtran::main
> > > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > > >
> > > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> > > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > > 55af68e62fff [unknown]
> > > >
> > > > Signed-off-by: Ivan Babrou <[email protected]>
> > > > Fixes: 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> > This change makes sense to me. Namhyung, wdyt?
> Hi Namhyung,
>
> This is a really trivial patch and it would be good to get a word from you.
Hi, this solves the case for cgroup and I think it should be merged, but
what about the other fields that are being printed after the callchain
gets printed?
I looked and we would have to introduce a __sample__fprintf_sym that
didn't call sample__fprintf_callchain and use it in perf script's
process_event() then later call sample__fprintf_callchain after all the
fields that print on the same line.
Anyway, Namhyung, can I have your Acked-by for this patch to move things
forward at least for cgroups?
- Arnaldo
Em Tue, Aug 08, 2023 at 10:44:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Aug 07, 2023 at 11:02:01AM -0700, Ivan Babrou escreveu:
> > On Fri, Jul 28, 2023 at 10:57 AM Ian Rogers <[email protected]> wrote:
> > > On Fri, Jul 28, 2023 at 10:42 AM Ivan Babrou <[email protected]> wrote:
> > > > On Mon, Jul 17, 2023 at 5:07 PM Ivan Babrou <[email protected]> wrote:
> > > > > Commit 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> > > > > added support for printing cgroup path in perf script output.
>
> > > > > It was okay if you didn't want any stacks:
>
> > > > > $ sudo perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup
> > > > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
>
> > > > > With stacks it gets messier as cgroup is printed after the stack:
>
> > > > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > > > jpegtran:23f4bf 3321915 [013] 404718.587488:
> > > > > 5c554 compress_output
> > > > > 570d9 jpeg_finish_compress
> > > > > 3476e jpegtran_main
> > > > > 330ee jpegtran::main
> > > > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > > > > /idle.slice/polish.service
> > > > > jpegtran:23f4bf 3321915 [031] 404718.592073:
> > > > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > > > 55af68e62fff [unknown]
> > > > > /idle.slice/polish.service
> > > > >
> > > > > Let's instead print cgroup on the same line as comm:
> > > > >
> > > > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > > > 5c554 compress_output
> > > > > 570d9 jpeg_finish_compress
> > > > > 3476e jpegtran_main
> > > > > 330ee jpegtran::main
> > > > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > > > >
> > > > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> > > > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > > > 55af68e62fff [unknown]
> > > > >
> > > > > Signed-off-by: Ivan Babrou <[email protected]>
> > > > > Fixes: 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
>
> > > This change makes sense to me. Namhyung, wdyt?
>
> > Hi Namhyung,
> >
> > This is a really trivial patch and it would be good to get a word from you.
>
> Hi, this solves the case for cgroup and I think it should be merged, but
> what about the other fields that are being printed after the callchain
> gets printed?
>
> I looked and we would have to introduce a __sample__fprintf_sym that
> didn't call sample__fprintf_callchain and use it in perf script's
> process_event() then later call sample__fprintf_callchain after all the
> fields that print on the same line.
Nah, or simply moving sample__fprintf_sym() to the end of that function.
- Arnaldo
> Anyway, Namhyung, can I have your Acked-by for this patch to move things
> forward at least for cgroups?
Hello,
Sorry for the late reply.
On Tue, Aug 8, 2023 at 10:44 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Mon, Aug 07, 2023 at 11:02:01AM -0700, Ivan Babrou escreveu:
> > On Fri, Jul 28, 2023 at 10:57 AM Ian Rogers <[email protected]> wrote:
> > > On Fri, Jul 28, 2023 at 10:42 AM Ivan Babrou <[email protected]> wrote:
> > > > On Mon, Jul 17, 2023 at 5:07 PM Ivan Babrou <[email protected]> wrote:
> > > > > Commit 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
> > > > > added support for printing cgroup path in perf script output.
>
> > > > > It was okay if you didn't want any stacks:
>
> > > > > $ sudo perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup
> > > > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
>
> > > > > With stacks it gets messier as cgroup is printed after the stack:
>
> > > > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > > > jpegtran:23f4bf 3321915 [013] 404718.587488:
> > > > > 5c554 compress_output
> > > > > 570d9 jpeg_finish_compress
> > > > > 3476e jpegtran_main
> > > > > 330ee jpegtran::main
> > > > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > > > > /idle.slice/polish.service
> > > > > jpegtran:23f4bf 3321915 [031] 404718.592073:
> > > > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > > > 55af68e62fff [unknown]
> > > > > /idle.slice/polish.service
> > > > >
> > > > > Let's instead print cgroup on the same line as comm:
> > > > >
> > > > > $ perf script --comms jpegtran:23f4bf -F comm,tid,cpu,time,cgroup,ip,sym
> > > > > jpegtran:23f4bf 3321915 [013] 404718.587488: /idle.slice/polish.service
> > > > > 5c554 compress_output
> > > > > 570d9 jpeg_finish_compress
> > > > > 3476e jpegtran_main
> > > > > 330ee jpegtran::main
> > > > > 326e2 core::ops::function::FnOnce::call_once (inlined)
> > > > > 326e2 std::sys_common::backtrace::__rust_begin_short_backtrace
> > > > >
> > > > > jpegtran:23f4bf 3321915 [031] 404718.592073: /idle.slice/polish.service
> > > > > 8474d jsimd_encode_mcu_AC_first_prepare_sse2.PADDING
> > > > > 55af68e62fff [unknown]
> > > > >
> > > > > Signed-off-by: Ivan Babrou <[email protected]>
> > > > > Fixes: 3fd7a168bf51 ("perf script: Add 'cgroup' field for output")
>
> > > This change makes sense to me. Namhyung, wdyt?
>
> > Hi Namhyung,
> >
> > This is a really trivial patch and it would be good to get a word from you.
>
> Hi, this solves the case for cgroup and I think it should be merged, but
> what about the other fields that are being printed after the callchain
> gets printed?
>
> I looked and we would have to introduce a __sample__fprintf_sym that
> didn't call sample__fprintf_callchain and use it in perf script's
> process_event() then later call sample__fprintf_callchain after all the
> fields that print on the same line.
>
> Anyway, Namhyung, can I have your Acked-by for this patch to move things
> forward at least for cgroups?
I'm ok with the change itself. But I'm afraid other fields might be
changed too. Anyway,
Acked-by: Namhyung Kim <[email protected]>
Thanks,
Namhyung