2022-03-24 03:10:33

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 1/3] perf lock: Add --synth=no option for record

The perf lock command has nothing to symbolize and lock names come
from the tracepoint. Moreover, kernel symbols are available even the
--synth=no option is given.

This will reduce the startup time by avoiding unnecessary synthesis.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-lock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 57b9ebd7118a..1ebff88bc5ba 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -990,7 +990,7 @@ static int __cmd_report(bool display_info)
static int __cmd_record(int argc, const char **argv)
{
const char *record_args[] = {
- "record", "-R", "-m", "1024", "-c", "1",
+ "record", "-R", "-m", "1024", "-c", "1", "--synth", "no",
};
unsigned int rec_argc, i, j, ret;
const char **rec_argv;
--
2.35.1.894.gb6a874cedc-goog


2022-03-24 13:33:17

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 3/3] perf lock: Add -F/--field option to control output

The -F/--field option is to customize output field.

$ perf lock report -F contended,wait_max -k avg_wait
Name contended max wait (ns) avg wait (ns)

slock-AF_INET6 1 23543 23543
&lruvec->lru_lock 5 18317 11254
slock-AF_INET6 1 10379 10379
rcu_node_1 1 2104 2104
&dentry->d_lockr... 1 1844 1844
&dentry->d_lockr... 1 1672 1672
&newf->file_lock 15 2279 1025
&dentry->d_lockr... 1 792 792

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/Documentation/perf-lock.txt | 6 +++
tools/perf/builtin-lock.c | 55 +++++++++++++++++++++++---
2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
index f5eb95788969..b43222229807 100644
--- a/tools/perf/Documentation/perf-lock.txt
+++ b/tools/perf/Documentation/perf-lock.txt
@@ -54,6 +54,12 @@ REPORT OPTIONS
Sorting key. Possible values: acquired (default), contended,
avg_wait, wait_total, wait_max, wait_min.

+-F::
+--field=<value>::
+ Output fields. By default it shows all the fields but users can
+ customize that using this. Possible values: acquired, contended,
+ avg_wait, wait_total, wait_max, wait_min.
+
-c::
--combine-locks::
Merge lock instances in the same class (based on name).
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index c2ecb6acb87d..a7089760a7de 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -282,6 +282,7 @@ static struct rb_root sorted; /* place to store intermediate data */
static struct rb_root result; /* place to store sorted data */

static LIST_HEAD(lock_keys);
+static const char *output_fields;

#define DEF_KEY_LOCK(name, header, fn_suffix, len) \
{ #name, header, len, lock_stat_key_ ## fn_suffix, lock_stat_key_print_ ## fn_suffix, {} }
@@ -304,23 +305,65 @@ static int select_key(void)
for (i = 0; keys[i].name; i++) {
if (!strcmp(keys[i].name, sort_key)) {
compare = keys[i].key;
+
+ /* selected key should be in the output fields */
+ if (list_empty(&keys[i].list))
+ list_add_tail(&keys[i].list, &lock_keys);
+
return 0;
}
}

pr_err("Unknown compare key: %s\n", sort_key);
-
return -1;
}

-static int setup_output_field(void)
+static int add_output_field(struct list_head *head, char *name)
{
int i;

+ for (i = 0; keys[i].name; i++) {
+ if (strcmp(keys[i].name, name))
+ continue;
+
+ /* prevent double link */
+ if (list_empty(&keys[i].list))
+ list_add_tail(&keys[i].list, head);
+
+ return 0;
+ }
+
+ pr_err("Unknown output field: %s\n", name);
+ return -1;
+}
+
+static int setup_output_field(const char *str)
+{
+ char *tok, *tmp, *orig;
+ int i, ret = 0;
+
+ /* no output field given: use all of them */
+ if (str == NULL) {
+ for (i = 0; keys[i].name; i++)
+ list_add_tail(&keys[i].list, &lock_keys);
+ return 0;
+ }
+
for (i = 0; keys[i].name; i++)
- list_add_tail(&keys[i].list, &lock_keys);
+ INIT_LIST_HEAD(&keys[i].list);

- return 0;
+ orig = tmp = strdup(str);
+ if (orig == NULL)
+ return -ENOMEM;
+
+ while ((tok = strsep(&tmp, ",")) != NULL){
+ ret = add_output_field(&lock_keys, tok);
+ if (ret < 0)
+ break;
+ }
+ free(orig);
+
+ return ret;
}

