Re: [PATCH 1/1] perf lock info: Display both map and thread by default

[Namhyung Kim]

Hello,

On Mon, Jun 3, 2024 at 2:38 AM Nick Forrington <nick.forrington@xxxxxxx> 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@xxxxxxx>
> > ---
> >   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);