* [PATCH v2 0/1] Perf lock improvements @ 2024-05-02 9:14 Nick Forrington 2024-05-02 9:14 ` [PATCH v2 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-02 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. 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 | 22 +++++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) -- 2.44.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] perf lock info: Display both map and thread by default 2024-05-02 9:14 [PATCH v2 0/1] Perf lock improvements Nick Forrington @ 2024-05-02 9:14 ` Nick Forrington 2024-05-02 18:32 ` Namhyung Kim 0 siblings, 1 reply; 4+ messages in thread From: Nick Forrington @ 2024-05-02 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 | 22 +++++++++++++++++----- 2 files changed, 19 insertions(+), 7 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..86c68c39aac0 100644 --- a/tools/perf/builtin-lock.c +++ b/tools/perf/builtin-lock.c @@ -1483,11 +1483,16 @@ static int dump_info(void) if (info_threads) dump_threads(); - else if (info_map) + + if (info_map) { + if (info_threads) + fputc('\n', lock_output); dump_map(); - else { + } + + if (!info_threads && !info_map) { rc = -1; - pr_err("Unknown type of information\n"); + pr_err("No lock info specified\n"); } return rc; @@ -2578,9 +2583,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 only the thread list in perf.data"), OPT_BOOLEAN('m', "map", &info_map, - "map of lock instances (address:name table)"), + "dump only the map of lock instances (address:name table)"), OPT_PARENT(lock_options) }; @@ -2694,6 +2699,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 v2 1/1] perf lock info: Display both map and thread by default 2024-05-02 9:14 ` [PATCH v2 1/1] perf lock info: Display both map and thread by default Nick Forrington @ 2024-05-02 18:32 ` Namhyung Kim 2024-05-08 9:15 ` Nick Forrington 0 siblings, 1 reply; 4+ messages in thread From: Namhyung Kim @ 2024-05-02 18:32 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 Thu, May 2, 2024 at 2:15 AM Nick Forrington <nick.forrington@arm.com> 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 | 22 +++++++++++++++++----- > 2 files changed, 19 insertions(+), 7 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) But your change allows both thread and map info to be printed, right? > > > CONTENTION OPTIONS > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c > index 230461280e45..86c68c39aac0 100644 > --- a/tools/perf/builtin-lock.c > +++ b/tools/perf/builtin-lock.c > @@ -1483,11 +1483,16 @@ static int dump_info(void) > > if (info_threads) > dump_threads(); > - else if (info_map) > + > + if (info_map) { > + if (info_threads) > + fputc('\n', lock_output); > dump_map(); > - else { > + } > + > + if (!info_threads && !info_map) { > rc = -1; > - pr_err("Unknown type of information\n"); > + pr_err("No lock info specified\n"); > } I guess you made this condition impossible. > > return rc; > @@ -2578,9 +2583,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 only the thread list in perf.data"), > OPT_BOOLEAN('m', "map", &info_map, > - "map of lock instances (address:name table)"), > + "dump only the map of lock instances (address:name table)"), Ditto, I think we can drop the 'only' part. Thanks, Namhyung > OPT_PARENT(lock_options) > }; > > @@ -2694,6 +2699,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 [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] perf lock info: Display both map and thread by default 2024-05-02 18:32 ` Namhyung Kim @ 2024-05-08 9:15 ` Nick Forrington 0 siblings, 0 replies; 4+ messages in thread From: Nick Forrington @ 2024-05-08 9:15 UTC (permalink / raw) To: Namhyung Kim Cc: linux-kernel, linux-perf-users, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Liang, Kan On 02/05/2024 19:32, Namhyung Kim wrote: > Hello, > > On Thu, May 2, 2024 at 2:15 AM Nick Forrington<nick.forrington@arm.com> 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 | 22 +++++++++++++++++----- >> 2 files changed, 19 insertions(+), 7 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) > But your change allows both thread and map info to be printed, right? Yes >> CONTENTION OPTIONS >> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c >> index 230461280e45..86c68c39aac0 100644 >> --- a/tools/perf/builtin-lock.c >> +++ b/tools/perf/builtin-lock.c >> @@ -1483,11 +1483,16 @@ static int dump_info(void) >> >> if (info_threads) >> dump_threads(); >> - else if (info_map) >> + >> + if (info_map) { >> + if (info_threads) >> + fputc('\n', lock_output); >> dump_map(); >> - else { >> + } >> + >> + if (!info_threads && !info_map) { >> rc = -1; >> - pr_err("Unknown type of information\n"); >> + pr_err("No lock info specified\n"); >> } > I guess you made this condition impossible. Yes. (I kept/updated the error handling for potential future errors) >> return rc; >> @@ -2578,9 +2583,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 only the thread list in perf.data"), >> OPT_BOOLEAN('m', "map", &info_map, >> - "map of lock instances (address:name table)"), >> + "dump only the map of lock instances (address:name table)"), > Ditto, I think we can drop the 'only' part. Ok, I'll submit a new version with this removed > Thanks, > Namhyung > > >> OPT_PARENT(lock_options) >> }; >> >> @@ -2694,6 +2699,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 [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-08 9:15 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-02 9:14 [PATCH v2 0/1] Perf lock improvements Nick Forrington 2024-05-02 9:14 ` [PATCH v2 1/1] perf lock info: Display both map and thread by default Nick Forrington 2024-05-02 18:32 ` Namhyung Kim 2024-05-08 9:15 ` Nick Forrington
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).