static void combine_lock_stats(struct lock_stat *st)
@@ -1002,7 +1045,7 @@ static int __cmd_report(bool display_info)
goto out_delete;
}

- if (setup_output_field())
+ if (setup_output_field(output_fields))
goto out_delete;

if (select_key())
@@ -1090,6 +1133,8 @@ int cmd_lock(int argc, const char **argv)
const struct option report_options[] = {
OPT_STRING('k', "key", &sort_key, "acquired",
"key for sorting (acquired / contended / avg_wait / wait_total / wait_max / wait_min)"),
+ OPT_STRING('F', "field", &output_fields, NULL,
+ "output fields (acquired / contended / avg_wait / wait_total / wait_max / wait_min)"),
/* TODO: type */
OPT_BOOLEAN('c', "combine-locks", &combine_locks,
"combine locks in the same class"),
--
2.35.1.894.gb6a874cedc-goog

2022-03-24 21:18:20

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/3] perf lock: Extend struct lock_key to have print function

And use it to print output for each key field. No functional change
intended and the output should be identical.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-lock.c | 91 ++++++++++++++++++++++++++++-----------
1 file changed, 65 insertions(+), 26 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 1ebff88bc5ba..c2ecb6acb87d 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -237,9 +237,43 @@ struct lock_key {
* e.g. nr_acquired -> acquired, wait_time_total -> wait_total
*/
const char *name;
+ /* header: the string printed on the header line */
+ const char *header;
+ /* len: the printing width of the field */
+ int len;
+ /* key: a pointer to function to compare two lock stats for sorting */
int (*key)(struct lock_stat*, struct lock_stat*);
+ /* print: a pointer to function to print a given lock stats */
+ void (*print)(struct lock_key*, struct lock_stat*);
+ /* list: list entry to link this */
+ struct list_head list;
};

+#define PRINT_KEY(member) \
+static void lock_stat_key_print_ ## member(struct lock_key *key, \
+ struct lock_stat *ls) \
+{ \
+ pr_info("%*llu", key->len, (unsigned long long)ls->member); \
+}
+
+PRINT_KEY(nr_acquired)
+PRINT_KEY(nr_contended)
+PRINT_KEY(avg_wait_time)
+PRINT_KEY(wait_time_total)
+PRINT_KEY(wait_time_max)
+
+static void lock_stat_key_print_wait_time_min(struct lock_key *key,
+ struct lock_stat *ls)
+{
+ u64 wait_time = ls->wait_time_min;
+
+ if (wait_time == ULLONG_MAX)
+ wait_time = 0;
+
+ pr_info("%*"PRIu64, key->len, wait_time);
+}
+
+
static const char *sort_key = "acquired";

static int (*compare)(struct lock_stat *, struct lock_stat *);
@@ -247,19 +281,20 @@ static int (*compare)(struct lock_stat *, struct lock_stat *);
static struct rb_root sorted; /* place to store intermediate data */
static struct rb_root result; /* place to store sorted data */

