* [PATCH v4 0/1] Perf lock improvements @ 2024-05-13 9:14 Nick Forrington 2024-05-13 9:14 ` [PATCH 1/1] perf lock info: Display both map and thread by default Nick Forrington 0 siblings, 1 reply; 4+ messages in thread From: Nick Forrington @ 2024-05-13 9:14 UTC (permalink / raw) To: linux-kernel, linux-perf-users Cc: Nick Forrington, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan Improvement to command line handling for perf lock info. --- v1: https://lore.kernel.org/all/20231031120526.11502-1-nick.forrington@arm.com/ v2: * Drop previous "perf lock report: Restore aggregation by caller by default" patch. Not required for newly collected data with LOCKDEP tracepoints. * Display map and thread info (rather than an error) when neither or both of --map and --thread are specified. v3: * Update --help output v4: * Remove redundant error handling Nick Forrington (1): perf lock info: Display both map and thread by default tools/perf/Documentation/perf-lock.txt | 4 ++-- tools/perf/builtin-lock.c | 27 ++++++++++++++------------ 2 files changed, 17 insertions(+), 14 deletions(-) -- 2.44.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] perf lock info: Display both map and thread by default 2024-05-13 9:14 [PATCH v4 0/1] Perf lock improvements Nick Forrington @ 2024-05-13 9:14 ` Nick Forrington 2024-06-03 9:37 ` Nick Forrington 0 siblings, 1 reply; 4+ messages in thread From: Nick Forrington @ 2024-05-13 9:14 UTC (permalink / raw) To: linux-kernel, linux-perf-users Cc: Nick Forrington, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan Change "perf lock info" argument handling to: Display both map and thread info (rather than an error) when neither are specified. Display both map and thread info (rather than just thread info) when both are requested. Signed-off-by: Nick Forrington <nick.forrington@arm.com> --- tools/perf/Documentation/perf-lock.txt | 4 ++-- tools/perf/builtin-lock.c | 27 ++++++++++++++------------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt index f5938d616d75..57a940399de0 100644 --- a/tools/perf/Documentation/perf-lock.txt +++ b/tools/perf/Documentation/perf-lock.txt @@ -111,11 +111,11 @@ INFO OPTIONS -t:: --threads:: - dump thread list in perf.data + dump only the thread list in perf.data -m:: --map:: - dump map of lock instances (address:name table) + dump only the map of lock instances (address:name table) CONTENTION OPTIONS diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c index 230461280e45..7eb115aeb927 100644 --- a/tools/perf/builtin-lock.c +++ b/tools/perf/builtin-lock.c @@ -1477,20 +1477,16 @@ static void dump_map(void) fprintf(lock_output, " %#llx: %s\n", (unsigned long long)st->addr, st->name); } -static int dump_info(void) +static void dump_info(void) { - int rc = 0; - if (info_threads) dump_threads(); - else if (info_map) + + if (info_map) { + if (info_threads) + fputc('\n', lock_output); dump_map(); - else { - rc = -1; - pr_err("Unknown type of information\n"); } - - return rc; } static const struct evsel_str_handler lock_tracepoints[] = { @@ -1992,7 +1988,7 @@ static int __cmd_report(bool display_info) setup_pager(); if (display_info) /* used for info subcommand */ - err = dump_info(); + dump_info(); else { combine_result(); sort_result(); @@ -2578,9 +2574,9 @@ int cmd_lock(int argc, const char **argv) const struct option info_options[] = { OPT_BOOLEAN('t', "threads", &info_threads, - "dump thread list in perf.data"), + "dump the thread list in perf.data"), OPT_BOOLEAN('m', "map", &info_map, - "map of lock instances (address:name table)"), + "dump the map of lock instances (address:name table)"), OPT_PARENT(lock_options) }; @@ -2694,6 +2690,13 @@ int cmd_lock(int argc, const char **argv) if (argc) usage_with_options(info_usage, info_options); } + + /* If neither threads nor map requested, display both */ + if (!info_threads && !info_map) { + info_threads = true; + info_map = true; + } + /* recycling report_lock_ops */ trace_handler = &report_lock_ops; rc = __cmd_report(true); -- 2.44.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] perf lock info: Display both map and thread by default 2024-05-13 9:14 ` [PATCH 1/1] perf lock info: Display both map and thread by default Nick Forrington @ 2024-06-03 9:37 ` Nick Forrington 2024-06-04 22:17 ` Namhyung Kim 0 siblings, 1 reply; 4+ messages in thread From: Nick Forrington @ 2024-06-03 9:37 UTC (permalink / raw) To: linux-kernel, linux-perf-users, namhyung Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan Is this ok to be applied now? Thanks, Nick On 13/05/2024 10:14, Nick Forrington wrote: > Change "perf lock info" argument handling to: > > Display both map and thread info (rather than an error) when neither are > specified. > > Display both map and thread info (rather than just thread info) when > both are requested. > > Signed-off-by: Nick Forrington <nick.forrington@arm.com> > --- > tools/perf/Documentation/perf-lock.txt | 4 ++-- > tools/perf/builtin-lock.c | 27 ++++++++++++++------------ > 2 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt > index f5938d616d75..57a940399de0 100644 > --- a/tools/perf/Documentation/perf-lock.txt > +++ b/tools/perf/Documentation/perf-lock.txt > @@ -111,11 +111,11 @@ INFO OPTIONS > > -t:: > --threads:: > - dump thread list in perf.data > + dump only the thread list in perf.data > > -m:: > --map:: > - dump map of lock instances (address:name table) > + dump only the map of lock instances (address:name table) > > > CONTENTION OPTIONS > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c > index 230461280e45..7eb115aeb927 100644 > --- a/tools/perf/builtin-lock.c > +++ b/tools/perf/builtin-lock.c > @@ -1477,20 +1477,16 @@ static void dump_map(void) > fprintf(lock_output, " %#llx: %s\n", (unsigned long long)st->addr, st->name); > } > > -static int dump_info(void) > +static void dump_info(void) > { > - int rc = 0; > - > if (info_threads) > dump_threads(); > - else if (info_map) > + > + if (info_map) { > + if (info_threads) > + fputc('\n', lock_output); > dump_map(); > - else { > - rc = -1; > - pr_err("Unknown type of information\n"); > } > - > - return rc; > } > > static const struct evsel_str_handler lock_tracepoints[] = { > @@ -1992,7 +1988,7 @@ static int __cmd_report(bool display_info) > > setup_pager(); > if (display_info) /* used for info subcommand */ > - err = dump_info(); > + dump_info(); > else { > combine_result(); > sort_result(); > @@ -2578,9 +2574,9 @@ int cmd_lock(int argc, const char **argv) > > const struct option info_options[] = { > OPT_BOOLEAN('t', "threads", &info_threads, > - "dump thread list in perf.data"), > + "dump the thread list in perf.data"), > OPT_BOOLEAN('m', "map", &info_map, > - "map of lock instances (address:name table)"), > + "dump the map of lock instances (address:name table)"), > OPT_PARENT(lock_options) > }; > > @@ -2694,6 +2690,13 @@ int cmd_lock(int argc, const char **argv) > if (argc) > usage_with_options(info_usage, info_options); > } > + > + /* If neither threads nor map requested, display both */ > + if (!info_threads && !info_map) { > + info_threads = true; > + info_map = true; > + } > + > /* recycling report_lock_ops */ > trace_handler = &report_lock_ops; > rc = __cmd_report(true); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] perf lock info: Display both map and thread by default 2024-06-03 9:37 ` Nick Forrington @ 2024-06-04 22:17 ` Namhyung Kim 0 siblings, 0 replies; 4+ messages in thread From: Namhyung Kim @ 2024-06-04 22:17 UTC (permalink / raw) To: Nick Forrington Cc: linux-kernel, linux-perf-users, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan Hello, On Mon, Jun 3, 2024 at 2:38 AM Nick Forrington <nick.forrington@arm.com> wrote: > > Is this ok to be applied now? Sorry for the delay. Yep, it's now applied to perf-tools-next. Thanks, Namhyung > > On 13/05/2024 10:14, Nick Forrington wrote: > > Change "perf lock info" argument handling to: > > > > Display both map and thread info (rather than an error) when neither are > > specified. > > > > Display both map and thread info (rather than just thread info) when > > both are requested. > > > > Signed-off-by: Nick Forrington <nick.forrington@arm.com> > > --- > > tools/perf/Documentation/perf-lock.txt | 4 ++-- > > tools/perf/builtin-lock.c | 27 ++++++++++++++------------ > > 2 files changed, 17 insertions(+), 14 deletions(-) > > > > diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt > > index f5938d616d75..57a940399de0 100644 > > --- a/tools/perf/Documentation/perf-lock.txt > > +++ b/tools/perf/Documentation/perf-lock.txt > > @@ -111,11 +111,11 @@ INFO OPTIONS > > > > -t:: > > --threads:: > > - dump thread list in perf.data > > + dump only the thread list in perf.data > > > > -m:: > > --map:: > > - dump map of lock instances (address:name table) > > + dump only the map of lock instances (address:name table) > > > > > > CONTENTION OPTIONS > > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c > > index 230461280e45..7eb115aeb927 100644 > > --- a/tools/perf/builtin-lock.c > > +++ b/tools/perf/builtin-lock.c > > @@ -1477,20 +1477,16 @@ static void dump_map(void) > > fprintf(lock_output, " %#llx: %s\n", (unsigned long long)st->addr, st->name); > > } > > > > -static int dump_info(void) > > +static void dump_info(void) > > { > > - int rc = 0; > > - > > if (info_threads) > > dump_threads(); > > - else if (info_map) > > + > > + if (info_map) { > > + if (info_threads) > > + fputc('\n', lock_output); > > dump_map(); > > - else { > > - rc = -1; > > - pr_err("Unknown type of information\n"); > > } > > - > > - return rc; > > } > > > > static const struct evsel_str_handler lock_tracepoints[] = { > > @@ -1992,7 +1988,7 @@ static int __cmd_report(bool display_info) > > > > setup_pager(); > > if (display_info) /* used for info subcommand */ > > - err = dump_info(); > > + dump_info(); > > else { > > combine_result(); > > sort_result(); > > @@ -2578,9 +2574,9 @@ int cmd_lock(int argc, const char **argv) > > > > const struct option info_options[] = { > > OPT_BOOLEAN('t', "threads", &info_threads, > > - "dump thread list in perf.data"), > > + "dump the thread list in perf.data"), > > OPT_BOOLEAN('m', "map", &info_map, > > - "map of lock instances (address:name table)"), > > + "dump the map of lock instances (address:name table)"), > > OPT_PARENT(lock_options) > > }; > > > > @@ -2694,6 +2690,13 @@ int cmd_lock(int argc, const char **argv) > > if (argc) > > usage_with_options(info_usage, info_options); > > } > > + > > + /* If neither threads nor map requested, display both */ > > + if (!info_threads && !info_map) { > > + info_threads = true; > > + info_map = true; > > + } > > + > > /* recycling report_lock_ops */ > > trace_handler = &report_lock_ops; > > rc = __cmd_report(true); ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-04 22:17 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-13 9:14 [PATCH v4 0/1] Perf lock improvements Nick Forrington 2024-05-13 9:14 ` [PATCH 1/1] perf lock info: Display both map and thread by default Nick Forrington 2024-06-03 9:37 ` Nick Forrington 2024-06-04 22:17 ` 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).