linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option
  2023-08-30 23:01 [PATCHSET 0/5] perf lock contention: Add cgroup support (v1) Namhyung Kim
@ 2023-08-30 23:01 ` Namhyung Kim
  0 siblings, 0 replies; 10+ messages in thread
From: Namhyung Kim @ 2023-08-30 23:01 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).

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-lock.txt        |  4 ++
 tools/perf/builtin-lock.c                     | 42 ++++++++++++++++++-
 tools/perf/util/bpf_lock_contention.c         | 14 ++++---
 .../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 69086d94f588..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;
@@ -2159,7 +2196,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;
@@ -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 9d36b07eccf5..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,7 +230,7 @@ static const char *lock_contention_get_name(struct lock_contention *con,
 		return "";
 	}
 
-	if (con->use_cgroup) {
+	if (con->aggr_mode == LOCK_AGGR_CGROUP) {
 		u64 cgrp_id = key->lock_addr_or_cgroup;
 		struct cgroup *cgrp = __cgroup__find(&con->cgroups, cgrp_id);
 
@@ -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] 10+ messages in thread

* [PATCHSET 0/5] perf lock contention: Add cgroup support (v2)
@ 2023-09-06 17:48 Namhyung Kim
  2023-09-06 17:48 ` [PATCH 1/5] perf tools: Add read_all_cgroups() and __cgroup_find() Namhyung Kim
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ 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

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
 
The first one is -g/--lock-cgroup option to show lock stats by cgroups
like below.  The cgroup names were shortened for brevity:

  $ sudo perf lock con -abg perf bench sched messaging
   contended   total wait     max wait     avg wait   cgroup

        1052      3.34 ms     84.71 us      3.17 us   /app-org.gnome.Terminal.slice/vte-spawn-52221fb8-b33f-4a52-b5c3-e35d1e6fc0e0.scope
          13    106.60 us     11.48 us      8.20 us   /session.slice/org.gnome.Shell@x11.service
          12     21.20 us      4.93 us      1.77 us   /
           3     12.10 us      8.80 us      4.03 us   /session-4.scope
           2     10.98 us      7.50 us      5.49 us   /app-gnome-firefox\x2desr-34054.scope
           2      6.04 us      4.88 us      3.02 us   /app-gnome-google\x2dchrome-6442.scope
           1      5.63 us      5.63 us      5.63 us   /app-org.gnome.Terminal.slice/gnome-terminal-server.service
           1      3.51 us      3.51 us      3.51 us   /pipewire.service
           1      2.15 us      2.15 us      2.15 us   /pipewire-pulse.service
           1       742 ns       742 ns       742 ns   /dbus.service

The other is -G/--cgroup-filter option to show lock stats only from the
given cgroups.  It doesn't support cgroup hierarchy and regex matching.

  $ sudo perf lock con -abt -G / perf bench sched messaging
   contended   total wait     max wait     avg wait          pid   comm

           2     10.58 us      8.39 us      5.29 us       257552   kworker/4:1
           2      9.76 us      7.96 us      4.88 us            0   swapper
           4      5.36 us      2.09 us      1.34 us       255462   kworker/0:2
           3      3.33 us      1.48 us      1.11 us       257680   kworker/3:1
           2      2.59 us      1.46 us      1.29 us       257478   kworker/2:2
           1      1.50 us      1.50 us      1.50 us           15   rcu_preempt

You can also use these two options together. :)

The two more test cases were added to the existing lock contention test.

The code is available at 'perf/lock-cgroup-v2' branch in the tree below.

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Thanks,
Namhyung


Namhyung Kim (5):
  perf tools: Add read_all_cgroups() and __cgroup_find()
  perf lock contention: Prepare to handle cgroups
  perf lock contention: Add -g/--lock-cgroup option
  perf lock contention: Add -G/--cgroup-filter option
  perf test: Improve perf lock contention test

 tools/perf/Documentation/perf-lock.txt        |  8 ++
 tools/perf/builtin-lock.c                     | 99 ++++++++++++++++++-
 tools/perf/tests/shell/lock_contention.sh     | 45 +++++++++
 tools/perf/util/bpf_lock_contention.c         | 51 +++++++++-
 .../perf/util/bpf_skel/lock_contention.bpf.c  | 48 ++++++++-
 tools/perf/util/bpf_skel/lock_data.h          |  3 +-
 tools/perf/util/cgroup.c                      | 63 ++++++++++--
 tools/perf/util/cgroup.h                      |  5 +
 tools/perf/util/lock-contention.h             | 10 +-
 9 files changed, 312 insertions(+), 20 deletions(-)


base-commit: 45fc4628c15ab2cb7b2f53354b21db63f0a41f81
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread

end of thread, other threads:[~2023-09-06 21:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [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
2023-09-06 17:49 ` [PATCH 4/5] perf lock contention: Add -G/--cgroup-filter option 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
  -- strict thread matches above, loose matches on Subject: below --
2023-08-30 23:01 [PATCHSET 0/5] perf lock contention: Add cgroup support (v1) Namhyung Kim
2023-08-30 23:01 ` [PATCH 3/5] perf lock contention: Add -g/--lock-cgroup option 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).