[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZPidlwe2yXEDZB+U@kernel.org>
Date: Wed, 6 Sep 2023 12:41:11 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org, Song Liu <song@...nel.org>,
Hao Luo <haoluo@...gle.com>, bpf@...r.kernel.org
Subject: Re: [PATCH 1/5] perf tools: Add read_all_cgroups() and
__cgroup_find()
Em Wed, Sep 06, 2023 at 12:38:17PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 30, 2023 at 04:01:22PM -0700, Namhyung Kim escreveu:
> > The read_all_cgroups() is to build a tree of cgroups in the system and
> > users can look up a cgroup using __cgroup_find().
>
> ⬢[acme@...lbox perf-tools-next]$ alias m='make -k BUILD_BPF_SKEL=1 CORESIGHT=1 O=/tmp/build/perf-tools-next -C tools/perf install-bin && git status && perf test python'
> ⬢[acme@...lbox perf-tools-next]$ m
> make: Entering directory '/var/home/acme/git/perf-tools-next/tools/perf'
> BUILD: Doing 'make -j32' parallel build
> Warning: Kernel ABI header differences:
> diff -u tools/include/uapi/linux/perf_event.h include/uapi/linux/perf_event.h
> diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h
> diff -u tools/arch/x86/include/asm/msr-index.h arch/x86/include/asm/msr-index.h
> diff -u tools/arch/arm64/include/uapi/asm/perf_regs.h arch/arm64/include/uapi/asm/perf_regs.h
>
> INSTALL libsubcmd_headers
> INSTALL libperf_headers
> INSTALL libapi_headers
> INSTALL libsymbol_headers
> INSTALL libbpf_headers
> CC /tmp/build/perf-tools-next/builtin-lock.o
> CC /tmp/build/perf-tools-next/util/bpf_lock_contention.o
> builtin-lock.c: In function ‘__cmd_contention’:
> builtin-lock.c:2162:9: error: too few arguments to function ‘lock_contention_finish’
> 2162 | lock_contention_finish();
> | ^~~~~~~~~~~~~~~~~~~~~~
> In file included from builtin-lock.c:14:
> util/lock-contention.h:156:5: note: declared here
> 156 | int lock_contention_finish(struct lock_contention *con);
> | ^~~~~~~~~~~~~~~~~~~~~~
> make[3]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: /tmp/build/perf-tools-next/builtin-lock.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> util/bpf_lock_contention.c: In function ‘lock_contention_get_name’:
> util/bpf_lock_contention.c:231:34: error: ‘struct contention_key’ has no member named ‘lock_addr_or_cgroup’
> 231 | u64 cgrp_id = key->lock_addr_or_cgroup;
> | ^~
> make[4]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: /tmp/build/perf-tools-next/util/bpf_lock_contention.o] Error 1
> make[3]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:150: util] Error 2
> make[2]: *** [Makefile.perf:662: /tmp/build/perf-tools-next/perf-in.o] Error 2
> make[1]: *** [Makefile.perf:238: sub-make] Error 2
> make: *** [Makefile:113: install-bin] Error 2
> make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf'
> ⬢[acme@...lbox perf-tools-next]$
>
> Trying to figure this out.
So it works on the following patch:
⬢[acme@...lbox perf-tools-next]$ git log --oneline -5
94a54d498ae35c66 (HEAD) perf lock contention: Add -g/--lock-cgroup option
defe88978441a00d perf lock contention: Prepare to handle cgroups
cc0717270d2f0daa perf cgroup: Add read_all_cgroups() and __cgroup_find()
752d73a1dd62cd4a perf shell completion: Support completion of metrics/metricgroups
72aa5816258bf9fe perf completion: Support completion of libpfm4 events
⬢[acme@...lbox perf-tools-next]$
Please check and submit a v2.
- Arnaldo
> - Arnaldo
>
> > Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> > ---
> > tools/perf/util/cgroup.c | 61 ++++++++++++++++++++++++++++++++++------
> > tools/perf/util/cgroup.h | 4 +++
> > 2 files changed, 57 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
> > index bfb13306d82c..2e969d1464f4 100644
> > --- a/tools/perf/util/cgroup.c
> > +++ b/tools/perf/util/cgroup.c
> > @@ -48,28 +48,36 @@ static int open_cgroup(const char *name)
> > }
> >
> > #ifdef HAVE_FILE_HANDLE
> > -int read_cgroup_id(struct cgroup *cgrp)
> > +static u64 __read_cgroup_id(const char *path)
> > {
> > - char path[PATH_MAX + 1];
> > - char mnt[PATH_MAX + 1];
> > struct {
> > struct file_handle fh;
> > uint64_t cgroup_id;
> > } handle;
> > int mount_id;
> >
> > + handle.fh.handle_bytes = sizeof(handle.cgroup_id);
> > + if (name_to_handle_at(AT_FDCWD, path, &handle.fh, &mount_id, 0) < 0)
> > + return -1ULL;
> > +
> > + return handle.cgroup_id;
> > +}
> > +
> > +int read_cgroup_id(struct cgroup *cgrp)
> > +{
> > + char path[PATH_MAX + 1];
> > + char mnt[PATH_MAX + 1];
> > +
> > if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1, "perf_event"))
> > return -1;
> >
> > scnprintf(path, PATH_MAX, "%s/%s", mnt, cgrp->name);
> >
> > - handle.fh.handle_bytes = sizeof(handle.cgroup_id);
> > - if (name_to_handle_at(AT_FDCWD, path, &handle.fh, &mount_id, 0) < 0)
> > - return -1;
> > -
> > - cgrp->id = handle.cgroup_id;
> > + cgrp->id = __read_cgroup_id(path);
> > return 0;
> > }
> > +#else
> > +static inline u64 __read_cgroup_id(const char *path) { return -1ULL; }
> > #endif /* HAVE_FILE_HANDLE */
> >
> > #ifndef CGROUP2_SUPER_MAGIC
> > @@ -562,6 +570,11 @@ struct cgroup *cgroup__findnew(struct perf_env *env, uint64_t id,
> > return cgrp;
> > }
> >
> > +struct cgroup *__cgroup__find(struct rb_root *root, uint64_t id)
> > +{
> > + return __cgroup__findnew(root, id, /*create=*/false, /*path=*/NULL);
> > +}
> > +
> > struct cgroup *cgroup__find(struct perf_env *env, uint64_t id)
> > {
> > struct cgroup *cgrp;
> > @@ -587,3 +600,35 @@ void perf_env__purge_cgroups(struct perf_env *env)
> > }
> > up_write(&env->cgroups.lock);
> > }
> > +
> > +void read_all_cgroups(struct rb_root *root)
> > +{
> > + char mnt[PATH_MAX];
> > + struct cgroup_name *cn;
> > + int prefix_len;
> > +
> > + if (cgroupfs_find_mountpoint(mnt, sizeof(mnt), "perf_event"))
> > + return;
> > +
> > + /* cgroup_name will have a full path, skip the root directory */
> > + prefix_len = strlen(mnt);
> > +
> > + /* collect all cgroups in the cgroup_list */
> > + if (nftw(mnt, add_cgroup_name, 20, 0) < 0)
> > + return;
> > +
> > + list_for_each_entry(cn, &cgroup_list, list) {
> > + const char *name;
> > + u64 cgrp_id;
> > +
> > + /* cgroup_name might have a full path, skip the prefix */
> > + name = cn->name + prefix_len;
> > + if (name[0] == '\0')
> > + name = "/";
> > +
> > + cgrp_id = __read_cgroup_id(cn->name);
> > + __cgroup__findnew(root, cgrp_id, /*create=*/true, name);
> > + }
> > +
> > + release_cgroup_list();
> > +}
> > diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
> > index 12256b78608c..beb6fe1012ed 100644
> > --- a/tools/perf/util/cgroup.h
> > +++ b/tools/perf/util/cgroup.h
> > @@ -37,6 +37,7 @@ int parse_cgroups(const struct option *opt, const char *str, int unset);
> > struct cgroup *cgroup__findnew(struct perf_env *env, uint64_t id,
> > const char *path);
> > struct cgroup *cgroup__find(struct perf_env *env, uint64_t id);
> > +struct cgroup *__cgroup__find(struct rb_root *root, uint64_t id);
> >
> > void perf_env__purge_cgroups(struct perf_env *env);
> >
> > @@ -49,6 +50,9 @@ static inline int read_cgroup_id(struct cgroup *cgrp __maybe_unused)
> > }
> > #endif /* HAVE_FILE_HANDLE */
> >
> > +/* read all cgroups in the system and save them in the rbtree */
> > +void read_all_cgroups(struct rb_root *root);
> > +
> > int cgroup_is_v2(const char *subsys);
> >
> > #endif /* __CGROUP_H__ */
> > --
> > 2.42.0.283.g2d96d420d3-goog
> >
>
> --
>
> - Arnaldo
--
- Arnaldo
Powered by blists - more mailing lists