* [PATCH 1/5] perf tools: Add read_all_cgroups() and __cgroup_find()
2023-09-06 17:48 [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Namhyung Kim
@ 2023-09-06 17:48 ` Namhyung Kim
2023-09-06 17:49 ` [PATCH 2/5] perf lock contention: Prepare to handle cgroups Namhyung Kim
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2023-09-06 17:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Song Liu, Hao Luo, bpf
The read_all_cgroups() is to build a tree of cgroups in the system and
users can look up a cgroup using __cgroup_find().
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/5] perf lock contention: Prepare to handle cgroups
2023-09-06 17:48 [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Namhyung Kim
2023-09-06 17:48 ` [PATCH 1/5] perf tools: Add read_all_cgroups() and __cgroup_find() Namhyung Kim
@ 2023-09-06 17:49 ` Namhyung Kim
2023-09-06 17:49 ` [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option Namhyung Kim
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2023-09-06 17:49 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Song Liu, Hao Luo, bpf
Save cgroup info and display cgroup names if requested. This is a
preparation for the next patch.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-lock.c | 3 ++-
tools/perf/util/bpf_lock_contention.c | 26 +++++++++++++++++++++++++-
tools/perf/util/lock-contention.h | 9 +++++++--
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index b141f2134274..06430980dfd7 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2040,6 +2040,7 @@ static int __cmd_contention(int argc, const char **argv)
.filters = &filters,
.save_callstack = needs_callstack(),
.owner = show_lock_owner,
+ .cgroups = RB_ROOT,
};
lockhash_table = calloc(LOCKHASH_SIZE, sizeof(*lockhash_table));
@@ -2158,7 +2159,7 @@ static int __cmd_contention(int argc, const char **argv)
out_delete:
lock_filter_finish();
evlist__delete(con.evlist);
- lock_contention_finish();
+ lock_contention_finish(&con);
perf_session__delete(session);
zfree(&lockhash_table);
return err;
diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
index e7dddf0127bc..c6bd7c9b2d57 100644
--- a/tools/perf/util/bpf_lock_contention.c
+++ b/tools/perf/util/bpf_lock_contention.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
+#include "util/cgroup.h"
#include "util/debug.h"
#include "util/evlist.h"
#include "util/machine.h"
@@ -151,6 +152,10 @@ int lock_contention_prepare(struct lock_contention *con)
skel->bss->needs_callstack = con->save_callstack;
skel->bss->lock_owner = con->owner;
+ if (con->use_cgroup) {
+ read_all_cgroups(&con->cgroups);
+ }
+
bpf_program__set_autoload(skel->progs.collect_lock_syms, false);
lock_contention_bpf__attach(skel);
@@ -222,6 +227,17 @@ static const char *lock_contention_get_name(struct lock_contention *con,
return "";
}
+ if (con->use_cgroup) {
+ u64 cgrp_id = key->lock_addr;
+ struct cgroup *cgrp = __cgroup__find(&con->cgroups, cgrp_id);
+
+ if (cgrp)
+ return cgrp->name;
+
+ snprintf(name_buf, sizeof(name_buf), "cgroup:%lu", cgrp_id);
+ return name_buf;
+ }
+
/* LOCK_AGGR_CALLER: skip lock internal functions */
while (machine__is_lock_function(machine, stack_trace[idx]) &&
idx < con->max_stack - 1)
@@ -364,12 +380,20 @@ int lock_contention_read(struct lock_contention *con)
return err;
}
-int lock_contention_finish(void)
+int lock_contention_finish(struct lock_contention *con)
{
if (skel) {
skel->bss->enabled = 0;
lock_contention_bpf__destroy(skel);
}
+ while (!RB_EMPTY_ROOT(&con->cgroups)) {
+ struct rb_node *node = rb_first(&con->cgroups);
+ struct cgroup *cgrp = rb_entry(node, struct cgroup, node);
+
+ rb_erase(node, &con->cgroups);
+ cgroup__put(cgrp);
+ }
+
return 0;
}
diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
index fa16532c971c..70423966d778 100644
--- a/tools/perf/util/lock-contention.h
+++ b/tools/perf/util/lock-contention.h
@@ -136,6 +136,7 @@ struct lock_contention {
struct hlist_head *result;
struct lock_filter *filters;
struct lock_contention_fails fails;
+ struct rb_root cgroups;
unsigned long map_nr_entries;
int max_stack;
int stack_skip;
@@ -143,6 +144,7 @@ struct lock_contention {
int owner;
int nr_filtered;
bool save_callstack;
+ bool use_cgroup;
};
#ifdef HAVE_BPF_SKEL
@@ -151,7 +153,7 @@ int lock_contention_prepare(struct lock_contention *con);
int lock_contention_start(void);
int lock_contention_stop(void);
int lock_contention_read(struct lock_contention *con);
-int lock_contention_finish(void);
+int lock_contention_finish(struct lock_contention *con);
#else /* !HAVE_BPF_SKEL */
@@ -162,7 +164,10 @@ static inline int lock_contention_prepare(struct lock_contention *con __maybe_un
static inline int lock_contention_start(void) { return 0; }
static inline int lock_contention_stop(void) { return 0; }
-static inline int lock_contention_finish(void) { return 0; }
+static inline int lock_contention_finish(struct lock_contention *con __maybe_unused)
+{
+ return 0;
+}
static inline int lock_contention_read(struct lock_contention *con __maybe_unused)
{
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option
2023-09-06 17:48 [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Namhyung Kim
2023-09-06 17:48 ` [PATCH 1/5] perf tools: Add read_all_cgroups() and __cgroup_find() Namhyung Kim
2023-09-06 17:49 ` [PATCH 2/5] perf lock contention: Prepare to handle cgroups Namhyung Kim
@ 2023-09-06 17:49 ` Namhyung Kim
2023-09-06 19:23 ` Arnaldo Carvalho de Melo
2023-09-06 17:49 ` [PATCH 4/5] perf lock contention: Add -G/--cgroup-filter option Namhyung Kim
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Namhyung Kim @ 2023-09-06 17:49 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Song Liu, Hao Luo, bpf
The -g option shows lock contention stats break down by cgroups.
Add LOCK_AGGR_CGROUP mode and use it instead of use_cgroup field.
$ sudo ./perf lock con -abg sleep 1
contended total wait max wait avg wait cgroup
8 15.70 us 6.34 us 1.96 us /
2 1.48 us 747 ns 738 ns /user.slice/.../app.slice/app-gnome-google\x2dchrome-6442.scope
1 848 ns 848 ns 848 ns /user.slice/.../session.slice/org.gnome.Shell@x11.service
1 220 ns 220 ns 220 ns /user.slice/.../session.slice/pipewire-pulse.service
For now, the cgroup mode only works with BPF (-b).
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-lock.txt | 4 ++
tools/perf/builtin-lock.c | 40 ++++++++++++++++++-
tools/perf/util/bpf_lock_contention.c | 16 +++++---
.../perf/util/bpf_skel/lock_contention.bpf.c | 31 +++++++++++++-
tools/perf/util/bpf_skel/lock_data.h | 3 +-
tools/perf/util/lock-contention.h | 1 -
6 files changed, 85 insertions(+), 10 deletions(-)
diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
index 30eea576721f..61c491df72b8 100644
--- a/tools/perf/Documentation/perf-lock.txt
+++ b/tools/perf/Documentation/perf-lock.txt
@@ -208,6 +208,10 @@ CONTENTION OPTIONS
Show results using a CSV-style output to make it easy to import directly
into spreadsheets. Columns are separated by the string specified in SEP.
+-g::
+--lock-cgroup::
+ Show lock contention stat by cgroup. Requires --use-bpf.
+
SEE ALSO
--------
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 06430980dfd7..b98948dd40ba 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -60,6 +60,7 @@ static bool combine_locks;
static bool show_thread_stats;
static bool show_lock_addrs;
static bool show_lock_owner;
+static bool show_lock_cgroups;
static bool use_bpf;
static unsigned long bpf_map_entries = MAX_ENTRIES;
static int max_stack_depth = CONTENTION_STACK_DEPTH;
@@ -619,6 +620,7 @@ static int get_key_by_aggr_mode_simple(u64 *key, u64 addr, u32 tid)
*key = tid;
break;
case LOCK_AGGR_CALLER:
+ case LOCK_AGGR_CGROUP:
default:
pr_err("Invalid aggregation mode: %d\n", aggr_mode);
return -EINVAL;
@@ -1103,6 +1105,7 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
if (lock_contention_caller(evsel, sample, buf, sizeof(buf)) < 0)
name = "Unknown";
break;
+ case LOCK_AGGR_CGROUP:
case LOCK_AGGR_TASK:
default:
break;
@@ -1653,6 +1656,9 @@ static void print_header_stdio(void)
case LOCK_AGGR_ADDR:
fprintf(lock_output, " %16s %s\n\n", "address", "symbol");
break;
+ case LOCK_AGGR_CGROUP:
+ fprintf(lock_output, " %s\n\n", "cgroup");
+ break;
default:
break;
}
@@ -1680,6 +1686,9 @@ static void print_header_csv(const char *sep)
case LOCK_AGGR_ADDR:
fprintf(lock_output, "%s%s %s%s %s\n", "address", sep, "symbol", sep, "type");
break;
+ case LOCK_AGGR_CGROUP:
+ fprintf(lock_output, "%s\n", "cgroup");
+ break;
default:
break;
}
@@ -1720,6 +1729,9 @@ static void print_lock_stat_stdio(struct lock_contention *con, struct lock_stat
fprintf(lock_output, " %016llx %s (%s)\n", (unsigned long long)st->addr,
st->name, get_type_name(st->flags));
break;
+ case LOCK_AGGR_CGROUP:
+ fprintf(lock_output, " %s\n", st->name);
+ break;
default:
break;
}
@@ -1770,6 +1782,9 @@ static void print_lock_stat_csv(struct lock_contention *con, struct lock_stat *s
fprintf(lock_output, "%llx%s %s%s %s\n", (unsigned long long)st->addr, sep,
st->name, sep, get_type_name(st->flags));
break;
+ case LOCK_AGGR_CGROUP:
+ fprintf(lock_output, "%s\n",st->name);
+ break;
default:
break;
}
@@ -1999,6 +2014,27 @@ static int check_lock_contention_options(const struct option *options,
return -1;
}
+ if (show_lock_cgroups && !use_bpf) {
+ pr_err("Cgroups are available only with BPF\n");
+ parse_options_usage(usage, options, "lock-cgroup", 0);
+ parse_options_usage(NULL, options, "use-bpf", 0);
+ return -1;
+ }
+
+ if (show_lock_cgroups && show_lock_addrs) {
+ pr_err("Cannot use cgroup and addr mode together\n");
+ parse_options_usage(usage, options, "lock-cgroup", 0);
+ parse_options_usage(NULL, options, "lock-addr", 0);
+ return -1;
+ }
+
+ if (show_lock_cgroups && show_thread_stats) {
+ pr_err("Cannot use cgroup and thread mode together\n");
+ parse_options_usage(usage, options, "lock-cgroup", 0);
+ parse_options_usage(NULL, options, "threads", 0);
+ return -1;
+ }
+
if (symbol_conf.field_sep) {
if (strstr(symbol_conf.field_sep, ":") || /* part of type flags */
strstr(symbol_conf.field_sep, "+") || /* part of caller offset */
@@ -2060,7 +2096,8 @@ static int __cmd_contention(int argc, const char **argv)
con.machine = &session->machines.host;
con.aggr_mode = aggr_mode = show_thread_stats ? LOCK_AGGR_TASK :
- show_lock_addrs ? LOCK_AGGR_ADDR : LOCK_AGGR_CALLER;
+ show_lock_addrs ? LOCK_AGGR_ADDR :
+ show_lock_cgroups ? LOCK_AGGR_CGROUP : LOCK_AGGR_CALLER;
if (con.aggr_mode == LOCK_AGGR_CALLER)
con.save_callstack = true;
@@ -2524,6 +2561,7 @@ int cmd_lock(int argc, const char **argv)
OPT_BOOLEAN('o', "lock-owner", &show_lock_owner, "show lock owners instead of waiters"),
OPT_STRING_NOEMPTY('x', "field-separator", &symbol_conf.field_sep, "separator",
"print result in CSV format with custom separator"),
+ OPT_BOOLEAN('g', "lock-cgroup", &show_lock_cgroups, "show lock stats by cgroup"),
OPT_PARENT(lock_options)
};
diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
index c6bd7c9b2d57..42753a0dfdc5 100644
--- a/tools/perf/util/bpf_lock_contention.c
+++ b/tools/perf/util/bpf_lock_contention.c
@@ -152,7 +152,10 @@ int lock_contention_prepare(struct lock_contention *con)
skel->bss->needs_callstack = con->save_callstack;
skel->bss->lock_owner = con->owner;
- if (con->use_cgroup) {
+ if (con->aggr_mode == LOCK_AGGR_CGROUP) {
+ if (cgroup_is_v2("perf_event"))
+ skel->bss->use_cgroup_v2 = 1;
+
read_all_cgroups(&con->cgroups);
}
@@ -214,12 +217,12 @@ static const char *lock_contention_get_name(struct lock_contention *con,
return "siglock";
/* global locks with symbols */
- sym = machine__find_kernel_symbol(machine, key->lock_addr, &kmap);
+ sym = machine__find_kernel_symbol(machine, key->lock_addr_or_cgroup, &kmap);
if (sym)
return sym->name;
/* try semi-global locks collected separately */
- if (!bpf_map_lookup_elem(lock_fd, &key->lock_addr, &flags)) {
+ if (!bpf_map_lookup_elem(lock_fd, &key->lock_addr_or_cgroup, &flags)) {
if (flags == LOCK_CLASS_RQLOCK)
return "rq_lock";
}
@@ -227,8 +230,8 @@ static const char *lock_contention_get_name(struct lock_contention *con,
return "";
}
- if (con->use_cgroup) {
- u64 cgrp_id = key->lock_addr;
+ if (con->aggr_mode == LOCK_AGGR_CGROUP) {
+ u64 cgrp_id = key->lock_addr_or_cgroup;
struct cgroup *cgrp = __cgroup__find(&con->cgroups, cgrp_id);
if (cgrp)
@@ -329,7 +332,8 @@ int lock_contention_read(struct lock_contention *con)
ls_key = key.pid;
break;
case LOCK_AGGR_ADDR:
- ls_key = key.lock_addr;
+ case LOCK_AGGR_CGROUP:
+ ls_key = key.lock_addr_or_cgroup;
break;
default:
goto next;
diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
index 8d3cfbb3cc65..823354999022 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -118,6 +118,9 @@ int needs_callstack;
int stack_skip;
int lock_owner;
+int use_cgroup_v2;
+int perf_subsys_id = -1;
+
/* determine the key of lock stat */
int aggr_mode;
@@ -130,6 +133,29 @@ int data_fail;
int task_map_full;
int data_map_full;
+static inline __u64 get_current_cgroup_id(void)
+{
+ struct task_struct *task;
+ struct cgroup *cgrp;
+
+ if (use_cgroup_v2)
+ return bpf_get_current_cgroup_id();
+
+ task = bpf_get_current_task_btf();
+
+ if (perf_subsys_id == -1) {
+#if __has_builtin(__builtin_preserve_enum_value)
+ perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id,
+ perf_event_cgrp_id);
+#else
+ perf_subsys_id = perf_event_cgrp_id;
+#endif
+ }
+
+ cgrp = BPF_CORE_READ(task, cgroups, subsys[perf_subsys_id], cgroup);
+ return BPF_CORE_READ(cgrp, kn, id);
+}
+
static inline int can_record(u64 *ctx)
{
if (has_cpu) {
@@ -364,10 +390,13 @@ int contention_end(u64 *ctx)
key.stack_id = pelem->stack_id;
break;
case LOCK_AGGR_ADDR:
- key.lock_addr = pelem->lock;
+ key.lock_addr_or_cgroup = pelem->lock;
if (needs_callstack)
key.stack_id = pelem->stack_id;
break;
+ case LOCK_AGGR_CGROUP:
+ key.lock_addr_or_cgroup = get_current_cgroup_id();
+ break;
default:
/* should not happen */
return 0;
diff --git a/tools/perf/util/bpf_skel/lock_data.h b/tools/perf/util/bpf_skel/lock_data.h
index 260062a9f2ab..08482daf61be 100644
--- a/tools/perf/util/bpf_skel/lock_data.h
+++ b/tools/perf/util/bpf_skel/lock_data.h
@@ -6,7 +6,7 @@
struct contention_key {
u32 stack_id;
u32 pid;
- u64 lock_addr;
+ u64 lock_addr_or_cgroup;
};
#define TASK_COMM_LEN 16
@@ -39,6 +39,7 @@ enum lock_aggr_mode {
LOCK_AGGR_ADDR = 0,
LOCK_AGGR_TASK,
LOCK_AGGR_CALLER,
+ LOCK_AGGR_CGROUP,
};
enum lock_class_sym {
diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
index 70423966d778..a073cc6a82d2 100644
--- a/tools/perf/util/lock-contention.h
+++ b/tools/perf/util/lock-contention.h
@@ -144,7 +144,6 @@ struct lock_contention {
int owner;
int nr_filtered;
bool save_callstack;
- bool use_cgroup;
};
#ifdef HAVE_BPF_SKEL
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option
2023-09-06 17:49 ` [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option Namhyung Kim
@ 2023-09-06 19:23 ` Arnaldo Carvalho de Melo
2023-09-06 19:29 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-09-06 19:23 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar,
LKML, linux-perf-users, Song Liu, Hao Luo, bpf
Em Wed, Sep 06, 2023 at 10:49:01AM -0700, Namhyung Kim escreveu:
> The -g option shows lock contention stats break down by cgroups.
> Add LOCK_AGGR_CGROUP mode and use it instead of use_cgroup field.
>
> $ sudo ./perf lock con -abg sleep 1
> contended total wait max wait avg wait cgroup
>
> 8 15.70 us 6.34 us 1.96 us /
> 2 1.48 us 747 ns 738 ns /user.slice/.../app.slice/app-gnome-google\x2dchrome-6442.scope
> 1 848 ns 848 ns 848 ns /user.slice/.../session.slice/org.gnome.Shell@x11.service
> 1 220 ns 220 ns 220 ns /user.slice/.../session.slice/pipewire-pulse.service
>
> For now, the cgroup mode only works with BPF (-b).
Can we try to be consistent with other tools?
[root@quaco ~]# perf record -h -g
Usage: perf record [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>]
-g enables call-graph recording
[root@quaco ~]# perf record -h -G
Usage: perf record [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>]
-G, --cgroup <name> monitor event in cgroup name only
[root@quaco ~]# set -o vi
[root@quaco ~]# perf lock contention -h -G
Usage: perf lock contention [<options>]
[root@quaco ~]#
I.e. use -G in this patch?
If you agree I can fixup things here, otherwise why not?
- Arnaldo
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/Documentation/perf-lock.txt | 4 ++
> tools/perf/builtin-lock.c | 40 ++++++++++++++++++-
> tools/perf/util/bpf_lock_contention.c | 16 +++++---
> .../perf/util/bpf_skel/lock_contention.bpf.c | 31 +++++++++++++-
> tools/perf/util/bpf_skel/lock_data.h | 3 +-
> tools/perf/util/lock-contention.h | 1 -
> 6 files changed, 85 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
> index 30eea576721f..61c491df72b8 100644
> --- a/tools/perf/Documentation/perf-lock.txt
> +++ b/tools/perf/Documentation/perf-lock.txt
> @@ -208,6 +208,10 @@ CONTENTION OPTIONS
> Show results using a CSV-style output to make it easy to import directly
> into spreadsheets. Columns are separated by the string specified in SEP.
>
> +-g::
> +--lock-cgroup::
> + Show lock contention stat by cgroup. Requires --use-bpf.
> +
>
> SEE ALSO
> --------
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index 06430980dfd7..b98948dd40ba 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -60,6 +60,7 @@ static bool combine_locks;
> static bool show_thread_stats;
> static bool show_lock_addrs;
> static bool show_lock_owner;
> +static bool show_lock_cgroups;
> static bool use_bpf;
> static unsigned long bpf_map_entries = MAX_ENTRIES;
> static int max_stack_depth = CONTENTION_STACK_DEPTH;
> @@ -619,6 +620,7 @@ static int get_key_by_aggr_mode_simple(u64 *key, u64 addr, u32 tid)
> *key = tid;
> break;
> case LOCK_AGGR_CALLER:
> + case LOCK_AGGR_CGROUP:
> default:
> pr_err("Invalid aggregation mode: %d\n", aggr_mode);
> return -EINVAL;
> @@ -1103,6 +1105,7 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
> if (lock_contention_caller(evsel, sample, buf, sizeof(buf)) < 0)
> name = "Unknown";
> break;
> + case LOCK_AGGR_CGROUP:
> case LOCK_AGGR_TASK:
> default:
> break;
> @@ -1653,6 +1656,9 @@ static void print_header_stdio(void)
> case LOCK_AGGR_ADDR:
> fprintf(lock_output, " %16s %s\n\n", "address", "symbol");
> break;
> + case LOCK_AGGR_CGROUP:
> + fprintf(lock_output, " %s\n\n", "cgroup");
> + break;
> default:
> break;
> }
> @@ -1680,6 +1686,9 @@ static void print_header_csv(const char *sep)
> case LOCK_AGGR_ADDR:
> fprintf(lock_output, "%s%s %s%s %s\n", "address", sep, "symbol", sep, "type");
> break;
> + case LOCK_AGGR_CGROUP:
> + fprintf(lock_output, "%s\n", "cgroup");
> + break;
> default:
> break;
> }
> @@ -1720,6 +1729,9 @@ static void print_lock_stat_stdio(struct lock_contention *con, struct lock_stat
> fprintf(lock_output, " %016llx %s (%s)\n", (unsigned long long)st->addr,
> st->name, get_type_name(st->flags));
> break;
> + case LOCK_AGGR_CGROUP:
> + fprintf(lock_output, " %s\n", st->name);
> + break;
> default:
> break;
> }
> @@ -1770,6 +1782,9 @@ static void print_lock_stat_csv(struct lock_contention *con, struct lock_stat *s
> fprintf(lock_output, "%llx%s %s%s %s\n", (unsigned long long)st->addr, sep,
> st->name, sep, get_type_name(st->flags));
> break;
> + case LOCK_AGGR_CGROUP:
> + fprintf(lock_output, "%s\n",st->name);
> + break;
> default:
> break;
> }
> @@ -1999,6 +2014,27 @@ static int check_lock_contention_options(const struct option *options,
> return -1;
> }
>
> + if (show_lock_cgroups && !use_bpf) {
> + pr_err("Cgroups are available only with BPF\n");
> + parse_options_usage(usage, options, "lock-cgroup", 0);
> + parse_options_usage(NULL, options, "use-bpf", 0);
> + return -1;
> + }
> +
> + if (show_lock_cgroups && show_lock_addrs) {
> + pr_err("Cannot use cgroup and addr mode together\n");
> + parse_options_usage(usage, options, "lock-cgroup", 0);
> + parse_options_usage(NULL, options, "lock-addr", 0);
> + return -1;
> + }
> +
> + if (show_lock_cgroups && show_thread_stats) {
> + pr_err("Cannot use cgroup and thread mode together\n");
> + parse_options_usage(usage, options, "lock-cgroup", 0);
> + parse_options_usage(NULL, options, "threads", 0);
> + return -1;
> + }
> +
> if (symbol_conf.field_sep) {
> if (strstr(symbol_conf.field_sep, ":") || /* part of type flags */
> strstr(symbol_conf.field_sep, "+") || /* part of caller offset */
> @@ -2060,7 +2096,8 @@ static int __cmd_contention(int argc, const char **argv)
> con.machine = &session->machines.host;
>
> con.aggr_mode = aggr_mode = show_thread_stats ? LOCK_AGGR_TASK :
> - show_lock_addrs ? LOCK_AGGR_ADDR : LOCK_AGGR_CALLER;
> + show_lock_addrs ? LOCK_AGGR_ADDR :
> + show_lock_cgroups ? LOCK_AGGR_CGROUP : LOCK_AGGR_CALLER;
>
> if (con.aggr_mode == LOCK_AGGR_CALLER)
> con.save_callstack = true;
> @@ -2524,6 +2561,7 @@ int cmd_lock(int argc, const char **argv)
> OPT_BOOLEAN('o', "lock-owner", &show_lock_owner, "show lock owners instead of waiters"),
> OPT_STRING_NOEMPTY('x', "field-separator", &symbol_conf.field_sep, "separator",
> "print result in CSV format with custom separator"),
> + OPT_BOOLEAN('g', "lock-cgroup", &show_lock_cgroups, "show lock stats by cgroup"),
> OPT_PARENT(lock_options)
> };
>
> diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
> index c6bd7c9b2d57..42753a0dfdc5 100644
> --- a/tools/perf/util/bpf_lock_contention.c
> +++ b/tools/perf/util/bpf_lock_contention.c
> @@ -152,7 +152,10 @@ int lock_contention_prepare(struct lock_contention *con)
> skel->bss->needs_callstack = con->save_callstack;
> skel->bss->lock_owner = con->owner;
>
> - if (con->use_cgroup) {
> + if (con->aggr_mode == LOCK_AGGR_CGROUP) {
> + if (cgroup_is_v2("perf_event"))
> + skel->bss->use_cgroup_v2 = 1;
> +
> read_all_cgroups(&con->cgroups);
> }
>
> @@ -214,12 +217,12 @@ static const char *lock_contention_get_name(struct lock_contention *con,
> return "siglock";
>
> /* global locks with symbols */
> - sym = machine__find_kernel_symbol(machine, key->lock_addr, &kmap);
> + sym = machine__find_kernel_symbol(machine, key->lock_addr_or_cgroup, &kmap);
> if (sym)
> return sym->name;
>
> /* try semi-global locks collected separately */
> - if (!bpf_map_lookup_elem(lock_fd, &key->lock_addr, &flags)) {
> + if (!bpf_map_lookup_elem(lock_fd, &key->lock_addr_or_cgroup, &flags)) {
> if (flags == LOCK_CLASS_RQLOCK)
> return "rq_lock";
> }
> @@ -227,8 +230,8 @@ static const char *lock_contention_get_name(struct lock_contention *con,
> return "";
> }
>
> - if (con->use_cgroup) {
> - u64 cgrp_id = key->lock_addr;
> + if (con->aggr_mode == LOCK_AGGR_CGROUP) {
> + u64 cgrp_id = key->lock_addr_or_cgroup;
> struct cgroup *cgrp = __cgroup__find(&con->cgroups, cgrp_id);
>
> if (cgrp)
> @@ -329,7 +332,8 @@ int lock_contention_read(struct lock_contention *con)
> ls_key = key.pid;
> break;
> case LOCK_AGGR_ADDR:
> - ls_key = key.lock_addr;
> + case LOCK_AGGR_CGROUP:
> + ls_key = key.lock_addr_or_cgroup;
> break;
> default:
> goto next;
> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index 8d3cfbb3cc65..823354999022 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> @@ -118,6 +118,9 @@ int needs_callstack;
> int stack_skip;
> int lock_owner;
>
> +int use_cgroup_v2;
> +int perf_subsys_id = -1;
> +
> /* determine the key of lock stat */
> int aggr_mode;
>
> @@ -130,6 +133,29 @@ int data_fail;
> int task_map_full;
> int data_map_full;
>
> +static inline __u64 get_current_cgroup_id(void)
> +{
> + struct task_struct *task;
> + struct cgroup *cgrp;
> +
> + if (use_cgroup_v2)
> + return bpf_get_current_cgroup_id();
> +
> + task = bpf_get_current_task_btf();
> +
> + if (perf_subsys_id == -1) {
> +#if __has_builtin(__builtin_preserve_enum_value)
> + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id,
> + perf_event_cgrp_id);
> +#else
> + perf_subsys_id = perf_event_cgrp_id;
> +#endif
> + }
> +
> + cgrp = BPF_CORE_READ(task, cgroups, subsys[perf_subsys_id], cgroup);
> + return BPF_CORE_READ(cgrp, kn, id);
> +}
> +
> static inline int can_record(u64 *ctx)
> {
> if (has_cpu) {
> @@ -364,10 +390,13 @@ int contention_end(u64 *ctx)
> key.stack_id = pelem->stack_id;
> break;
> case LOCK_AGGR_ADDR:
> - key.lock_addr = pelem->lock;
> + key.lock_addr_or_cgroup = pelem->lock;
> if (needs_callstack)
> key.stack_id = pelem->stack_id;
> break;
> + case LOCK_AGGR_CGROUP:
> + key.lock_addr_or_cgroup = get_current_cgroup_id();
> + break;
> default:
> /* should not happen */
> return 0;
> diff --git a/tools/perf/util/bpf_skel/lock_data.h b/tools/perf/util/bpf_skel/lock_data.h
> index 260062a9f2ab..08482daf61be 100644
> --- a/tools/perf/util/bpf_skel/lock_data.h
> +++ b/tools/perf/util/bpf_skel/lock_data.h
> @@ -6,7 +6,7 @@
> struct contention_key {
> u32 stack_id;
> u32 pid;
> - u64 lock_addr;
> + u64 lock_addr_or_cgroup;
> };
>
> #define TASK_COMM_LEN 16
> @@ -39,6 +39,7 @@ enum lock_aggr_mode {
> LOCK_AGGR_ADDR = 0,
> LOCK_AGGR_TASK,
> LOCK_AGGR_CALLER,
> + LOCK_AGGR_CGROUP,
> };
>
> enum lock_class_sym {
> diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
> index 70423966d778..a073cc6a82d2 100644
> --- a/tools/perf/util/lock-contention.h
> +++ b/tools/perf/util/lock-contention.h
> @@ -144,7 +144,6 @@ struct lock_contention {
> int owner;
> int nr_filtered;
> bool save_callstack;
> - bool use_cgroup;
> };
>
> #ifdef HAVE_BPF_SKEL
> --
> 2.42.0.283.g2d96d420d3-goog
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option
2023-09-06 19:23 ` Arnaldo Carvalho de Melo
@ 2023-09-06 19:29 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-09-06 19:29 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar,
LKML, linux-perf-users, Song Liu, Hao Luo, bpf
Em Wed, Sep 06, 2023 at 04:23:17PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Sep 06, 2023 at 10:49:01AM -0700, Namhyung Kim escreveu:
> > The -g option shows lock contention stats break down by cgroups.
> > Add LOCK_AGGR_CGROUP mode and use it instead of use_cgroup field.
> >
> > $ sudo ./perf lock con -abg sleep 1
> > contended total wait max wait avg wait cgroup
> >
> > 8 15.70 us 6.34 us 1.96 us /
> > 2 1.48 us 747 ns 738 ns /user.slice/.../app.slice/app-gnome-google\x2dchrome-6442.scope
> > 1 848 ns 848 ns 848 ns /user.slice/.../session.slice/org.gnome.Shell@x11.service
> > 1 220 ns 220 ns 220 ns /user.slice/.../session.slice/pipewire-pulse.service
> >
> > For now, the cgroup mode only works with BPF (-b).
>
> Can we try to be consistent with other tools?
>
> [root@quaco ~]# perf record -h -g
>
> Usage: perf record [<options>] [<command>]
> or: perf record [<options>] -- <command> [<options>]
>
> -g enables call-graph recording
>
> [root@quaco ~]# perf record -h -G
>
> Usage: perf record [<options>] [<command>]
> or: perf record [<options>] -- <command> [<options>]
>
> -G, --cgroup <name> monitor event in cgroup name only
>
> [root@quaco ~]# set -o vi
> [root@quaco ~]# perf lock contention -h -G
>
> Usage: perf lock contention [<options>]
>
>
> [root@quaco ~]#
>
> I.e. use -G in this patch?
>
> If you agree I can fixup things here, otherwise why not?
I see that you use -G in 4/5, unsure now, but this looks like --sort in
'perf report', no?
- Arnaldo
> - Arnaldo
>
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/Documentation/perf-lock.txt | 4 ++
> > tools/perf/builtin-lock.c | 40 ++++++++++++++++++-
> > tools/perf/util/bpf_lock_contention.c | 16 +++++---
> > .../perf/util/bpf_skel/lock_contention.bpf.c | 31 +++++++++++++-
> > tools/perf/util/bpf_skel/lock_data.h | 3 +-
> > tools/perf/util/lock-contention.h | 1 -
> > 6 files changed, 85 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
> > index 30eea576721f..61c491df72b8 100644
> > --- a/tools/perf/Documentation/perf-lock.txt
> > +++ b/tools/perf/Documentation/perf-lock.txt
> > @@ -208,6 +208,10 @@ CONTENTION OPTIONS
> > Show results using a CSV-style output to make it easy to import directly
> > into spreadsheets. Columns are separated by the string specified in SEP.
> >
> > +-g::
> > +--lock-cgroup::
> > + Show lock contention stat by cgroup. Requires --use-bpf.
> > +
> >
> > SEE ALSO
> > --------
> > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> > index 06430980dfd7..b98948dd40ba 100644
> > --- a/tools/perf/builtin-lock.c
> > +++ b/tools/perf/builtin-lock.c
> > @@ -60,6 +60,7 @@ static bool combine_locks;
> > static bool show_thread_stats;
> > static bool show_lock_addrs;
> > static bool show_lock_owner;
> > +static bool show_lock_cgroups;
> > static bool use_bpf;
> > static unsigned long bpf_map_entries = MAX_ENTRIES;
> > static int max_stack_depth = CONTENTION_STACK_DEPTH;
> > @@ -619,6 +620,7 @@ static int get_key_by_aggr_mode_simple(u64 *key, u64 addr, u32 tid)
> > *key = tid;
> > break;
> > case LOCK_AGGR_CALLER:
> > + case LOCK_AGGR_CGROUP:
> > default:
> > pr_err("Invalid aggregation mode: %d\n", aggr_mode);
> > return -EINVAL;
> > @@ -1103,6 +1105,7 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
> > if (lock_contention_caller(evsel, sample, buf, sizeof(buf)) < 0)
> > name = "Unknown";
> > break;
> > + case LOCK_AGGR_CGROUP:
> > case LOCK_AGGR_TASK:
> > default:
> > break;
> > @@ -1653,6 +1656,9 @@ static void print_header_stdio(void)
> > case LOCK_AGGR_ADDR:
> > fprintf(lock_output, " %16s %s\n\n", "address", "symbol");
> > break;
> > + case LOCK_AGGR_CGROUP:
> > + fprintf(lock_output, " %s\n\n", "cgroup");
> > + break;
> > default:
> > break;
> > }
> > @@ -1680,6 +1686,9 @@ static void print_header_csv(const char *sep)
> > case LOCK_AGGR_ADDR:
> > fprintf(lock_output, "%s%s %s%s %s\n", "address", sep, "symbol", sep, "type");
> > break;
> > + case LOCK_AGGR_CGROUP:
> > + fprintf(lock_output, "%s\n", "cgroup");
> > + break;
> > default:
> > break;
> > }
> > @@ -1720,6 +1729,9 @@ static void print_lock_stat_stdio(struct lock_contention *con, struct lock_stat
> > fprintf(lock_output, " %016llx %s (%s)\n", (unsigned long long)st->addr,
> > st->name, get_type_name(st->flags));
> > break;
> > + case LOCK_AGGR_CGROUP:
> > + fprintf(lock_output, " %s\n", st->name);
> > + break;
> > default:
> > break;
> > }
> > @@ -1770,6 +1782,9 @@ static void print_lock_stat_csv(struct lock_contention *con, struct lock_stat *s
> > fprintf(lock_output, "%llx%s %s%s %s\n", (unsigned long long)st->addr, sep,
> > st->name, sep, get_type_name(st->flags));
> > break;
> > + case LOCK_AGGR_CGROUP:
> > + fprintf(lock_output, "%s\n",st->name);
> > + break;
> > default:
> > break;
> > }
> > @@ -1999,6 +2014,27 @@ static int check_lock_contention_options(const struct option *options,
> > return -1;
> > }
> >
> > + if (show_lock_cgroups && !use_bpf) {
> > + pr_err("Cgroups are available only with BPF\n");
> > + parse_options_usage(usage, options, "lock-cgroup", 0);
> > + parse_options_usage(NULL, options, "use-bpf", 0);
> > + return -1;
> > + }
> > +
> > + if (show_lock_cgroups && show_lock_addrs) {
> > + pr_err("Cannot use cgroup and addr mode together\n");
> > + parse_options_usage(usage, options, "lock-cgroup", 0);
> > + parse_options_usage(NULL, options, "lock-addr", 0);
> > + return -1;
> > + }
> > +
> > + if (show_lock_cgroups && show_thread_stats) {
> > + pr_err("Cannot use cgroup and thread mode together\n");
> > + parse_options_usage(usage, options, "lock-cgroup", 0);
> > + parse_options_usage(NULL, options, "threads", 0);
> > + return -1;
> > + }
> > +
> > if (symbol_conf.field_sep) {
> > if (strstr(symbol_conf.field_sep, ":") || /* part of type flags */
> > strstr(symbol_conf.field_sep, "+") || /* part of caller offset */
> > @@ -2060,7 +2096,8 @@ static int __cmd_contention(int argc, const char **argv)
> > con.machine = &session->machines.host;
> >
> > con.aggr_mode = aggr_mode = show_thread_stats ? LOCK_AGGR_TASK :
> > - show_lock_addrs ? LOCK_AGGR_ADDR : LOCK_AGGR_CALLER;
> > + show_lock_addrs ? LOCK_AGGR_ADDR :
> > + show_lock_cgroups ? LOCK_AGGR_CGROUP : LOCK_AGGR_CALLER;
> >
> > if (con.aggr_mode == LOCK_AGGR_CALLER)
> > con.save_callstack = true;
> > @@ -2524,6 +2561,7 @@ int cmd_lock(int argc, const char **argv)
> > OPT_BOOLEAN('o', "lock-owner", &show_lock_owner, "show lock owners instead of waiters"),
> > OPT_STRING_NOEMPTY('x', "field-separator", &symbol_conf.field_sep, "separator",
> > "print result in CSV format with custom separator"),
> > + OPT_BOOLEAN('g', "lock-cgroup", &show_lock_cgroups, "show lock stats by cgroup"),
> > OPT_PARENT(lock_options)
> > };
> >
> > diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
> > index c6bd7c9b2d57..42753a0dfdc5 100644
> > --- a/tools/perf/util/bpf_lock_contention.c
> > +++ b/tools/perf/util/bpf_lock_contention.c
> > @@ -152,7 +152,10 @@ int lock_contention_prepare(struct lock_contention *con)
> > skel->bss->needs_callstack = con->save_callstack;
> > skel->bss->lock_owner = con->owner;
> >
> > - if (con->use_cgroup) {
> > + if (con->aggr_mode == LOCK_AGGR_CGROUP) {
> > + if (cgroup_is_v2("perf_event"))
> > + skel->bss->use_cgroup_v2 = 1;
> > +
> > read_all_cgroups(&con->cgroups);
> > }
> >
> > @@ -214,12 +217,12 @@ static const char *lock_contention_get_name(struct lock_contention *con,
> > return "siglock";
> >
> > /* global locks with symbols */
> > - sym = machine__find_kernel_symbol(machine, key->lock_addr, &kmap);
> > + sym = machine__find_kernel_symbol(machine, key->lock_addr_or_cgroup, &kmap);
> > if (sym)
> > return sym->name;
> >
> > /* try semi-global locks collected separately */
> > - if (!bpf_map_lookup_elem(lock_fd, &key->lock_addr, &flags)) {
> > + if (!bpf_map_lookup_elem(lock_fd, &key->lock_addr_or_cgroup, &flags)) {
> > if (flags == LOCK_CLASS_RQLOCK)
> > return "rq_lock";
> > }
> > @@ -227,8 +230,8 @@ static const char *lock_contention_get_name(struct lock_contention *con,
> > return "";
> > }
> >
> > - if (con->use_cgroup) {
> > - u64 cgrp_id = key->lock_addr;
> > + if (con->aggr_mode == LOCK_AGGR_CGROUP) {
> > + u64 cgrp_id = key->lock_addr_or_cgroup;
> > struct cgroup *cgrp = __cgroup__find(&con->cgroups, cgrp_id);
> >
> > if (cgrp)
> > @@ -329,7 +332,8 @@ int lock_contention_read(struct lock_contention *con)
> > ls_key = key.pid;
> > break;
> > case LOCK_AGGR_ADDR:
> > - ls_key = key.lock_addr;
> > + case LOCK_AGGR_CGROUP:
> > + ls_key = key.lock_addr_or_cgroup;
> > break;
> > default:
> > goto next;
> > diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > index 8d3cfbb3cc65..823354999022 100644
> > --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > @@ -118,6 +118,9 @@ int needs_callstack;
> > int stack_skip;
> > int lock_owner;
> >
> > +int use_cgroup_v2;
> > +int perf_subsys_id = -1;
> > +
> > /* determine the key of lock stat */
> > int aggr_mode;
> >
> > @@ -130,6 +133,29 @@ int data_fail;
> > int task_map_full;
> > int data_map_full;
> >
> > +static inline __u64 get_current_cgroup_id(void)
> > +{
> > + struct task_struct *task;
> > + struct cgroup *cgrp;
> > +
> > + if (use_cgroup_v2)
> > + return bpf_get_current_cgroup_id();
> > +
> > + task = bpf_get_current_task_btf();
> > +
> > + if (perf_subsys_id == -1) {
> > +#if __has_builtin(__builtin_preserve_enum_value)
> > + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id,
> > + perf_event_cgrp_id);
> > +#else
> > + perf_subsys_id = perf_event_cgrp_id;
> > +#endif
> > + }
> > +
> > + cgrp = BPF_CORE_READ(task, cgroups, subsys[perf_subsys_id], cgroup);
> > + return BPF_CORE_READ(cgrp, kn, id);
> > +}
> > +
> > static inline int can_record(u64 *ctx)
> > {
> > if (has_cpu) {
> > @@ -364,10 +390,13 @@ int contention_end(u64 *ctx)
> > key.stack_id = pelem->stack_id;
> > break;
> > case LOCK_AGGR_ADDR:
> > - key.lock_addr = pelem->lock;
> > + key.lock_addr_or_cgroup = pelem->lock;
> > if (needs_callstack)
> > key.stack_id = pelem->stack_id;
> > break;
> > + case LOCK_AGGR_CGROUP:
> > + key.lock_addr_or_cgroup = get_current_cgroup_id();
> > + break;
> > default:
> > /* should not happen */
> > return 0;
> > diff --git a/tools/perf/util/bpf_skel/lock_data.h b/tools/perf/util/bpf_skel/lock_data.h
> > index 260062a9f2ab..08482daf61be 100644
> > --- a/tools/perf/util/bpf_skel/lock_data.h
> > +++ b/tools/perf/util/bpf_skel/lock_data.h
> > @@ -6,7 +6,7 @@
> > struct contention_key {
> > u32 stack_id;
> > u32 pid;
> > - u64 lock_addr;
> > + u64 lock_addr_or_cgroup;
> > };
> >
> > #define TASK_COMM_LEN 16
> > @@ -39,6 +39,7 @@ enum lock_aggr_mode {
> > LOCK_AGGR_ADDR = 0,
> > LOCK_AGGR_TASK,
> > LOCK_AGGR_CALLER,
> > + LOCK_AGGR_CGROUP,
> > };
> >
> > enum lock_class_sym {
> > diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
> > index 70423966d778..a073cc6a82d2 100644
> > --- a/tools/perf/util/lock-contention.h
> > +++ b/tools/perf/util/lock-contention.h
> > @@ -144,7 +144,6 @@ struct lock_contention {
> > int owner;
> > int nr_filtered;
> > bool save_callstack;
> > - bool use_cgroup;
> > };
> >
> > #ifdef HAVE_BPF_SKEL
> > --
> > 2.42.0.283.g2d96d420d3-goog
> >
>
> --
>
> - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/5] perf lock contention: Add -G/--cgroup-filter option
2023-09-06 17:48 [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Namhyung Kim
` (2 preceding siblings ...)
2023-09-06 17:49 ` [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option Namhyung Kim
@ 2023-09-06 17:49 ` Namhyung Kim
2023-09-06 17:49 ` [PATCH 5/5] perf test: Improve perf lock contention test Namhyung Kim
2023-09-06 21:28 ` [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Arnaldo Carvalho de Melo
5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2023-09-06 17:49 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Song Liu, Hao Luo, bpf
The -G/--cgroup-filter is to limit lock contention collection on the
tasks in the specific cgroups only.
$ sudo ./perf lock con -abt -G /user.slice/.../vte-spawn-52221fb8-b33f-4a52-b5c3-e35d1e6fc0e0.scope \
./perf bench sched messaging
# Running 'sched/messaging' benchmark:
# 20 sender and receiver processes per group
# 10 groups == 400 processes run
Total time: 0.174 [sec]
contended total wait max wait avg wait pid comm
4 114.45 us 60.06 us 28.61 us 214847 sched-messaging
2 111.40 us 60.84 us 55.70 us 214848 sched-messaging
2 106.09 us 59.42 us 53.04 us 214837 sched-messaging
1 81.70 us 81.70 us 81.70 us 214709 sched-messaging
68 78.44 us 6.83 us 1.15 us 214633 sched-messaging
69 73.71 us 2.69 us 1.07 us 214632 sched-messaging
4 72.62 us 60.83 us 18.15 us 214850 sched-messaging
2 71.75 us 67.60 us 35.88 us 214840 sched-messaging
2 69.29 us 67.53 us 34.65 us 214804 sched-messaging
2 69.00 us 68.23 us 34.50 us 214826 sched-messaging
...
Export cgroup__new() function as it's needed from outside.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-lock.txt | 4 ++
tools/perf/builtin-lock.c | 56 +++++++++++++++++++
tools/perf/util/bpf_lock_contention.c | 15 ++++-
.../perf/util/bpf_skel/lock_contention.bpf.c | 17 ++++++
tools/perf/util/cgroup.c | 2 +-
tools/perf/util/cgroup.h | 1 +
tools/perf/util/lock-contention.h | 2 +
7 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
index 61c491df72b8..0897443948b7 100644
--- a/tools/perf/Documentation/perf-lock.txt
+++ b/tools/perf/Documentation/perf-lock.txt
@@ -212,6 +212,10 @@ CONTENTION OPTIONS
--lock-cgroup::
Show lock contention stat by cgroup. Requires --use-bpf.
+-G::
+--cgroup-filter=<value>::
+ Show lock contention only in the given cgroups (comma separated list).
+
SEE ALSO
--------
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index b98948dd40ba..3902780d5229 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -10,6 +10,7 @@
#include "util/thread.h"
#include "util/header.h"
#include "util/target.h"
+#include "util/cgroup.h"
#include "util/callchain.h"
#include "util/lock-contention.h"
#include "util/bpf_skel/lock_data.h"
@@ -1631,6 +1632,9 @@ static void lock_filter_finish(void)
zfree(&filters.syms);
filters.nr_syms = 0;
+
+ zfree(&filters.cgrps);
+ filters.nr_cgrps = 0;
}
static void sort_contention_result(void)
@@ -2488,6 +2492,56 @@ static int parse_output(const struct option *opt __maybe_unused, const char *str
return 0;
}
+static bool add_lock_cgroup(char *name)
+{
+ u64 *tmp;
+ struct cgroup *cgrp;
+
+ cgrp = cgroup__new(name, /*do_open=*/false);
+ if (cgrp == NULL) {
+ pr_err("Failed to create cgroup: %s\n", name);
+ return false;
+ }
+
+ if (read_cgroup_id(cgrp) < 0) {
+ pr_err("Failed to read cgroup id for %s\n", name);
+ cgroup__put(cgrp);
+ return false;
+ }
+
+ tmp = realloc(filters.cgrps, (filters.nr_cgrps + 1) * sizeof(*filters.cgrps));
+ if (tmp == NULL) {
+ pr_err("Memory allocation failure\n");
+ return false;
+ }
+
+ tmp[filters.nr_cgrps++] = cgrp->id;
+ filters.cgrps = tmp;
+ cgroup__put(cgrp);
+ return true;
+}
+
+static int parse_cgroup_filter(const struct option *opt __maybe_unused, const char *str,
+ int unset __maybe_unused)
+{
+ char *s, *tmp, *tok;
+ int ret = 0;
+
+ s = strdup(str);
+ if (s == NULL)
+ return -1;
+
+ for (tok = strtok_r(s, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) {
+ if (!add_lock_cgroup(tok)) {
+ ret = -1;
+ break;
+ }
+ }
+
+ free(s);
+ return ret;
+}
+
int cmd_lock(int argc, const char **argv)
{
const struct option lock_options[] = {
@@ -2562,6 +2616,8 @@ int cmd_lock(int argc, const char **argv)
OPT_STRING_NOEMPTY('x', "field-separator", &symbol_conf.field_sep, "separator",
"print result in CSV format with custom separator"),
OPT_BOOLEAN('g', "lock-cgroup", &show_lock_cgroups, "show lock stats by cgroup"),
+ OPT_CALLBACK('G', "cgroup-filter", NULL, "CGROUPS",
+ "Filter specific cgroups", parse_cgroup_filter),
OPT_PARENT(lock_options)
};
diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
index 42753a0dfdc5..e105245eb905 100644
--- a/tools/perf/util/bpf_lock_contention.c
+++ b/tools/perf/util/bpf_lock_contention.c
@@ -21,7 +21,7 @@ static struct lock_contention_bpf *skel;
int lock_contention_prepare(struct lock_contention *con)
{
int i, fd;
- int ncpus = 1, ntasks = 1, ntypes = 1, naddrs = 1;
+ int ncpus = 1, ntasks = 1, ntypes = 1, naddrs = 1, ncgrps = 1;
struct evlist *evlist = con->evlist;
struct target *target = con->target;
@@ -51,6 +51,8 @@ int lock_contention_prepare(struct lock_contention *con)
ntasks = perf_thread_map__nr(evlist->core.threads);
if (con->filters->nr_types)
ntypes = con->filters->nr_types;
+ if (con->filters->nr_cgrps)
+ ncgrps = con->filters->nr_cgrps;
/* resolve lock name filters to addr */
if (con->filters->nr_syms) {
@@ -85,6 +87,7 @@ int lock_contention_prepare(struct lock_contention *con)
bpf_map__set_max_entries(skel->maps.task_filter, ntasks);
bpf_map__set_max_entries(skel->maps.type_filter, ntypes);
bpf_map__set_max_entries(skel->maps.addr_filter, naddrs);
+ bpf_map__set_max_entries(skel->maps.cgroup_filter, ncgrps);
if (lock_contention_bpf__load(skel) < 0) {
pr_err("Failed to load lock-contention BPF skeleton\n");
@@ -146,6 +149,16 @@ int lock_contention_prepare(struct lock_contention *con)
bpf_map_update_elem(fd, &con->filters->addrs[i], &val, BPF_ANY);
}
+ if (con->filters->nr_cgrps) {
+ u8 val = 1;
+
+ skel->bss->has_cgroup = 1;
+ fd = bpf_map__fd(skel->maps.cgroup_filter);
+
+ for (i = 0; i < con->filters->nr_cgrps; i++)
+ bpf_map_update_elem(fd, &con->filters->cgrps[i], &val, BPF_ANY);
+ }
+
/* these don't work well if in the rodata section */
skel->bss->stack_skip = con->stack_skip;
skel->bss->aggr_mode = con->aggr_mode;
diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
index 823354999022..4900a5dfb4a4 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -92,6 +92,13 @@ struct {
__uint(max_entries, 1);
} addr_filter SEC(".maps");
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(key_size, sizeof(__u64));
+ __uint(value_size, sizeof(__u8));
+ __uint(max_entries, 1);
+} cgroup_filter SEC(".maps");
+
struct rw_semaphore___old {
struct task_struct *owner;
} __attribute__((preserve_access_index));
@@ -114,6 +121,7 @@ int has_cpu;
int has_task;
int has_type;
int has_addr;
+int has_cgroup;
int needs_callstack;
int stack_skip;
int lock_owner;
@@ -194,6 +202,15 @@ static inline int can_record(u64 *ctx)
return 0;
}
+ if (has_cgroup) {
+ __u8 *ok;
+ __u64 cgrp = get_current_cgroup_id();
+
+ ok = bpf_map_lookup_elem(&cgroup_filter, &cgrp);
+ if (!ok)
+ return 0;
+ }
+
return 1;
}
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index 2e969d1464f4..b8499da2ef48 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -114,7 +114,7 @@ static struct cgroup *evlist__find_cgroup(struct evlist *evlist, const char *str
return NULL;
}
-static struct cgroup *cgroup__new(const char *name, bool do_open)
+struct cgroup *cgroup__new(const char *name, bool do_open)
{
struct cgroup *cgroup = zalloc(sizeof(*cgroup));
diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
index beb6fe1012ed..de8882d6e8d3 100644
--- a/tools/perf/util/cgroup.h
+++ b/tools/perf/util/cgroup.h
@@ -26,6 +26,7 @@ void cgroup__put(struct cgroup *cgroup);
struct evlist;
struct rblist;
+struct cgroup *cgroup__new(const char *name, bool do_open);
struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups,
struct rblist *metric_events, bool open_cgroup);
diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
index a073cc6a82d2..1a7248ff3889 100644
--- a/tools/perf/util/lock-contention.h
+++ b/tools/perf/util/lock-contention.h
@@ -9,9 +9,11 @@ struct lock_filter {
int nr_types;
int nr_addrs;
int nr_syms;
+ int nr_cgrps;
unsigned int *types;
unsigned long *addrs;
char **syms;
+ u64 *cgrps;
};
struct lock_stat {
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/5] perf test: Improve perf lock contention test
2023-09-06 17:48 [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Namhyung Kim
` (3 preceding siblings ...)
2023-09-06 17:49 ` [PATCH 4/5] perf lock contention: Add -G/--cgroup-filter option Namhyung Kim
@ 2023-09-06 17:49 ` Namhyung Kim
2023-09-06 21:28 ` [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Arnaldo Carvalho de Melo
5 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2023-09-06 17:49 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Song Liu, Hao Luo, bpf
Add cgroup aggregation and filter tests.
$ sudo ./perf test -v contention
84: kernel lock contention analysis test :
--- start ---
test child forked, pid 222423
Testing perf lock record and perf lock contention
Testing perf lock contention --use-bpf
Testing perf lock record and perf lock contention at the same time
Testing perf lock contention --threads
Testing perf lock contention --lock-addr
Testing perf lock contention --lock-cgroup
Testing perf lock contention --type-filter (w/ spinlock)
Testing perf lock contention --lock-filter (w/ tasklist_lock)
Testing perf lock contention --callstack-filter (w/ unix_stream)
Testing perf lock contention --callstack-filter with task aggregation
Testing perf lock contention --cgroup-filter
Testing perf lock contention CSV output
test child finished with 0
---- end ----
kernel lock contention analysis test: Ok
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/tests/shell/lock_contention.sh | 45 +++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index d120e83db7d9..966e67db75f3 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -123,6 +123,24 @@ test_aggr_addr()
fi
}
+test_aggr_cgroup()
+{
+ echo "Testing perf lock contention --lock-cgroup"
+
+ if ! perf lock con -b true > /dev/null 2>&1 ; then
+ echo "[Skip] No BPF support"
+ return
+ fi
+
+ # the perf lock contention output goes to the stderr
+ perf lock con -a -b -g -E 1 -q -- perf bench sched messaging > /dev/null 2> ${result}
+ if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+ echo "[Fail] BPF result count is not 1:" "$(cat "${result}" | wc -l)"
+ err=1
+ exit
+ fi
+}
+
test_type_filter()
{
echo "Testing perf lock contention --type-filter (w/ spinlock)"
@@ -232,6 +250,31 @@ test_aggr_task_stack_filter()
exit
fi
}
+test_cgroup_filter()
+{
+ echo "Testing perf lock contention --cgroup-filter"
+
+ if ! perf lock con -b true > /dev/null 2>&1 ; then
+ echo "[Skip] No BPF support"
+ return
+ fi
+
+ perf lock con -a -b -g -E 1 -F wait_total -q -- perf bench sched messaging > /dev/null 2> ${result}
+ if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+ echo "[Fail] BPF result should have a cgroup result:" "$(cat "${result}")"
+ err=1
+ exit
+ fi
+
+ cgroup=$(cat "${result}" | awk '{ print $3 }')
+ perf lock con -a -b -g -E 1 -G "${cgroup}" -q -- perf bench sched messaging > /dev/null 2> ${result}
+ if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+ echo "[Fail] BPF result should have a result with cgroup filter:" "$(cat "${cgroup}")"
+ err=1
+ exit
+ fi
+}
+
test_csv_output()
{
@@ -275,10 +318,12 @@ test_bpf
test_record_concurrent
test_aggr_task
test_aggr_addr
+test_aggr_cgroup
test_type_filter
test_lock_filter
test_stack_filter
test_aggr_task_stack_filter
+test_cgroup_filter
test_csv_output
exit ${err}
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCHSET 0/5] perf lock contention: Add cgroup support (v2)
2023-09-06 17:48 [PATCHSET 0/5] perf lock contention: Add cgroup support (v2) Namhyung Kim
` (4 preceding siblings ...)
2023-09-06 17:49 ` [PATCH 5/5] perf test: Improve perf lock contention test Namhyung Kim
@ 2023-09-06 21:28 ` Arnaldo Carvalho de Melo
5 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-09-06 21:28 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar,
LKML, linux-perf-users, Song Liu, Hao Luo, bpf
Em Wed, Sep 06, 2023 at 10:48:58AM -0700, Namhyung Kim escreveu:
> Hello,
>
> The cgroup support comes with two flavors. One is to aggregate the
> result by cgroups and the other is to filter result for the given
> cgroups. For now, it only works in BPF mode.
>
> * v2 changes
> - fix compiler errors (Arnaldo)
> - add Reviewed-by from Ian
Thanks, applied, tested, pushing to tmp.perf-tools-next.
- Arnaldo
^ permalink raw reply [flat|nested] 9+ messages in thread