Skip to content

Commit 1be9264

Browse files
Chun-Tse Shaonamhyung
authored andcommitted
perf lock: Fix parse_lock_type which only retrieve one lock flag
`parse_lock_type` can only add the first lock flag in `lock_type_table` given input `str`. For example, for `Y rwlock`, it only adds `rwlock:R` into this perf session. Another example is for `-Y mutex`, it only adds the mutex without `LCB_F_SPIN` flag. The patch fixes this issue, makes sure both `rwlock:R` and `rwlock:W` will be added with `-Y rwlock`, and so on. Testing: $ ./perf lock con -ab -Y mutex,rwlock -- perf bench sched pipe # Running 'sched/pipe' benchmark: # Executed 1000000 pipe operations between two processes Total time: 9.313 [sec] 9.313976 usecs/op 107365 ops/sec contended total wait max wait avg wait type caller 176 1.65 ms 19.43 us 9.38 us mutex pipe_read+0x57 34 180.14 us 10.93 us 5.30 us mutex pipe_write+0x50 7 77.48 us 16.09 us 11.07 us mutex do_epoll_wait+0x24d 7 74.70 us 13.50 us 10.67 us mutex do_epoll_wait+0x24d 3 35.97 us 14.44 us 11.99 us rwlock:W ep_done_scan+0x2d 3 35.00 us 12.23 us 11.66 us rwlock:W do_epoll_wait+0x255 2 15.88 us 11.96 us 7.94 us rwlock:W do_epoll_wait+0x47c 1 15.23 us 15.23 us 15.23 us rwlock:W do_epoll_wait+0x4d0 1 14.26 us 14.26 us 14.26 us rwlock:W ep_done_scan+0x2d 2 14.00 us 7.99 us 7.00 us mutex pipe_read+0x282 1 12.29 us 12.29 us 12.29 us rwlock:R ep_poll_callback+0x35 1 12.02 us 12.02 us 12.02 us rwlock:W do_epoll_ctl+0xb65 1 10.25 us 10.25 us 10.25 us rwlock:R ep_poll_callback+0x35 1 7.86 us 7.86 us 7.86 us mutex do_epoll_ctl+0x6c1 1 5.04 us 5.04 us 5.04 us mutex do_epoll_ctl+0x3d4 [namhyung: Add a comment and rename to 'mutex:spin' for consistency Fixes: d783ea8 ("perf lock contention: Simplify parse_lock_type()") Reviewed-by: Namhyung Kim <namhyung@kernel.org> Signed-off-by: Chun-Tse Shao <ctshao@google.com> Cc: nick.forrington@arm.com Link: https://lore.kernel.org/r/20250116235838.2769691-1-ctshao@google.com Signed-off-by: Namhyung Kim <namhyung@kernel.org>
1 parent 83196dd commit 1be9264

File tree

1 file changed

+41
-25
lines changed

1 file changed

+41
-25
lines changed

tools/perf/builtin-lock.c

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,8 +1484,8 @@ static const struct {
14841484
{ LCB_F_PERCPU | LCB_F_WRITE, "pcpu-sem:W", "percpu-rwsem" },
14851485
{ LCB_F_MUTEX, "mutex", "mutex" },
14861486
{ LCB_F_MUTEX | LCB_F_SPIN, "mutex", "mutex" },
1487-
/* alias for get_type_flag() */
1488-
{ LCB_F_MUTEX | LCB_F_SPIN, "mutex-spin", "mutex" },
1487+
/* alias for optimistic spinning only */
1488+
{ LCB_F_MUTEX | LCB_F_SPIN, "mutex:spin", "mutex-spin" },
14891489
};
14901490

14911491
static const char *get_type_str(unsigned int flags)
@@ -1510,19 +1510,6 @@ static const char *get_type_name(unsigned int flags)
15101510
return "unknown";
15111511
}
15121512

1513-
static unsigned int get_type_flag(const char *str)
1514-
{
1515-
for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
1516-
if (!strcmp(lock_type_table[i].name, str))
1517-
return lock_type_table[i].flags;
1518-
}
1519-
for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
1520-
if (!strcmp(lock_type_table[i].str, str))
1521-
return lock_type_table[i].flags;
1522-
}
1523-
return UINT_MAX;
1524-
}
1525-
15261513
static void lock_filter_finish(void)
15271514
{
15281515
zfree(&filters.types);
@@ -2254,29 +2241,58 @@ static int parse_lock_type(const struct option *opt __maybe_unused, const char *
22542241
int unset __maybe_unused)
22552242
{
22562243
char *s, *tmp, *tok;
2257-
int ret = 0;
22582244

22592245
s = strdup(str);
22602246
if (s == NULL)
22612247
return -1;
22622248

22632249
for (tok = strtok_r(s, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) {
2264-
unsigned int flags = get_type_flag(tok);
2250+
bool found = false;
22652251

2266-
if (flags == -1U) {
2267-
pr_err("Unknown lock flags: %s\n", tok);
2268-
ret = -1;
2269-
break;
2252+
/* `tok` is `str` in `lock_type_table` if it contains ':'. */
2253+
if (strchr(tok, ':')) {
2254+
for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
2255+
if (!strcmp(lock_type_table[i].str, tok) &&
2256+
add_lock_type(lock_type_table[i].flags)) {
2257+
found = true;
2258+
break;
2259+
}
2260+
}
2261+
2262+
if (!found) {
2263+
pr_err("Unknown lock flags name: %s\n", tok);
2264+
free(s);
2265+
return -1;
2266+
}
2267+
2268+
continue;
22702269
}
22712270

2272-
if (!add_lock_type(flags)) {
2273-
ret = -1;
2274-
break;
2271+
/*
2272+
* Otherwise `tok` is `name` in `lock_type_table`.
2273+
* Single lock name could contain multiple flags.
2274+
*/
2275+
for (unsigned int i = 0; i < ARRAY_SIZE(lock_type_table); i++) {
2276+
if (!strcmp(lock_type_table[i].name, tok)) {
2277+
if (add_lock_type(lock_type_table[i].flags)) {
2278+
found = true;
2279+
} else {
2280+
free(s);
2281+
return -1;
2282+
}
2283+
}
22752284
}
2285+
2286+
if (!found) {
2287+
pr_err("Unknown lock name: %s\n", tok);
2288+
free(s);
2289+
return -1;
2290+
}
2291+
22762292
}
22772293

22782294
free(s);
2279-
return ret;
2295+
return 0;
22802296
}
22812297

22822298
static bool add_lock_addr(unsigned long addr)

0 commit comments

Comments
 (0)