Messages in this thread | | | From | Ian Rogers <> | Date | Wed, 1 Jun 2022 23:21:34 -0700 | Subject | Re: [PATCH 2/5] perf lock: Add lock contention tracepoints record support |
| |
On Tue, May 31, 2022 at 11:58 PM Namhyung Kim <namhyung@kernel.org> wrote: > > When LOCKDEP and LOCK_STAT events are not available, it falls back to > record the new lock contention tracepoints. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/builtin-lock.c | 70 +++++++++++++++++++++++++++++++++++---- > 1 file changed, 63 insertions(+), 7 deletions(-) > > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c > index 23a33ac15e68..3e3320b8cede 100644 > --- a/tools/perf/builtin-lock.c > +++ b/tools/perf/builtin-lock.c > @@ -495,6 +495,12 @@ struct trace_lock_handler { > > int (*release_event)(struct evsel *evsel, > struct perf_sample *sample); > + > + int (*contention_begin_event)(struct evsel *evsel, > + struct perf_sample *sample); > + > + int (*contention_end_event)(struct evsel *evsel, > + struct perf_sample *sample);
Would it make sense to add a comment here about LOCKDEP and LOCK_STAT? It could be confusing if the handler isn't called on different kernels.
Thanks, Ian
> }; > > static struct lock_seq_stat *get_seq(struct thread_stat *ts, u64 addr) > @@ -822,6 +828,20 @@ static int evsel__process_lock_release(struct evsel *evsel, struct perf_sample * > return 0; > } > > +static int evsel__process_contention_begin(struct evsel *evsel, struct perf_sample *sample) > +{ > + if (trace_handler->contention_begin_event) > + return trace_handler->contention_begin_event(evsel, sample); > + return 0; > +} > + > +static int evsel__process_contention_end(struct evsel *evsel, struct perf_sample *sample) > +{ > + if (trace_handler->contention_end_event) > + return trace_handler->contention_end_event(evsel, sample); > + return 0; > +} > + > static void print_bad_events(int bad, int total) > { > /* Output for debug, this have to be removed */ > @@ -1023,6 +1043,11 @@ static const struct evsel_str_handler lock_tracepoints[] = { > { "lock:lock_release", evsel__process_lock_release, }, /* CONFIG_LOCKDEP */ > }; > > +static const struct evsel_str_handler contention_tracepoints[] = { > + { "lock:contention_begin", evsel__process_contention_begin, }, > + { "lock:contention_end", evsel__process_contention_end, }, > +}; > + > static bool force; > > static int __cmd_report(bool display_info) > @@ -1086,20 +1111,41 @@ static int __cmd_record(int argc, const char **argv) > "record", "-R", "-m", "1024", "-c", "1", "--synth", "task", > }; > unsigned int rec_argc, i, j, ret; > + unsigned int nr_tracepoints; > const char **rec_argv; > + bool has_lock_stat = true; > > for (i = 0; i < ARRAY_SIZE(lock_tracepoints); i++) { > if (!is_valid_tracepoint(lock_tracepoints[i].name)) { > - pr_err("tracepoint %s is not enabled. " > - "Are CONFIG_LOCKDEP and CONFIG_LOCK_STAT enabled?\n", > - lock_tracepoints[i].name); > - return 1; > + pr_debug("tracepoint %s is not enabled. " > + "Are CONFIG_LOCKDEP and CONFIG_LOCK_STAT enabled?\n", > + lock_tracepoints[i].name); > + has_lock_stat = false; > + break; > + } > + } > + > + if (has_lock_stat) > + goto setup_args; > + > + for (i = 0; i < ARRAY_SIZE(contention_tracepoints); i++) { > + if (!is_valid_tracepoint(contention_tracepoints[i].name)) { > + pr_err("tracepoint %s is not enabled.\n", > + contention_tracepoints[i].name); > + return 1; > } > } > > +setup_args: > rec_argc = ARRAY_SIZE(record_args) + argc - 1; > + > + if (has_lock_stat) > + nr_tracepoints = ARRAY_SIZE(lock_tracepoints); > + else > + nr_tracepoints = ARRAY_SIZE(contention_tracepoints); > + > /* factor of 2 is for -e in front of each tracepoint */ > - rec_argc += 2 * ARRAY_SIZE(lock_tracepoints); > + rec_argc += 2 * nr_tracepoints; > > rec_argv = calloc(rec_argc + 1, sizeof(char *)); > if (!rec_argv) > @@ -1108,9 +1154,19 @@ static int __cmd_record(int argc, const char **argv) > for (i = 0; i < ARRAY_SIZE(record_args); i++) > rec_argv[i] = strdup(record_args[i]); > > - for (j = 0; j < ARRAY_SIZE(lock_tracepoints); j++) { > + for (j = 0; j < nr_tracepoints; j++) { > + const char *ev_name; > + > + if (has_lock_stat) > + ev_name = strdup(lock_tracepoints[j].name); > + else > + ev_name = strdup(contention_tracepoints[j].name); > + > + if (!ev_name) > + return -ENOMEM; > + > rec_argv[i++] = "-e"; > - rec_argv[i++] = strdup(lock_tracepoints[j].name); > + rec_argv[i++] = ev_name; > } > > for (j = 1; j < (unsigned int)argc; j++, i++) > -- > 2.36.1.255.ge46751e96f-goog >
| |