perf mem: Rework command option handling

Split the common option and ones for record or report.  Otherwise -U in
the record option cannot be used because it clashes with in the common
(or report) option.  Also rename report_events() to __cmd_report() to
follow the convention and to be sync with the record part.

Also set the flag PARSE_OPT_STOP_AT_NON_OPTION for the common option so
that it can show the help message in the subcommand like below:

  $ perf mem record -h

   Usage: perf mem record [<options>] [<command>]
      or: perf mem record [<options>] -- <command> [<options>]

      -C, --cpu <cpu>       list of cpus to profile
      -e, --event <event>   event selector. use 'perf mem record -e list' to list available events
      -f, --force           don't complain, do it
      -K, --all-kernel      collect only kernel level data
      -p, --phys-data       Record/Report sample physical addresses
      -t, --type <type>     memory operations(load,store) Default load,store
      -U, --all-user        collect only user level data
      -v, --verbose         be more verbose (show counter open errors, etc)
          --data-page-size  Record/Report sample data address page size
          --ldlat <n>       mem-loads latency

Cc: Aditya Gupta <adityag@linux.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: https://lore.kernel.org/r/20240731235505.710436-4-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 93413cfcd585af532966a2f73b1a7ad64c4dd587..819edaf6b1df399b9e8fcb20bf8862b3fd9264df 100644 (file)
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -34,6 +34,8 @@ struct perf_mem {
        bool                    force;
        bool                    phys_addr;
        bool                    data_page_size;
+       bool                    all_kernel;
+       bool                    all_user;
        int                     operation;
        const char              *cpu_list;
        DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
@@ -62,33 +64,19 @@ static int parse_record_events(const struct option *opt,
        return 0;
 }
 
-static const char * const __usage[] = {
-       "perf mem record [<options>] [<command>]",
-       "perf mem record [<options>] -- <command> [<options>]",
-       NULL
-};
-
-static const char * const *record_mem_usage = __usage;
-
-static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
+static int __cmd_record(int argc, const char **argv, struct perf_mem *mem,
+                       const struct option *options)
 {
        int rec_argc, i = 0, j;
        int start, end;
        const char **rec_argv;
        int ret;
-       bool all_user = false, all_kernel = false;
        struct perf_mem_event *e;
        struct perf_pmu *pmu;
-       struct option options[] = {
-       OPT_CALLBACK('e', "event", &mem, "event",
-                    "event selector. use 'perf mem record -e list' to list available events",
-                    parse_record_events),
-       OPT_UINTEGER(0, "ldlat", &perf_mem_events__loads_ldlat, "mem-loads latency"),
-       OPT_INCR('v', "verbose", &verbose,
-                "be more verbose (show counter open errors, etc)"),
-       OPT_BOOLEAN('U', "all-user", &all_user, "collect only user level data"),
-       OPT_BOOLEAN('K', "all-kernel", &all_kernel, "collect only kernel level data"),
-       OPT_END()
+       const char * const record_usage[] = {
+               "perf mem record [<options>] [<command>]",
+               "perf mem record [<options>] -- <command> [<options>]",
+               NULL
        };
 
        pmu = perf_mem_events_find_pmu();
@@ -102,7 +90,7 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
                return -1;
        }
 
-       argc = parse_options(argc, argv, options, record_mem_usage,
+       argc = parse_options(argc, argv, options, record_usage,
                             PARSE_OPT_KEEP_UNKNOWN);
 
        /* Max number of arguments multiplied by number of PMUs that can support them. */
@@ -158,10 +146,10 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
                goto out;
        end = i;
 
-       if (all_user)
+       if (mem->all_user)
                rec_argv[i++] = "--all-user";
 
-       if (all_kernel)
+       if (mem->all_kernel)
                rec_argv[i++] = "--all-kernel";
 
        if (mem->cpu_list) {
@@ -319,6 +307,7 @@ out_delete:
        perf_session__delete(session);
        return ret;
 }
+
 static char *get_sort_order(struct perf_mem *mem)
 {
        bool has_extra_options = (mem->phys_addr | mem->data_page_size) ? true : false;
@@ -346,11 +335,19 @@ static char *get_sort_order(struct perf_mem *mem)
        return strdup(sort);
 }
 
-static int report_events(int argc, const char **argv, struct perf_mem *mem)
+static int __cmd_report(int argc, const char **argv, struct perf_mem *mem,
+                       const struct option *options)
 {
        const char **rep_argv;
        int ret, i = 0, j, rep_argc;
        char *new_sort_order;
+       const char * const report_usage[] = {
+               "perf mem report [<options>]",
+               NULL
+       };
+
+       argc = parse_options(argc, argv, options, report_usage,
+                            PARSE_OPT_KEEP_UNKNOWN);
 
        if (mem->dump_raw)
                return report_raw_events(mem);
@@ -368,7 +365,7 @@ static int report_events(int argc, const char **argv, struct perf_mem *mem)
        if (new_sort_order)
                rep_argv[i++] = new_sort_order;
 
-       for (j = 1; j < argc; j++, i++)
+       for (j = 0; j < argc; j++, i++)
                rep_argv[i] = argv[j];
 
        ret = cmd_report(i, rep_argv);
@@ -475,22 +472,36 @@ int cmd_mem(int argc, const char **argv)
        OPT_CALLBACK('t', "type", &mem.operation,
                   "type", "memory operations(load,store) Default load,store",
                    parse_mem_ops),
+       OPT_STRING('C', "cpu", &mem.cpu_list, "cpu",
+                  "list of cpus to profile"),
+       OPT_BOOLEAN('f', "force", &mem.force, "don't complain, do it"),
+       OPT_INCR('v', "verbose", &verbose,
+                "be more verbose (show counter open errors, etc)"),
+       OPT_BOOLEAN('p', "phys-data", &mem.phys_addr, "Record/Report sample physical addresses"),
+       OPT_BOOLEAN(0, "data-page-size", &mem.data_page_size, "Record/Report sample data address page size"),
+       OPT_END()
+       };
+       const struct option record_options[] = {
+       OPT_CALLBACK('e', "event", &mem, "event",
+                    "event selector. use 'perf mem record -e list' to list available events",
+                    parse_record_events),
+       OPT_UINTEGER(0, "ldlat", &perf_mem_events__loads_ldlat, "mem-loads latency"),
+       OPT_BOOLEAN('U', "all-user", &mem.all_user, "collect only user level data"),
+       OPT_BOOLEAN('K', "all-kernel", &mem.all_kernel, "collect only kernel level data"),
+       OPT_PARENT(mem_options)
+       };
+       const struct option report_options[] = {
        OPT_BOOLEAN('D', "dump-raw-samples", &mem.dump_raw,
                    "dump raw samples in ASCII"),
        OPT_BOOLEAN('U', "hide-unresolved", &mem.hide_unresolved,
                    "Only display entries resolved to a symbol"),
        OPT_STRING('i', "input", &input_name, "file",
                   "input file name"),
-       OPT_STRING('C', "cpu", &mem.cpu_list, "cpu",
-                  "list of cpus to profile"),
        OPT_STRING_NOEMPTY('x', "field-separator", &symbol_conf.field_sep,
                   "separator",
                   "separator for columns, no spaces will be added"
                   " between columns '.' is reserved."),
-       OPT_BOOLEAN('f', "force", &mem.force, "don't complain, do it"),
-       OPT_BOOLEAN('p', "phys-data", &mem.phys_addr, "Record/Report sample physical addresses"),
-       OPT_BOOLEAN(0, "data-page-size", &mem.data_page_size, "Record/Report sample data address page size"),
-       OPT_END()
+       OPT_PARENT(mem_options)
        };
        const char *const mem_subcommands[] = { "record", "report", NULL };
        const char *mem_usage[] = {
@@ -499,7 +510,7 @@ int cmd_mem(int argc, const char **argv)
        };
 
        argc = parse_options_subcommand(argc, argv, mem_options, mem_subcommands,
-                                       mem_usage, PARSE_OPT_KEEP_UNKNOWN);
+                                       mem_usage, PARSE_OPT_STOP_AT_NON_OPTION);
 
        if (!argc || !(strncmp(argv[0], "rec", 3) || mem.operation))
                usage_with_options(mem_usage, mem_options);
@@ -512,9 +523,9 @@ int cmd_mem(int argc, const char **argv)
        }
 
        if (strlen(argv[0]) > 2 && strstarts("record", argv[0]))
-               return __cmd_record(argc, argv, &mem);
+               return __cmd_record(argc, argv, &mem, record_options);
        else if (strlen(argv[0]) > 2 && strstarts("report", argv[0]))
-               return report_events(argc, argv, &mem);
+               return __cmd_report(argc, argv, &mem, report_options);
        else
                usage_with_options(mem_usage, mem_options);