* [PATCH] perf record: Synthesize cgroup events only if needed
@ 2020-11-27 5:43 Namhyung Kim
2020-11-27 15:45 ` Jiri Olsa
0 siblings, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2020-11-27 5:43 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Ian Rogers
It didn't check the tool->cgroup_events bit which is set when
the --all-cgroups option is given. Without it, samples will not have
cgroup info so no reason to synthesize.
We can check the PERF_RECORD_CGROUP records after running perf record
*WITHOUT* the --all-cgroups option:
Before:
$ perf report -D | grep CGROUP
0 0 0x8430 [0x38]: PERF_RECORD_CGROUP cgroup: 1 /
CGROUP events: 1
CGROUP events: 0
CGROUP events: 0
After:
$ perf report -D | grep CGROUP
CGROUP events: 0
CGROUP events: 0
CGROUP events: 0
Fixes: 8fb4b67939e16 ("perf record: Add --all-cgroups option")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/synthetic-events.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 8a23391558cf..d9c624377da7 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -563,6 +563,9 @@ int perf_event__synthesize_cgroups(struct perf_tool *tool,
char cgrp_root[PATH_MAX];
size_t mount_len; /* length of mount point in the path */
+ if (!tool || !tool->cgroup_events)
+ return 0;
+
if (cgroupfs_find_mountpoint(cgrp_root, PATH_MAX, "perf_event") < 0) {
pr_debug("cannot find cgroup mount point\n");
return -1;
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] perf record: Synthesize cgroup events only if needed
2020-11-27 5:43 [PATCH] perf record: Synthesize cgroup events only if needed Namhyung Kim
@ 2020-11-27 15:45 ` Jiri Olsa
2020-11-27 17:30 ` Arnaldo Carvalho de Melo
2020-11-30 13:26 ` Namhyung Kim
0 siblings, 2 replies; 5+ messages in thread
From: Jiri Olsa @ 2020-11-27 15:45 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Ian Rogers
On Fri, Nov 27, 2020 at 02:43:56PM +0900, Namhyung Kim wrote:
> It didn't check the tool->cgroup_events bit which is set when
> the --all-cgroups option is given. Without it, samples will not have
> cgroup info so no reason to synthesize.
>
> We can check the PERF_RECORD_CGROUP records after running perf record
> *WITHOUT* the --all-cgroups option:
>
> Before:
> $ perf report -D | grep CGROUP
> 0 0 0x8430 [0x38]: PERF_RECORD_CGROUP cgroup: 1 /
> CGROUP events: 1
> CGROUP events: 0
> CGROUP events: 0
>
> After:
> $ perf report -D | grep CGROUP
> CGROUP events: 0
> CGROUP events: 0
> CGROUP events: 0
>
> Fixes: 8fb4b67939e16 ("perf record: Add --all-cgroups option")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/synthetic-events.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 8a23391558cf..d9c624377da7 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -563,6 +563,9 @@ int perf_event__synthesize_cgroups(struct perf_tool *tool,
> char cgrp_root[PATH_MAX];
> size_t mount_len; /* length of mount point in the path */
>
> + if (!tool || !tool->cgroup_events)
> + return 0;
can !tool actually happen here? or it's just being extra cautious
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
> +
> if (cgroupfs_find_mountpoint(cgrp_root, PATH_MAX, "perf_event") < 0) {
> pr_debug("cannot find cgroup mount point\n");
> return -1;
> --
> 2.29.2.454.gaff20da3a2-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf record: Synthesize cgroup events only if needed
2020-11-27 15:45 ` Jiri Olsa
@ 2020-11-27 17:30 ` Arnaldo Carvalho de Melo
2020-11-30 13:27 ` Namhyung Kim
2020-11-30 13:26 ` Namhyung Kim
1 sibling, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-11-27 17:30 UTC (permalink / raw)
To: Jiri Olsa
Cc: Namhyung Kim, Ingo Molnar, Peter Zijlstra, Mark Rutland,
Alexander Shishkin, LKML, Stephane Eranian, Ian Rogers
Em Fri, Nov 27, 2020 at 04:45:57PM +0100, Jiri Olsa escreveu:
> On Fri, Nov 27, 2020 at 02:43:56PM +0900, Namhyung Kim wrote:
> > It didn't check the tool->cgroup_events bit which is set when
> > the --all-cgroups option is given. Without it, samples will not have
> > cgroup info so no reason to synthesize.
> >
> > We can check the PERF_RECORD_CGROUP records after running perf record
> > *WITHOUT* the --all-cgroups option:
> >
> > Before:
> > $ perf report -D | grep CGROUP
> > 0 0 0x8430 [0x38]: PERF_RECORD_CGROUP cgroup: 1 /
> > CGROUP events: 1
> > CGROUP events: 0
> > CGROUP events: 0
> >
> > After:
> > $ perf report -D | grep CGROUP
> > CGROUP events: 0
> > CGROUP events: 0
> > CGROUP events: 0
> >
> > Fixes: 8fb4b67939e16 ("perf record: Add --all-cgroups option")
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/synthetic-events.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> > index 8a23391558cf..d9c624377da7 100644
> > --- a/tools/perf/util/synthetic-events.c
> > +++ b/tools/perf/util/synthetic-events.c
> > @@ -563,6 +563,9 @@ int perf_event__synthesize_cgroups(struct perf_tool *tool,
> > char cgrp_root[PATH_MAX];
> > size_t mount_len; /* length of mount point in the path */
> >
> > + if (!tool || !tool->cgroup_events)
> > + return 0;
>
> can !tool actually happen here? or it's just being extra cautious
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
Thanks, tested with/without --all-cgroups and applied.
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf record: Synthesize cgroup events only if needed
2020-11-27 17:30 ` Arnaldo Carvalho de Melo
@ 2020-11-30 13:27 ` Namhyung Kim
0 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2020-11-30 13:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, Mark Rutland,
Alexander Shishkin, LKML, Stephane Eranian, Ian Rogers
Hi Arnaldo,
On Sat, Nov 28, 2020 at 2:30 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Fri, Nov 27, 2020 at 04:45:57PM +0100, Jiri Olsa escreveu:
> > On Fri, Nov 27, 2020 at 02:43:56PM +0900, Namhyung Kim wrote:
> > > It didn't check the tool->cgroup_events bit which is set when
> > > the --all-cgroups option is given. Without it, samples will not have
> > > cgroup info so no reason to synthesize.
> > >
> > > We can check the PERF_RECORD_CGROUP records after running perf record
> > > *WITHOUT* the --all-cgroups option:
> > >
> > > Before:
> > > $ perf report -D | grep CGROUP
> > > 0 0 0x8430 [0x38]: PERF_RECORD_CGROUP cgroup: 1 /
> > > CGROUP events: 1
> > > CGROUP events: 0
> > > CGROUP events: 0
> > >
> > > After:
> > > $ perf report -D | grep CGROUP
> > > CGROUP events: 0
> > > CGROUP events: 0
> > > CGROUP events: 0
> > >
> > > Fixes: 8fb4b67939e16 ("perf record: Add --all-cgroups option")
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > > tools/perf/util/synthetic-events.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> > > index 8a23391558cf..d9c624377da7 100644
> > > --- a/tools/perf/util/synthetic-events.c
> > > +++ b/tools/perf/util/synthetic-events.c
> > > @@ -563,6 +563,9 @@ int perf_event__synthesize_cgroups(struct perf_tool *tool,
> > > char cgrp_root[PATH_MAX];
> > > size_t mount_len; /* length of mount point in the path */
> > >
> > > + if (!tool || !tool->cgroup_events)
> > > + return 0;
> >
> > can !tool actually happen here? or it's just being extra cautious
> >
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
>
> Thanks, tested with/without --all-cgroups and applied.
Thanks for doing that!
Namhyung
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf record: Synthesize cgroup events only if needed
2020-11-27 15:45 ` Jiri Olsa
2020-11-27 17:30 ` Arnaldo Carvalho de Melo
@ 2020-11-30 13:26 ` Namhyung Kim
1 sibling, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2020-11-30 13:26 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Ian Rogers
Hi Jiri,
On Sat, Nov 28, 2020 at 12:46 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Nov 27, 2020 at 02:43:56PM +0900, Namhyung Kim wrote:
> > It didn't check the tool->cgroup_events bit which is set when
> > the --all-cgroups option is given. Without it, samples will not have
> > cgroup info so no reason to synthesize.
> >
> > We can check the PERF_RECORD_CGROUP records after running perf record
> > *WITHOUT* the --all-cgroups option:
> >
> > Before:
> > $ perf report -D | grep CGROUP
> > 0 0 0x8430 [0x38]: PERF_RECORD_CGROUP cgroup: 1 /
> > CGROUP events: 1
> > CGROUP events: 0
> > CGROUP events: 0
> >
> > After:
> > $ perf report -D | grep CGROUP
> > CGROUP events: 0
> > CGROUP events: 0
> > CGROUP events: 0
> >
> > Fixes: 8fb4b67939e16 ("perf record: Add --all-cgroups option")
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/synthetic-events.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> > index 8a23391558cf..d9c624377da7 100644
> > --- a/tools/perf/util/synthetic-events.c
> > +++ b/tools/perf/util/synthetic-events.c
> > @@ -563,6 +563,9 @@ int perf_event__synthesize_cgroups(struct perf_tool *tool,
> > char cgrp_root[PATH_MAX];
> > size_t mount_len; /* length of mount point in the path */
> >
> > + if (!tool || !tool->cgroup_events)
> > + return 0;
>
> can !tool actually happen here? or it's just being extra cautious
Actually I just copied the namespace code. :).
It's only called from perf record and top, and neither of them pass NULL.
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-30 13:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-27 5:43 [PATCH] perf record: Synthesize cgroup events only if needed Namhyung Kim
2020-11-27 15:45 ` Jiri Olsa
2020-11-27 17:30 ` Arnaldo Carvalho de Melo
2020-11-30 13:27 ` Namhyung Kim
2020-11-30 13:26 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).