-#define DEF_KEY_LOCK(name, fn_suffix) \
- { #name, lock_stat_key_ ## fn_suffix }
+static LIST_HEAD(lock_keys);
+
+#define DEF_KEY_LOCK(name, header, fn_suffix, len) \
+ { #name, header, len, lock_stat_key_ ## fn_suffix, lock_stat_key_print_ ## fn_suffix, {} }
struct lock_key keys[] = {
- DEF_KEY_LOCK(acquired, nr_acquired),
- DEF_KEY_LOCK(contended, nr_contended),
- DEF_KEY_LOCK(avg_wait, avg_wait_time),
- DEF_KEY_LOCK(wait_total, wait_time_total),
- DEF_KEY_LOCK(wait_min, wait_time_min),
- DEF_KEY_LOCK(wait_max, wait_time_max),
+ DEF_KEY_LOCK(acquired, "acquired", nr_acquired, 10),
+ DEF_KEY_LOCK(contended, "contended", nr_contended, 10),
+ DEF_KEY_LOCK(avg_wait, "avg wait (ns)", avg_wait_time, 15),
+ DEF_KEY_LOCK(wait_total, "total wait (ns)", wait_time_total, 15),
+ DEF_KEY_LOCK(wait_max, "max wait (ns)", wait_time_max, 15),
+ DEF_KEY_LOCK(wait_min, "min wait (ns)", wait_time_min, 15),

/* extra comparisons much complicated should be here */
-
- { NULL, NULL }
+ { }
};

static int select_key(void)
@@ -278,6 +313,16 @@ static int select_key(void)
return -1;
}

+static int setup_output_field(void)
+{
+ int i;
+
+ for (i = 0; keys[i].name; i++)
+ list_add_tail(&keys[i].list, &lock_keys);
+
+ return 0;
+}
+
static void combine_lock_stats(struct lock_stat *st)
{
struct rb_node **rb = &sorted.rb_node;
@@ -753,18 +798,13 @@ static void print_bad_events(int bad, int total)
static void print_result(void)
{
struct lock_stat *st;
+ struct lock_key *key;
char cut_name[20];
int bad, total;

pr_info("%20s ", "Name");
- pr_info("%10s ", "acquired");
- pr_info("%10s ", "contended");
-
- pr_info("%15s ", "avg wait (ns)");
- pr_info("%15s ", "total wait (ns)");
- pr_info("%15s ", "max wait (ns)");
- pr_info("%15s ", "min wait (ns)");
-
+ list_for_each_entry(key, &lock_keys, list)
+ pr_info("%*s ", key->len, key->header);
pr_info("\n\n");

bad = total = 0;
@@ -789,14 +829,10 @@ static void print_result(void)
pr_info("%20s ", cut_name);
}

- pr_info("%10u ", st->nr_acquired);
- pr_info("%10u ", st->nr_contended);
-
- pr_info("%15" PRIu64 " ", st->avg_wait_time);
- pr_info("%15" PRIu64 " ", st->wait_time_total);
- pr_info("%15" PRIu64 " ", st->wait_time_max);
- pr_info("%15" PRIu64 " ", st->wait_time_min == ULLONG_MAX ?
- 0 : st->wait_time_min);
+ list_for_each_entry(key, &lock_keys, list) {
+ key->print(key, st);
+ pr_info(" ");
+ }
pr_info("\n");
}

@@ -966,6 +1002,9 @@ static int __cmd_report(bool display_info)
goto out_delete;
}

+ if (setup_output_field())
+ goto out_delete;
+
if (select_key())
goto out_delete;

--
2.35.1.894.gb6a874cedc-goog

2022-03-25 20:23:04

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf lock: Add --synth=no option for record

Em Wed, Mar 23, 2022 at 04:02:57PM -0700, Namhyung Kim escreveu:
> The perf lock command has nothing to symbolize and lock names come
> from the tracepoint. Moreover, kernel symbols are available even the
> --synth=no option is given.
>
> This will reduce the startup time by avoiding unnecessary synthesis.

Thanks, applied the series.

- Arnaldo


> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/builtin-lock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index 57b9ebd7118a..1ebff88bc5ba 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -990,7 +990,7 @@ static int __cmd_report(bool display_info)
> static int __cmd_record(int argc, const char **argv)
> {
> const char *record_args[] = {
> - "record", "-R", "-m", "1024", "-c", "1",
> + "record", "-R", "-m", "1024", "-c", "1", "--synth", "no",
> };
> unsigned int rec_argc, i, j, ret;
> const char **rec_argv;
> --
> 2.35.1.894.gb6a874cedc-goog

--

- Arnaldo