[PATCH 0/6] perf lock contention: Add more filter options (v1)

[PATCH 0/6] perf lock contention: Add more filter options (v1)

[Namhyung Kim]

Hello,

This patchset adds a couple of filters to perf lock contention command.

The -Y/--type-filter is to filter by lock types like spinlock or mutex.

  $ sudo ./perf lock con -ab -Y spinlock -E 3 -- ./perf bench sched messaging
  # Running 'sched/messaging' benchmark:
  # 20 sender and receiver processes per group
  # 10 groups == 400 processes run

       Total time: 0.167 [sec]
   contended   total wait     max wait     avg wait         type   caller

          11    669.31 us    107.17 us     60.85 us     spinlock   remove_wait_queue+0x14
          10    586.85 us     87.62 us     58.68 us     spinlock   prepare_to_wait+0x27
         186    497.36 us     12.94 us      2.67 us     spinlock   try_to_wake_up+0x1f5

For the same workload, you can see the rwlock results only like below.

  $ sudo ./perf lock con -ab -Y rwlock -E 3 -- ./perf bench sched messaging
  # Running 'sched/messaging' benchmark:
  # 20 sender and receiver processes per group
  # 10 groups == 400 processes run

       Total time: 0.171 [sec]
   contended   total wait     max wait     avg wait         type   caller

          20    142.11 us     17.10 us      7.11 us     rwlock:W   do_exit+0x36d
           3     26.49 us     12.04 us      8.83 us     rwlock:W   release_task+0x6e
           5     12.46 us      5.12 us      2.49 us     rwlock:R   do_wait+0x8b

The -L/--lock-filter is to filter by lock address or name.  You can use
the existing -l/--lock-addr option to get the info.

  $ sudo ./perf lock con -abl -- ./perf bench sched messaging 2>&1 | grep tasklist_lock
          25     39.78 us     16.51 us      1.59 us   ffffffff9d006080   tasklist_lock

And use it with -L option like below.

  $ sudo ./perf lock con -ab -L tasklist_lock -- ./perf bench sched messaging 2>&1
  # 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         type   caller

          22    227.18 us     24.16 us     10.33 us     rwlock:W   do_exit+0x36d
           3     26.12 us     18.03 us      8.71 us     rwlock:W   release_task+0x6e

Passing the address is supported too.

  $ sudo ./perf lock con -ab -L ffffffff9d006080 -- ./perf bench sched messaging 2>&1
  # Running 'sched/messaging' benchmark:
  # 20 sender and receiver processes per group
  # 10 groups == 400 processes run

       Total time: 0.190 [sec]
   contended   total wait     max wait     avg wait         type   caller

          28    276.62 us     16.90 us      9.88 us     rwlock:W   do_exit+0x36d
           4     22.36 us      7.04 us      5.59 us     rwlock:R   do_wait+0x8b
           2     10.51 us      5.38 us      5.25 us     rwlock:W   release_task+0x6e

You can get it from 'perf/lock-filter-v1' branch in

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

Thanks,
Namhyung


Namhyung Kim (6):
  perf lock contention: Factor out lock_type_table
  perf lock contention: Add -Y/--type-filter option
  perf lock contention: Support lock type filtering for BPF
  perf lock contention: Add -L/--lock-filter option
  perf lock contention: Support lock addr/name filtering for BPF
  perf test: Update perf lock contention test

 tools/perf/Documentation/perf-lock.txt        |  27 +-
 tools/perf/builtin-lock.c                     | 305 ++++++++++++++++--
 tools/perf/tests/shell/lock_contention.sh     |  58 +++-
 tools/perf/util/bpf_lock_contention.c         |  55 +++-
 .../perf/util/bpf_skel/lock_contention.bpf.c  |  38 ++-
 tools/perf/util/lock-contention.h             |  10 +
 6 files changed, 451 insertions(+), 42 deletions(-)


base-commit: 51c4f2bf5397b34b79a6712221606e0ab2e6f7ed
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 1/6] perf lock contention: Factor out lock_type_table

[Namhyung Kim]

Move it out of get_type_str() so that we can reuse the table for others
later.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-lock.c | 48 +++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 25c0a5e5051f..548d81eb0b18 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -1437,30 +1437,30 @@ static void sort_result(void)
 	}
 }
 
-static const char *get_type_str(struct lock_stat *st)
-{
-	static const struct {
-		unsigned int flags;
-		const char *name;
-	} table[] = {
-		{ 0,				"semaphore" },
-		{ LCB_F_SPIN,			"spinlock" },
-		{ LCB_F_SPIN | LCB_F_READ,	"rwlock:R" },
-		{ LCB_F_SPIN | LCB_F_WRITE,	"rwlock:W"},
-		{ LCB_F_READ,			"rwsem:R" },
-		{ LCB_F_WRITE,			"rwsem:W" },
-		{ LCB_F_RT,			"rtmutex" },
-		{ LCB_F_RT | LCB_F_READ,	"rwlock-rt:R" },
-		{ LCB_F_RT | LCB_F_WRITE,	"rwlock-rt:W"},
-		{ LCB_F_PERCPU | LCB_F_READ,	"pcpu-sem:R" },
-		{ LCB_F_PERCPU | LCB_F_WRITE,	"pcpu-sem:W" },
-		{ LCB_F_MUTEX,			"mutex" },
-		{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex" },
-	};
+static const struct {
+	unsigned int flags;
+	const char *name;
+} lock_type_table[] = {
+	{ 0,				"semaphore" },
+	{ LCB_F_SPIN,			"spinlock" },
+	{ LCB_F_SPIN | LCB_F_READ,	"rwlock:R" },
+	{ LCB_F_SPIN | LCB_F_WRITE,	"rwlock:W"},
+	{ LCB_F_READ,			"rwsem:R" },
+	{ LCB_F_WRITE,			"rwsem:W" },
+	{ LCB_F_RT,			"rtmutex" },
+	{ LCB_F_RT | LCB_F_READ,	"rwlock-rt:R" },
+	{ LCB_F_RT | LCB_F_WRITE,	"rwlock-rt:W"},
+	{ LCB_F_PERCPU | LCB_F_READ,	"pcpu-sem:R" },
+	{ LCB_F_PERCPU | LCB_F_WRITE,	"pcpu-sem:W" },
+	{ LCB_F_MUTEX,			"mutex" },
+	{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex" },
+};
 
-	for (unsigned int i = 0; i < ARRAY_SIZE(table); i++) {
-		if (table[i].flags == st->flags)
-			return table[i].name;
+static const char *get_type_str(unsigned int flags)
+{
+	for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
+		if (lock_type_table[i].flags == flags)
+			return lock_type_table[i].name;
 	}
 	return "unknown";
 }
@@ -1514,7 +1514,7 @@ static void print_contention_result(struct lock_contention *con)
 
 		switch (aggr_mode) {
 		case LOCK_AGGR_CALLER:
-			pr_info("  %10s   %s\n", get_type_str(st), st->name);
+			pr_info("  %10s   %s\n", get_type_str(st->flags), st->name);
 			break;
 		case LOCK_AGGR_TASK:
 			pid = st->addr;
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 2/6] perf lock contention: Add -Y/--type-filter option

[Namhyung Kim]

The -Y/--type-filter option is to filter the result for specific lock
types only.  It can accept comma-separated values.  Note that it would
accept type names like one in the output.  spinlock, mutex, rwsem:R and
so on.

For RW-variant lock types, it converts the name to the both variants.
In other words, "rwsem" is same as "rwsem:R,rwsem:W".  Also note that
"mutex" has two different encoding - one for sleeping wait, another for
optimistic spinning.  Add "mutex-spin" entry for the lock_type_table so
that we can add it for "mutex" under the table.

  $ sudo ./perf lock record -a -- ./perf bench sched messaging

  $ sudo ./perf lock con -E 5 -Y spinlock
   contended   total wait     max wait     avg wait         type   caller

         802      1.26 ms     11.73 us      1.58 us     spinlock   __wake_up_common_lock+0x62
          13    787.16 us    105.44 us     60.55 us     spinlock   remove_wait_queue+0x14
          12    612.96 us     78.70 us     51.08 us     spinlock   prepare_to_wait+0x27
         114    340.68 us     12.61 us      2.99 us     spinlock   try_to_wake_up+0x1f5
          83    226.38 us      9.15 us      2.73 us     spinlock   folio_lruvec_lock_irqsave+0x5e

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-lock.txt |  23 +++--
 tools/perf/builtin-lock.c              | 116 ++++++++++++++++++++++++-
 tools/perf/util/lock-contention.h      |   5 ++
 3 files changed, 136 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
index 38e79d45e426..dea04ad5c28e 100644
--- a/tools/perf/Documentation/perf-lock.txt
+++ b/tools/perf/Documentation/perf-lock.txt
@@ -143,25 +143,25 @@ CONTENTION OPTIONS
         System-wide collection from all CPUs.
 
 -C::
---cpu::
+--cpu=<value>::
 	Collect samples only on the list of CPUs provided. Multiple CPUs can be
 	provided as a comma-separated list with no space: 0,1. Ranges of CPUs
 	are specified with -: 0-2.  Default is to monitor all CPUs.
 
 -p::
---pid=::
+--pid=<value>::
 	Record events on existing process ID (comma separated list).
 
---tid=::
+--tid=<value>::
         Record events on existing thread ID (comma separated list).
 
---map-nr-entries::
+--map-nr-entries=<value>::
 	Maximum number of BPF map entries (default: 10240).
 
---max-stack::
+--max-stack=<value>::
 	Maximum stack depth when collecting lock contention (default: 8).
 
---stack-skip
+--stack-skip=<value>::
 	Number of stack depth to skip when finding a lock caller (default: 3).
 
 -E::
@@ -172,6 +172,17 @@ CONTENTION OPTIONS
 --lock-addr::
 	Show lock contention stat by address
 
+-Y::
+--type-filter=<value>::
+	Show lock contention only for given lock types (comma separated list).
+	Available values are:
+	  semaphore, spinlock, rwlock, rwlock:R, rwlock:W, rwsem, rwsem:R, rwsem:W,
+	  rtmutex, rwlock-rt, rwlock-rt:R, rwlock-rt:W, pcpu-sem, pcpu-sem:R, pcpu-sem:W,
+	  mutex
+
+	Note that RW-variant of locks have :R and :W suffix.  Names without the
+	suffix are shortcuts for the both variants.  Ex) rwsem = rwsem:R + rwsem:W.
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 548d81eb0b18..49b4add53204 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -63,6 +63,8 @@ static int max_stack_depth = CONTENTION_STACK_DEPTH;
 static int stack_skip = CONTENTION_STACK_SKIP;
 static int print_nr_entries = INT_MAX / 2;
 
+static struct lock_filter filters;
+
 static enum lock_aggr_mode aggr_mode = LOCK_AGGR_ADDR;
 
 static struct thread_stat *thread_stat_find(u32 tid)
@@ -990,8 +992,9 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
 	struct thread_stat *ts;
 	struct lock_seq_stat *seq;
 	u64 addr = evsel__intval(evsel, sample, "lock_addr");
+	unsigned int flags = evsel__intval(evsel, sample, "flags");
 	u64 key;
-	int ret;
+	int i, ret;
 
 	ret = get_key_by_aggr_mode(&key, addr, evsel, sample);
 	if (ret < 0)
@@ -1001,7 +1004,6 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
 	if (!ls) {
 		char buf[128];
 		const char *name = "";
-		unsigned int flags = evsel__intval(evsel, sample, "flags");
 		struct machine *machine = &session->machines.host;
 		struct map *kmap;
 		struct symbol *sym;
@@ -1036,6 +1038,20 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
 		}
 	}
 
+	if (filters.nr_types) {
+		bool found = false;
+
+		for (i = 0; i < filters.nr_types; i++) {
+			if (flags == filters.types[i]) {
+				found = true;
+				break;
+			}
+		}
+
+		if (!found)
+			return 0;
+	}
+
 	ts = thread_stat_findnew(sample->tid);
 	if (!ts)
 		return -ENOMEM;
@@ -1454,6 +1470,8 @@ static const struct {
 	{ LCB_F_PERCPU | LCB_F_WRITE,	"pcpu-sem:W" },
 	{ LCB_F_MUTEX,			"mutex" },
 	{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex" },
+	/* alias for get_type_flag() */
+	{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex-spin" },
 };
 
 static const char *get_type_str(unsigned int flags)
@@ -1465,6 +1483,21 @@ static const char *get_type_str(unsigned int flags)
 	return "unknown";
 }
 
+static unsigned int get_type_flag(const char *str)
+{
+	for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
+		if (!strcmp(lock_type_table[i].name, str))
+			return lock_type_table[i].flags;
+	}
+	return -1U;
+}
+
+static void lock_filter_finish(void)
+{
+	zfree(&filters.types);
+	filters.nr_types = 0;
+}
+
 static void sort_contention_result(void)
 {
 	sort_result();
@@ -1507,6 +1540,9 @@ static void print_contention_result(struct lock_contention *con)
 		if (st->broken)
 			bad++;
 
+		if (!st->wait_time_total)
+			continue;
+
 		list_for_each_entry(key, &lock_keys, list) {
 			key->print(key, st);
 			pr_info(" ");
@@ -1753,6 +1789,7 @@ static int __cmd_contention(int argc, const char **argv)
 	print_contention_result(&con);
 
 out_delete:
+	lock_filter_finish();
 	evlist__delete(con.evlist);
 	lock_contention_finish();
 	perf_session__delete(session);
@@ -1884,6 +1921,79 @@ static int parse_max_stack(const struct option *opt, const char *str,
 	return 0;
 }
 
+static bool add_lock_type(unsigned int flags)
+{
+	unsigned int *tmp;
+
+	tmp = realloc(filters.types, (filters.nr_types + 1) * sizeof(*filters.types));
+	if (tmp == NULL)
+		return false;
+
+	tmp[filters.nr_types++] = flags;
+	filters.types = tmp;
+	return true;
+}
+
+static int parse_lock_type(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)) {
+		unsigned int flags = get_type_flag(tok);
+
+		if (flags == -1U) {
+			char buf[32];
+
+			if (strchr(tok, ':'))
+			    continue;
+
+			/* try :R and :W suffixes for rwlock, rwsem, ... */
+			scnprintf(buf, sizeof(buf), "%s:R", tok);
+			flags = get_type_flag(buf);
+			if (flags != -1UL) {
+				if (!add_lock_type(flags)) {
+					ret = -1;
+					break;
+				}
+			}
+
+			scnprintf(buf, sizeof(buf), "%s:W", tok);
+			flags = get_type_flag(buf);
+			if (flags != -1UL) {
+				if (!add_lock_type(flags)) {
+					ret = -1;
+					break;
+				}
+			}
+			continue;
+		}
+
+		if (!add_lock_type(flags)) {
+			ret = -1;
+			break;
+		}
+
+		if (!strcmp(tok, "mutex")) {
+			flags = get_type_flag("mutex-spin");
+			if (flags != -1UL) {
+				if (!add_lock_type(flags)) {
+					ret = -1;
+					break;
+				}
+			}
+		}
+	}
+
+	free(s);
+	return ret;
+}
+
 int cmd_lock(int argc, const char **argv)
 {
 	const struct option lock_options[] = {
@@ -1947,6 +2057,8 @@ int cmd_lock(int argc, const char **argv)
 		    "Default: " __stringify(CONTENTION_STACK_SKIP)),
 	OPT_INTEGER('E', "entries", &print_nr_entries, "display this many functions"),
 	OPT_BOOLEAN('l', "lock-addr", &show_lock_addrs, "show lock stats by address"),
+	OPT_CALLBACK('Y', "type-filter", NULL, "FLAGS",
+		     "Filter specific type of locks", parse_lock_type),
 	OPT_PARENT(lock_options)
 	};
 
diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
index 47fd47fb56c1..d5b75b222d8e 100644
--- a/tools/perf/util/lock-contention.h
+++ b/tools/perf/util/lock-contention.h
@@ -5,6 +5,11 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 
+struct lock_filter {
+	int			nr_types;
+	unsigned int		*types;
+};
+
 struct lock_stat {
 	struct hlist_node	hash_entry;
 	struct rb_node		rb;		/* used for sorting */
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 3/6] perf lock contention: Support lock type filtering for BPF

[Namhyung Kim]

Likewise, add type_filter BPF hash map and check it when user gave a
lock type filter.

  $ sudo ./perf lock con -ab -Y rwlock -- ./perf bench sched messaging
  # Running 'sched/messaging' benchmark:
  # 20 sender and receiver processes per group
  # 10 groups == 400 processes run

       Total time: 0.203 [sec]
   contended   total wait     max wait     avg wait         type   caller

          15    156.19 us     19.45 us     10.41 us     rwlock:W   do_exit+0x36d
           1     11.12 us     11.12 us     11.12 us     rwlock:R   do_wait+0x8b
           1      5.09 us      5.09 us      5.09 us     rwlock:W   release_task+0x6e

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-lock.c                     |  1 +
 tools/perf/util/bpf_lock_contention.c         | 15 ++++++++++++-
 .../perf/util/bpf_skel/lock_contention.bpf.c  | 21 +++++++++++++++++--
 tools/perf/util/lock-contention.h             |  1 +
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 49b4add53204..e4e785d3b4ec 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -1689,6 +1689,7 @@ static int __cmd_contention(int argc, const char **argv)
 		.map_nr_entries = bpf_map_entries,
 		.max_stack = max_stack_depth,
 		.stack_skip = stack_skip,
+		.filters = &filters,
 	};
 
 	session = perf_session__new(use_bpf ? NULL : &data, &eops);
diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
index 8e1b791dc58f..b8590b82ad3d 100644
--- a/tools/perf/util/bpf_lock_contention.c
+++ b/tools/perf/util/bpf_lock_contention.c
@@ -20,7 +20,7 @@ static struct lock_contention_bpf *skel;
 int lock_contention_prepare(struct lock_contention *con)
 {
 	int i, fd;
-	int ncpus = 1, ntasks = 1;
+	int ncpus = 1, ntasks = 1, ntypes = 1;
 	struct evlist *evlist = con->evlist;
 	struct target *target = con->target;
 
@@ -46,9 +46,12 @@ int lock_contention_prepare(struct lock_contention *con)
 		ncpus = perf_cpu_map__nr(evlist->core.user_requested_cpus);
 	if (target__has_task(target))
 		ntasks = perf_thread_map__nr(evlist->core.threads);
+	if (con->filters->nr_types)
+		ntypes = con->filters->nr_types;
 
 	bpf_map__set_max_entries(skel->maps.cpu_filter, ncpus);
 	bpf_map__set_max_entries(skel->maps.task_filter, ntasks);
+	bpf_map__set_max_entries(skel->maps.type_filter, ntypes);
 
 	if (lock_contention_bpf__load(skel) < 0) {
 		pr_err("Failed to load lock-contention BPF skeleton\n");
@@ -90,6 +93,16 @@ int lock_contention_prepare(struct lock_contention *con)
 		bpf_map_update_elem(fd, &pid, &val, BPF_ANY);
 	}
 
+	if (con->filters->nr_types) {
+		u8 val = 1;
+
+		skel->bss->has_type = 1;
+		fd = bpf_map__fd(skel->maps.type_filter);
+
+		for (i = 0; i < con->filters->nr_types; i++)
+			bpf_map_update_elem(fd, &con->filters->types[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 11b0fc7ee53b..fb0128de7c00 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -62,10 +62,18 @@ struct {
 	__uint(max_entries, 1);
 } task_filter SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(key_size, sizeof(__u32));
+	__uint(value_size, sizeof(__u8));
+	__uint(max_entries, 1);
+} type_filter SEC(".maps");
+
 /* control flags */
 int enabled;
 int has_cpu;
 int has_task;
+int has_type;
 int stack_skip;
 
 /* determine the key of lock stat */
@@ -74,7 +82,7 @@ int aggr_mode;
 /* error stat */
 int lost;
 
-static inline int can_record(void)
+static inline int can_record(u64 *ctx)
 {
 	if (has_cpu) {
 		__u32 cpu = bpf_get_smp_processor_id();
@@ -94,6 +102,15 @@ static inline int can_record(void)
 			return 0;
 	}
 
+	if (has_type) {
+		__u8 *ok;
+		__u32 flags = (__u32)ctx[1];
+
+		ok = bpf_map_lookup_elem(&type_filter, &flags);
+		if (!ok)
+			return 0;
+	}
+
 	return 1;
 }
 
@@ -116,7 +133,7 @@ int contention_begin(u64 *ctx)
 	__u32 pid;
 	struct tstamp_data *pelem;
 
-	if (!enabled || !can_record())
+	if (!enabled || !can_record(ctx))
 		return 0;
 
 	pid = bpf_get_current_pid_tgid();
diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
index d5b75b222d8e..dc621386a16b 100644
--- a/tools/perf/util/lock-contention.h
+++ b/tools/perf/util/lock-contention.h
@@ -118,6 +118,7 @@ struct lock_contention {
 	struct target *target;
 	struct machine *machine;
 	struct hlist_head *result;
+	struct lock_filter *filters;
 	unsigned long map_nr_entries;
 	int lost;
 	int max_stack;
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 4/6] perf lock contention: Add -L/--lock-filter option

[Namhyung Kim]

The -L/--lock-filter option is to filter only given locks.  The locks
can be specified by address or name (if exists).

  $ sudo ./perf lock record -a  sleep 1

  $ sudo ./perf lock con -l
   contended   total wait     max wait     avg wait            address   symbol

          57      1.11 ms     42.83 us     19.54 us   ffff9f4140059000
          15    280.88 us     23.51 us     18.73 us   ffffffff9d007a40   jiffies_lock
           1     20.49 us     20.49 us     20.49 us   ffffffff9d0d50c0   rcu_state
           1      9.02 us      9.02 us      9.02 us   ffff9f41759e9ba0

  $ sudo ./perf lock con -L jiffies_lock,rcu_state
   contended   total wait     max wait     avg wait         type   caller

          15    280.88 us     23.51 us     18.73 us     spinlock   tick_sched_do_timer+0x93
           1     20.49 us     20.49 us     20.49 us     spinlock   __softirqentry_text_start+0xeb

  $ sudo ./perf lock con -L ffff9f4140059000
   contended   total wait     max wait     avg wait         type   caller

          38    779.40 us     42.83 us     20.51 us     spinlock   worker_thread+0x50
          11    216.30 us     39.87 us     19.66 us     spinlock   queue_work_on+0x39
           8    118.13 us     20.51 us     14.77 us     spinlock   kthread+0xe5

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-lock.txt |   4 +
 tools/perf/builtin-lock.c              | 140 +++++++++++++++++++++++--
 tools/perf/util/lock-contention.h      |   4 +
 3 files changed, 142 insertions(+), 6 deletions(-)

diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
index dea04ad5c28e..0f9f720e599d 100644
--- a/tools/perf/Documentation/perf-lock.txt
+++ b/tools/perf/Documentation/perf-lock.txt
@@ -183,6 +183,10 @@ CONTENTION OPTIONS
 	Note that RW-variant of locks have :R and :W suffix.  Names without the
 	suffix are shortcuts for the both variants.  Ex) rwsem = rwsem:R + rwsem:W.
 
+-L::
+--lock-filter=<value>::
+	Show lock contention only for given lock addresses or names (comma separated list).
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index e4e785d3b4ec..6b8ea2f0b90a 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -32,6 +32,7 @@
 #include <semaphore.h>
 #include <math.h>
 #include <limits.h>
+#include <ctype.h>
 
 #include <linux/list.h>
 #include <linux/hash.h>
@@ -995,24 +996,52 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
 	unsigned int flags = evsel__intval(evsel, sample, "flags");
 	u64 key;
 	int i, ret;
+	static bool kmap_loaded;
+	struct machine *machine = &session->machines.host;
+	struct map *kmap;
+	struct symbol *sym;
 
 	ret = get_key_by_aggr_mode(&key, addr, evsel, sample);
 	if (ret < 0)
 		return ret;
 
+	if (!kmap_loaded) {
+		unsigned long *addrs;
+
+		/* make sure it loads the kernel map to find lock symbols */
+		map__load(machine__kernel_map(machine));
+		kmap_loaded = true;
+
+		/* convert (kernel) symbols to addresses */
+		for (i = 0; i < filters.nr_syms; i++) {
+			sym = machine__find_kernel_symbol_by_name(machine,
+								  filters.syms[i],
+								  &kmap);
+			if (sym == NULL) {
+				pr_warning("ignore unknown symbol: %s\n",
+					   filters.syms[i]);
+				continue;
+			}
+
+			addrs = realloc(filters.addrs,
+					(filters.nr_addrs + 1) * sizeof(*addrs));
+			if (addrs == NULL) {
+				pr_warning("memory allocation failure\n");
+				return -ENOMEM;
+			}
+
+			addrs[filters.nr_addrs++] = kmap->unmap_ip(kmap, sym->start);
+			filters.addrs = addrs;
+		}
+	}
+
 	ls = lock_stat_find(key);
 	if (!ls) {
 		char buf[128];
 		const char *name = "";
-		struct machine *machine = &session->machines.host;
-		struct map *kmap;
-		struct symbol *sym;
 
 		switch (aggr_mode) {
 		case LOCK_AGGR_ADDR:
-			/* make sure it loads the kernel map to find lock symbols */
-			map__load(machine__kernel_map(machine));
-
 			sym = machine__find_kernel_symbol(machine, key, &kmap);
 			if (sym)
 				name = sym->name;
@@ -1052,6 +1081,20 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
 			return 0;
 	}
 
+	if (filters.nr_addrs) {
+		bool found = false;
+
+		for (i = 0; i < filters.nr_addrs; i++) {
+			if (addr == filters.addrs[i]) {
+				found = true;
+				break;
+			}
+		}
+
+		if (!found)
+			return 0;
+	}
+
 	ts = thread_stat_findnew(sample->tid);
 	if (!ts)
 		return -ENOMEM;
@@ -1496,6 +1539,15 @@ static void lock_filter_finish(void)
 {
 	zfree(&filters.types);
 	filters.nr_types = 0;
+
+	zfree(&filters.addrs);
+	filters.nr_addrs = 0;
+
+	for (int i = 0; i < filters.nr_syms; i++)
+		free(filters.syms[i]);
+
+	zfree(&filters.syms);
+	filters.nr_syms = 0;
 }
 
 static void sort_contention_result(void)
@@ -1995,6 +2047,80 @@ static int parse_lock_type(const struct option *opt __maybe_unused, const char *
 	return ret;
 }
 
+static bool add_lock_addr(unsigned long addr)
+{
+	unsigned long *tmp;
+
+	tmp = realloc(filters.addrs, (filters.nr_addrs + 1) * sizeof(*filters.addrs));
+	if (tmp == NULL) {
+		pr_err("Memory allocation failure\n");
+		return false;
+	}
+
+	tmp[filters.nr_addrs++] = addr;
+	filters.addrs = tmp;
+	return true;
+}
+
+static bool add_lock_sym(char *name)
+{
+	char **tmp;
+	char *sym = strdup(name);
+
+	if (sym == NULL) {
+		pr_err("Memory allocation failure\n");
+		return false;
+	}
+
+	tmp = realloc(filters.syms, (filters.nr_syms + 1) * sizeof(*filters.syms));
+	if (tmp == NULL) {
+		pr_err("Memory allocation failure\n");
+		free(sym);
+		return false;
+	}
+
+	tmp[filters.nr_syms++] = sym;
+	filters.syms = tmp;
+	return true;
+}
+
+static int parse_lock_addr(const struct option *opt __maybe_unused, const char *str,
+			   int unset __maybe_unused)
+{
+	char *s, *tmp, *tok;
+	int ret = 0;
+	u64 addr;
+
+	s = strdup(str);
+	if (s == NULL)
+		return -1;
+
+	for (tok = strtok_r(s, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) {
+		char *end;
+
+		addr = strtoul(tok, &end, 16);
+		if (*end == '\0') {
+			if (!add_lock_addr(addr)) {
+				ret = -1;
+				break;
+			}
+			continue;
+		}
+
+		/*
+		 * At this moment, we don't have kernel symbols.  Save the symbols
+		 * in a separate list and resolve them to addresses later.
+		 */
+		if (!add_lock_sym(tok)) {
+			ret = -1;
+			break;
+		}
+	}
+
+	free(s);
+	return ret;
+}
+
 int cmd_lock(int argc, const char **argv)
 {
 	const struct option lock_options[] = {
@@ -2060,6 +2186,8 @@ int cmd_lock(int argc, const char **argv)
 	OPT_BOOLEAN('l', "lock-addr", &show_lock_addrs, "show lock stats by address"),
 	OPT_CALLBACK('Y', "type-filter", NULL, "FLAGS",
 		     "Filter specific type of locks", parse_lock_type),
+	OPT_CALLBACK('L', "lock-filter", NULL, "ADDRS/NAMES",
+		     "Filter specific address/symbol of locks", parse_lock_addr),
 	OPT_PARENT(lock_options)
 	};
 
diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
index dc621386a16b..b99e83fccf5c 100644
--- a/tools/perf/util/lock-contention.h
+++ b/tools/perf/util/lock-contention.h
@@ -7,7 +7,11 @@
 
 struct lock_filter {
 	int			nr_types;
+	int			nr_addrs;
+	int			nr_syms;
 	unsigned int		*types;
+	unsigned long		*addrs;
+	char			**syms;
 };
 
 struct lock_stat {
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 5/6] perf lock contention: Support lock addr/name filtering for BPF

[Namhyung Kim]

Likewise, add addr_filter BPF hash map and check it with the lock
address.

  $ sudo ./perf lock con -ab -L tasklist_lock -- ./perf bench sched messaging
  # Running 'sched/messaging' benchmark:
  # 20 sender and receiver processes per group
  # 10 groups == 400 processes run

       Total time: 0.169 [sec]
   contended   total wait     max wait     avg wait         type   caller

          18    174.09 us     25.31 us      9.67 us     rwlock:W   do_exit+0x36d
           5     32.34 us     10.87 us      6.47 us     rwlock:R   do_wait+0x8b
           4     15.41 us      4.73 us      3.85 us     rwlock:W   release_task+0x6e

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/bpf_lock_contention.c         | 42 ++++++++++++++++++-
 .../perf/util/bpf_skel/lock_contention.bpf.c  | 17 ++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
index b8590b82ad3d..4a95c0ebcb3c 100644
--- a/tools/perf/util/bpf_lock_contention.c
+++ b/tools/perf/util/bpf_lock_contention.c
@@ -20,7 +20,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;
+	int ncpus = 1, ntasks = 1, ntypes = 1, naddrs = 1;
 	struct evlist *evlist = con->evlist;
 	struct target *target = con->target;
 
@@ -49,9 +49,39 @@ int lock_contention_prepare(struct lock_contention *con)
 	if (con->filters->nr_types)
 		ntypes = con->filters->nr_types;
 
+	/* resolve lock name filters to addr */
+	if (con->filters->nr_syms) {
+		struct symbol *sym;
+		struct map *kmap;
+		unsigned long *addrs;
+
+		for (i = 0; i < con->filters->nr_syms; i++) {
+			sym = machine__find_kernel_symbol_by_name(con->machine,
+								  con->filters->syms[i],
+								  &kmap);
+			if (sym == NULL) {
+				pr_warning("ignore unknown symbol: %s\n",
+					   con->filters->syms[i]);
+				continue;
+			}
+
+			addrs = realloc(con->filters->addrs,
+					(con->filters->nr_addrs + 1) * sizeof(*addrs));
+			if (addrs == NULL) {
+				pr_warning("memory allocation failure\n");
+				continue;
+			}
+
+			addrs[con->filters->nr_addrs++] = kmap->unmap_ip(kmap, sym->start);
+			con->filters->addrs = addrs;
+		}
+		naddrs = con->filters->nr_addrs;
+	}
+
 	bpf_map__set_max_entries(skel->maps.cpu_filter, ncpus);
 	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);
 
 	if (lock_contention_bpf__load(skel) < 0) {
 		pr_err("Failed to load lock-contention BPF skeleton\n");
@@ -103,6 +133,16 @@ int lock_contention_prepare(struct lock_contention *con)
 			bpf_map_update_elem(fd, &con->filters->types[i], &val, BPF_ANY);
 	}
 
+	if (con->filters->nr_addrs) {
+		u8 val = 1;
+
+		skel->bss->has_addr = 1;
+		fd = bpf_map__fd(skel->maps.addr_filter);
+
+		for (i = 0; i < con->filters->nr_addrs; i++)
+			bpf_map_update_elem(fd, &con->filters->addrs[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 fb0128de7c00..ad0ca5d50557 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -69,11 +69,19 @@ struct {
 	__uint(max_entries, 1);
 } type_filter SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(key_size, sizeof(__u64));
+	__uint(value_size, sizeof(__u8));
+	__uint(max_entries, 1);
+} addr_filter SEC(".maps");
+
 /* control flags */
 int enabled;
 int has_cpu;
 int has_task;
 int has_type;
+int has_addr;
 int stack_skip;
 
 /* determine the key of lock stat */
@@ -111,6 +119,15 @@ static inline int can_record(u64 *ctx)
 			return 0;
 	}
 
+	if (has_addr) {
+		__u8 *ok;
+		__u64 addr = ctx[0];
+
+		ok = bpf_map_lookup_elem(&addr_filter, &addr);
+		if (!ok)
+			return 0;
+	}
+
 	return 1;
 }
 
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 6/6] perf test: Update perf lock contention test

[Namhyung Kim]

Add more tests for the new filters.

  $ sudo perf test contention -v
   87: kernel lock contention analysis test                            :
  --- start ---
  test child forked, pid 412379
  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 --type-filter
  Testing perf lock contention --lock-filter
  test child finished with 0
  ---- end ----
  kernel lock contention analysis test: Ok

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/lock_contention.sh | 58 ++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index cc9ceb9e19ca..b05f1b1ca6c8 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -115,7 +115,7 @@ test_aggr_addr()
 	fi
 
 	# the perf lock contention output goes to the stderr
-	perf lock con -a -b -t -E 1 -q -- perf bench sched messaging > /dev/null 2> ${result}
+	perf lock con -a -b -l -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
@@ -123,6 +123,60 @@ test_aggr_addr()
 	fi
 }
 
+test_type_filter()
+{
+	echo "Testing perf lock contention --type-filter (w/ spinlock)"
+	perf lock contention -i ${perfdata} -Y spinlock -q 2> ${result}
+	if [ $(grep -c -v spinlock "${result}") != "0" ]; then
+		echo "[Fail] Recorded should not have non-spinlocks:" $(cat "${result}")
+		err=1
+		exit
+	fi
+
+	if ! perf lock con -b true > /dev/null 2>&1 ; then
+		return
+	fi
+
+	perf lock con -a -b -Y spinlock -q -- perf bench sched messaging > /dev/null 2> ${result}
+	if [ $(grep -c -v spinlock "${result}") != "0" ]; then
+		echo "[Fail] Recorded should not have non-spinlocks:" $(cat "${result}")
+		err=1
+		exit
+	fi
+}
+
+test_lock_filter()
+{
+	echo "Testing perf lock contention --lock-filter (w/ tasklist_lock)"
+	perf lock contention -i ${perfdata} -l -q 2> ${result}
+	if [ $(grep -c tasklist_lock "${result}") != "1" ]; then
+		echo "[Skip] Could not find 'tasklist_lock'"
+		return
+	fi
+
+	perf lock contention -i ${perfdata} -L tasklist_lock -q 2> ${result}
+
+	# find out the type of tasklist_lock
+	local type=$(head -1 "${result}" | awk '{ print $8 }' | sed -e 's/:.*//')
+
+	if [ $(grep -c -v "${type}" "${result}") != "0" ]; then
+		echo "[Fail] Recorded should not have non-${type} locks:" $(cat "${result}")
+		err=1
+		exit
+	fi
+
+	if ! perf lock con -b true > /dev/null 2>&1 ; then
+		return
+	fi
+
+	perf lock con -a -b -L tasklist_lock -q -- perf bench sched messaging > /dev/null 2> ${result}
+	if [ $(grep -c -v "${type}" "${result}") != "0" ]; then
+		echo "[Fail] Recorded should not have non-${type} locks:" $(cat "${result}")
+		err=1
+		exit
+	fi
+}
+
 check
 
 test_record
@@ -130,5 +184,7 @@ test_bpf
 test_record_concurrent
 test_aggr_task
 test_aggr_addr
+test_type_filter
+test_lock_filter
 
 exit ${err}
-- 
2.39.0.314.g84b9a713c41-goog

Re: [PATCH 0/6] perf lock contention: Add more filter options (v1)

[Arnaldo Carvalho de Melo]

Em Mon, Dec 19, 2022 at 12:17:26PM -0800, Namhyung Kim escreveu:
> Hello,
> 
> This patchset adds a couple of filters to perf lock contention command.

Thanks, applied.

- Arnaldo

 
> The -Y/--type-filter is to filter by lock types like spinlock or mutex.
> 
>   $ sudo ./perf lock con -ab -Y spinlock -E 3 -- ./perf bench sched messaging
>   # Running 'sched/messaging' benchmark:
>   # 20 sender and receiver processes per group
>   # 10 groups == 400 processes run
> 
>        Total time: 0.167 [sec]
>    contended   total wait     max wait     avg wait         type   caller
> 
>           11    669.31 us    107.17 us     60.85 us     spinlock   remove_wait_queue+0x14
>           10    586.85 us     87.62 us     58.68 us     spinlock   prepare_to_wait+0x27
>          186    497.36 us     12.94 us      2.67 us     spinlock   try_to_wake_up+0x1f5
> 
> For the same workload, you can see the rwlock results only like below.
> 
>   $ sudo ./perf lock con -ab -Y rwlock -E 3 -- ./perf bench sched messaging
>   # Running 'sched/messaging' benchmark:
>   # 20 sender and receiver processes per group
>   # 10 groups == 400 processes run
> 
>        Total time: 0.171 [sec]
>    contended   total wait     max wait     avg wait         type   caller
> 
>           20    142.11 us     17.10 us      7.11 us     rwlock:W   do_exit+0x36d
>            3     26.49 us     12.04 us      8.83 us     rwlock:W   release_task+0x6e
>            5     12.46 us      5.12 us      2.49 us     rwlock:R   do_wait+0x8b
> 
> The -L/--lock-filter is to filter by lock address or name.  You can use
> the existing -l/--lock-addr option to get the info.
> 
>   $ sudo ./perf lock con -abl -- ./perf bench sched messaging 2>&1 | grep tasklist_lock
>           25     39.78 us     16.51 us      1.59 us   ffffffff9d006080   tasklist_lock
> 
> And use it with -L option like below.
> 
>   $ sudo ./perf lock con -ab -L tasklist_lock -- ./perf bench sched messaging 2>&1
>   # 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         type   caller
> 
>           22    227.18 us     24.16 us     10.33 us     rwlock:W   do_exit+0x36d
>            3     26.12 us     18.03 us      8.71 us     rwlock:W   release_task+0x6e
> 
> Passing the address is supported too.
> 
>   $ sudo ./perf lock con -ab -L ffffffff9d006080 -- ./perf bench sched messaging 2>&1
>   # Running 'sched/messaging' benchmark:
>   # 20 sender and receiver processes per group
>   # 10 groups == 400 processes run
> 
>        Total time: 0.190 [sec]
>    contended   total wait     max wait     avg wait         type   caller
> 
>           28    276.62 us     16.90 us      9.88 us     rwlock:W   do_exit+0x36d
>            4     22.36 us      7.04 us      5.59 us     rwlock:R   do_wait+0x8b
>            2     10.51 us      5.38 us      5.25 us     rwlock:W   release_task+0x6e
> 
> You can get it from 'perf/lock-filter-v1' branch in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks,
> Namhyung
> 
> 
> Namhyung Kim (6):
>   perf lock contention: Factor out lock_type_table
>   perf lock contention: Add -Y/--type-filter option
>   perf lock contention: Support lock type filtering for BPF
>   perf lock contention: Add -L/--lock-filter option
>   perf lock contention: Support lock addr/name filtering for BPF
>   perf test: Update perf lock contention test
> 
>  tools/perf/Documentation/perf-lock.txt        |  27 +-
>  tools/perf/builtin-lock.c                     | 305 ++++++++++++++++--
>  tools/perf/tests/shell/lock_contention.sh     |  58 +++-
>  tools/perf/util/bpf_lock_contention.c         |  55 +++-
>  .../perf/util/bpf_skel/lock_contention.bpf.c  |  38 ++-
>  tools/perf/util/lock-contention.h             |  10 +
>  6 files changed, 451 insertions(+), 42 deletions(-)
> 
> 
> base-commit: 51c4f2bf5397b34b79a6712221606e0ab2e6f7ed
> -- 
> 2.39.0.314.g84b9a713c41-goog

-- 

- Arnaldo

Re: [PATCH 2/6] perf lock contention: Add -Y/--type-filter option

[Arnaldo Carvalho de Melo]

Em Mon, Dec 19, 2022 at 12:17:28PM -0800, Namhyung Kim escreveu:
> The -Y/--type-filter option is to filter the result for specific lock
> types only.  It can accept comma-separated values.  Note that it would
> accept type names like one in the output.  spinlock, mutex, rwsem:R and
> so on.
> 
> For RW-variant lock types, it converts the name to the both variants.
> In other words, "rwsem" is same as "rwsem:R,rwsem:W".  Also note that
> "mutex" has two different encoding - one for sleeping wait, another for
> optimistic spinning.  Add "mutex-spin" entry for the lock_type_table so
> that we can add it for "mutex" under the table.
> 
>   $ sudo ./perf lock record -a -- ./perf bench sched messaging
> 
>   $ sudo ./perf lock con -E 5 -Y spinlock
>    contended   total wait     max wait     avg wait         type   caller
> 
>          802      1.26 ms     11.73 us      1.58 us     spinlock   __wake_up_common_lock+0x62
>           13    787.16 us    105.44 us     60.55 us     spinlock   remove_wait_queue+0x14
>           12    612.96 us     78.70 us     51.08 us     spinlock   prepare_to_wait+0x27
>          114    340.68 us     12.61 us      2.99 us     spinlock   try_to_wake_up+0x1f5
>           83    226.38 us      9.15 us      2.73 us     spinlock   folio_lruvec_lock_irqsave+0x5e
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/Documentation/perf-lock.txt |  23 +++--
>  tools/perf/builtin-lock.c              | 116 ++++++++++++++++++++++++-
>  tools/perf/util/lock-contention.h      |   5 ++
>  3 files changed, 136 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
> index 38e79d45e426..dea04ad5c28e 100644
> --- a/tools/perf/Documentation/perf-lock.txt
> +++ b/tools/perf/Documentation/perf-lock.txt
> @@ -143,25 +143,25 @@ CONTENTION OPTIONS
>          System-wide collection from all CPUs.
>  
>  -C::
> ---cpu::
> +--cpu=<value>::
>  	Collect samples only on the list of CPUs provided. Multiple CPUs can be
>  	provided as a comma-separated list with no space: 0,1. Ranges of CPUs
>  	are specified with -: 0-2.  Default is to monitor all CPUs.
>  
>  -p::
> ---pid=::
> +--pid=<value>::
>  	Record events on existing process ID (comma separated list).
>  
> ---tid=::
> +--tid=<value>::
>          Record events on existing thread ID (comma separated list).
>  
> ---map-nr-entries::
> +--map-nr-entries=<value>::
>  	Maximum number of BPF map entries (default: 10240).
>  
> ---max-stack::
> +--max-stack=<value>::
>  	Maximum stack depth when collecting lock contention (default: 8).
>  
> ---stack-skip
> +--stack-skip=<value>::
>  	Number of stack depth to skip when finding a lock caller (default: 3).
>  
>  -E::
> @@ -172,6 +172,17 @@ CONTENTION OPTIONS
>  --lock-addr::
>  	Show lock contention stat by address
>  
> +-Y::
> +--type-filter=<value>::
> +	Show lock contention only for given lock types (comma separated list).
> +	Available values are:
> +	  semaphore, spinlock, rwlock, rwlock:R, rwlock:W, rwsem, rwsem:R, rwsem:W,
> +	  rtmutex, rwlock-rt, rwlock-rt:R, rwlock-rt:W, pcpu-sem, pcpu-sem:R, pcpu-sem:W,
> +	  mutex
> +
> +	Note that RW-variant of locks have :R and :W suffix.  Names without the
> +	suffix are shortcuts for the both variants.  Ex) rwsem = rwsem:R + rwsem:W.
> +
>  
>  SEE ALSO
>  --------
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index 548d81eb0b18..49b4add53204 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -63,6 +63,8 @@ static int max_stack_depth = CONTENTION_STACK_DEPTH;
>  static int stack_skip = CONTENTION_STACK_SKIP;
>  static int print_nr_entries = INT_MAX / 2;
>  
> +static struct lock_filter filters;
> +
>  static enum lock_aggr_mode aggr_mode = LOCK_AGGR_ADDR;
>  
>  static struct thread_stat *thread_stat_find(u32 tid)
> @@ -990,8 +992,9 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
>  	struct thread_stat *ts;
>  	struct lock_seq_stat *seq;
>  	u64 addr = evsel__intval(evsel, sample, "lock_addr");
> +	unsigned int flags = evsel__intval(evsel, sample, "flags");
>  	u64 key;
> -	int ret;
> +	int i, ret;
>  
>  	ret = get_key_by_aggr_mode(&key, addr, evsel, sample);
>  	if (ret < 0)
> @@ -1001,7 +1004,6 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
>  	if (!ls) {
>  		char buf[128];
>  		const char *name = "";
> -		unsigned int flags = evsel__intval(evsel, sample, "flags");
>  		struct machine *machine = &session->machines.host;
>  		struct map *kmap;
>  		struct symbol *sym;
> @@ -1036,6 +1038,20 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
>  		}
>  	}
>  
> +	if (filters.nr_types) {
> +		bool found = false;
> +
> +		for (i = 0; i < filters.nr_types; i++) {
> +			if (flags == filters.types[i]) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!found)
> +			return 0;
> +	}
> +
>  	ts = thread_stat_findnew(sample->tid);
>  	if (!ts)
>  		return -ENOMEM;
> @@ -1454,6 +1470,8 @@ static const struct {
>  	{ LCB_F_PERCPU | LCB_F_WRITE,	"pcpu-sem:W" },
>  	{ LCB_F_MUTEX,			"mutex" },
>  	{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex" },
> +	/* alias for get_type_flag() */
> +	{ LCB_F_MUTEX | LCB_F_SPIN,	"mutex-spin" },
>  };
>  
>  static const char *get_type_str(unsigned int flags)
> @@ -1465,6 +1483,21 @@ static const char *get_type_str(unsigned int flags)
>  	return "unknown";
>  }
>  
> +static unsigned int get_type_flag(const char *str)
> +{
> +	for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
> +		if (!strcmp(lock_type_table[i].name, str))
> +			return lock_type_table[i].flags;
> +	}
> +	return -1U;
> +}
> +
> +static void lock_filter_finish(void)
> +{
> +	zfree(&filters.types);
> +	filters.nr_types = 0;
> +}
> +
>  static void sort_contention_result(void)
>  {
>  	sort_result();
> @@ -1507,6 +1540,9 @@ static void print_contention_result(struct lock_contention *con)
>  		if (st->broken)
>  			bad++;
>  
> +		if (!st->wait_time_total)
> +			continue;
> +
>  		list_for_each_entry(key, &lock_keys, list) {
>  			key->print(key, st);
>  			pr_info(" ");
> @@ -1753,6 +1789,7 @@ static int __cmd_contention(int argc, const char **argv)
>  	print_contention_result(&con);
>  
>  out_delete:
> +	lock_filter_finish();
>  	evlist__delete(con.evlist);
>  	lock_contention_finish();
>  	perf_session__delete(session);
> @@ -1884,6 +1921,79 @@ static int parse_max_stack(const struct option *opt, const char *str,
>  	return 0;
>  }
>  
> +static bool add_lock_type(unsigned int flags)
> +{
> +	unsigned int *tmp;
> +
> +	tmp = realloc(filters.types, (filters.nr_types + 1) * sizeof(*filters.types));
> +	if (tmp == NULL)
> +		return false;
> +
> +	tmp[filters.nr_types++] = flags;
> +	filters.types = tmp;
> +	return true;
> +}
> +
> +static int parse_lock_type(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)) {
> +		unsigned int flags = get_type_flag(tok);
> +
> +		if (flags == -1U) {
> +			char buf[32];
> +
> +			if (strchr(tok, ':'))
> +			    continue;
> +
> +			/* try :R and :W suffixes for rwlock, rwsem, ... */
> +			scnprintf(buf, sizeof(buf), "%s:R", tok);
> +			flags = get_type_flag(buf);
> +			if (flags != -1UL) {
> +				if (!add_lock_type(flags)) {
> +					ret = -1;
> +					break;
> +				}
> +			}


clang doesn't like this:

  34    97.97 fedora:36                     : FAIL clang version 14.0.5 (Fedora 14.0.5-2.fc36)
    builtin-lock.c:2012:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
                            if (flags != -1UL) {
                                ~~~~~ ^  ~~~~
    builtin-lock.c:2021:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
                            if (flags != -1UL) {
                                ~~~~~ ^  ~~~~
    builtin-lock.c:2037:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
                            if (flags != -1UL) {
                                ~~~~~ ^  ~~~~
    3 errors generated.

I applied this on top, Ack?

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index daef37fe0ccda249..c73d02082cdf4b3a 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -1489,7 +1489,7 @@ static unsigned int get_type_flag(const char *str)
 		if (!strcmp(lock_type_table[i].name, str))
 			return lock_type_table[i].flags;
 	}
-	return -1U;
+	return UINT_MAX;
 }
 
 static void lock_filter_finish(void)
@@ -1956,7 +1956,7 @@ static int parse_lock_type(const struct option *opt __maybe_unused, const char *
 			/* try :R and :W suffixes for rwlock, rwsem, ... */
 			scnprintf(buf, sizeof(buf), "%s:R", tok);
 			flags = get_type_flag(buf);
-			if (flags != -1UL) {
+			if (flags != UINT_MAX) {
 				if (!add_lock_type(flags)) {
 					ret = -1;
 					break;
@@ -1965,7 +1965,7 @@ static int parse_lock_type(const struct option *opt __maybe_unused, const char *
 
 			scnprintf(buf, sizeof(buf), "%s:W", tok);
 			flags = get_type_flag(buf);
-			if (flags != -1UL) {
+			if (flags != UINT_MAX) {
 				if (!add_lock_type(flags)) {
 					ret = -1;
 					break;
@@ -1981,7 +1981,7 @@ static int parse_lock_type(const struct option *opt __maybe_unused, const char *
 
 		if (!strcmp(tok, "mutex")) {
 			flags = get_type_flag("mutex-spin");
-			if (flags != -1UL) {
+			if (flags != UINT_MAX) {
 				if (!add_lock_type(flags)) {
 					ret = -1;
 					break;

> +			flags = get_type_flag(buf);
> +			if (flags != -1UL) {
> +				if (!add_lock_type(flags)) {
> +					ret = -1;
> +					break;
> +				}
> +			}
> +			continue;
> +		}
> +
> +		if (!add_lock_type(flags)) {
> +			ret = -1;
> +			break;
> +		}
> +
> +		if (!strcmp(tok, "mutex")) {
> +			flags = get_type_flag("mutex-spin");
> +			if (flags != -1UL) {
> +				if (!add_lock_type(flags)) {
> +					ret = -1;
> +					break;
> +				}
> +			}
> +		}
> +	}
> +
> +	free(s);
> +	return ret;
> +}
> +
>  int cmd_lock(int argc, const char **argv)
>  {
>  	const struct option lock_options[] = {
> @@ -1947,6 +2057,8 @@ int cmd_lock(int argc, const char **argv)
>  		    "Default: " __stringify(CONTENTION_STACK_SKIP)),
>  	OPT_INTEGER('E', "entries", &print_nr_entries, "display this many functions"),
>  	OPT_BOOLEAN('l', "lock-addr", &show_lock_addrs, "show lock stats by address"),
> +	OPT_CALLBACK('Y', "type-filter", NULL, "FLAGS",
> +		     "Filter specific type of locks", parse_lock_type),
>  	OPT_PARENT(lock_options)
>  	};
>  
> diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
> index 47fd47fb56c1..d5b75b222d8e 100644
> --- a/tools/perf/util/lock-contention.h
> +++ b/tools/perf/util/lock-contention.h
> @@ -5,6 +5,11 @@
>  #include <linux/list.h>
>  #include <linux/rbtree.h>
>  
> +struct lock_filter {
> +	int			nr_types;
> +	unsigned int		*types;
> +};
> +
>  struct lock_stat {
>  	struct hlist_node	hash_entry;
>  	struct rb_node		rb;		/* used for sorting */
> -- 
> 2.39.0.314.g84b9a713c41-goog

-- 

- Arnaldo

Re: [PATCH 2/6] perf lock contention: Add -Y/--type-filter option

[Namhyung Kim]

Hi Arnaldo,

On Wed, Dec 21, 2022 at 9:50 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Mon, Dec 19, 2022 at 12:17:28PM -0800, Namhyung Kim escreveu:

[SNIP]
> > @@ -1465,6 +1483,21 @@ static const char *get_type_str(unsigned int flags)
> >       return "unknown";
> >  }
> >
> > +static unsigned int get_type_flag(const char *str)
> > +{
> > +     for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
> > +             if (!strcmp(lock_type_table[i].name, str))
> > +                     return lock_type_table[i].flags;
> > +     }
> > +     return -1U;
> > +}
> > +
> > +static void lock_filter_finish(void)
> > +{
> > +     zfree(&filters.types);
> > +     filters.nr_types = 0;
> > +}
> > +
> >  static void sort_contention_result(void)
> >  {
> >       sort_result();
> > @@ -1507,6 +1540,9 @@ static void print_contention_result(struct lock_contention *con)
> >               if (st->broken)
> >                       bad++;
> >
> > +             if (!st->wait_time_total)
> > +                     continue;
> > +
> >               list_for_each_entry(key, &lock_keys, list) {
> >                       key->print(key, st);
> >                       pr_info(" ");
> > @@ -1753,6 +1789,7 @@ static int __cmd_contention(int argc, const char **argv)
> >       print_contention_result(&con);
> >
> >  out_delete:
> > +     lock_filter_finish();
> >       evlist__delete(con.evlist);
> >       lock_contention_finish();
> >       perf_session__delete(session);
> > @@ -1884,6 +1921,79 @@ static int parse_max_stack(const struct option *opt, const char *str,
> >       return 0;
> >  }
> >
> > +static bool add_lock_type(unsigned int flags)
> > +{
> > +     unsigned int *tmp;
> > +
> > +     tmp = realloc(filters.types, (filters.nr_types + 1) * sizeof(*filters.types));
> > +     if (tmp == NULL)
> > +             return false;
> > +
> > +     tmp[filters.nr_types++] = flags;
> > +     filters.types = tmp;
> > +     return true;
> > +}
> > +
> > +static int parse_lock_type(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)) {
> > +             unsigned int flags = get_type_flag(tok);
> > +
> > +             if (flags == -1U) {
> > +                     char buf[32];
> > +
> > +                     if (strchr(tok, ':'))
> > +                         continue;
> > +
> > +                     /* try :R and :W suffixes for rwlock, rwsem, ... */
> > +                     scnprintf(buf, sizeof(buf), "%s:R", tok);
> > +                     flags = get_type_flag(buf);
> > +                     if (flags != -1UL) {
> > +                             if (!add_lock_type(flags)) {
> > +                                     ret = -1;
> > +                                     break;
> > +                             }
> > +                     }
>
>
> clang doesn't like this:
>
>   34    97.97 fedora:36                     : FAIL clang version 14.0.5 (Fedora 14.0.5-2.fc36)
>     builtin-lock.c:2012:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
>                             if (flags != -1UL) {
>                                 ~~~~~ ^  ~~~~
>     builtin-lock.c:2021:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
>                             if (flags != -1UL) {
>                                 ~~~~~ ^  ~~~~
>     builtin-lock.c:2037:14: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
>                             if (flags != -1UL) {
>                                 ~~~~~ ^  ~~~~
>     3 errors generated.
>
> I applied this on top, Ack?

Oh.. sorry for that.  It could be just -1U.  But I'm ok with UINT_MAX too.

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung