Em Wed, Nov 01, 2023 at 11:00:42PM -0700, Namhyung Kim escreveu:
On Wed, Nov 1, 2023 at 7:35 AM Nick Forrington <nick.forrington@xxxxxxx> wrote:Yeah, that would be a better approach, I think.
Maybe we can default to display both. :)
On 31/10/2023 15:38, Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 31, 2023 at 12:05:25PM +0000, Nick Forrington escreveu:There isn't an obvious choice (to me) for which would be the default.
Improve error reporting for command line arguments.Shouldn't one of them be the default so that we type less for the most
Display error/usage if neither --map or --thread are specified (rather
than a non user-friendly error "Unknown type of information").
Display error/usage if both --map and --thread are specified (rather
than ignoring "--map" and displaying only thread information).
common usage?
- Arnaldo
Both options display completely different data/outputs, so I think it
makes sense to be explicit about which data is requested.
- Arnaldo
Thanks,
Namhyung
An alternative could be to use sub-commands e.g. "perf lock info
threads" or just "perf lock threads", although changing the existing
options would be more disruptive.
Cheers,
Nick
Signed-off-by: Nick Forrington <nick.forrington@xxxxxxx>
---
tools/perf/builtin-lock.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 3aa8ba5ad928..cf29f648d291 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2021,6 +2021,27 @@ static int check_lock_report_options(const struct option *options,
return 0;
}
+static int check_lock_info_options(const struct option *options,
+ const char * const *usage)
+{
+ if (!info_map && !info_threads) {
+ pr_err("Requires one of --map or --threads\n");
+ parse_options_usage(usage, options, "map", 0);
+ parse_options_usage(NULL, options, "threads", 0);
+ return -1;
+
+ }
+
+ if (info_map && info_threads) {
+ pr_err("Cannot show map and threads together\n");
+ parse_options_usage(usage, options, "map", 0);
+ parse_options_usage(NULL, options, "threads", 0);
+ return -1;
+ }
+
+ return 0;
+}
+
static int check_lock_contention_options(const struct option *options,
const char * const *usage)
@@ -2709,6 +2730,10 @@ int cmd_lock(int argc, const char **argv)
if (argc)
usage_with_options(info_usage, info_options);
}
+
+ if (check_lock_info_options(info_options, info_usage) < 0)
+ return -1;
+
/* recycling report_lock_ops */
trace_handler = &report_lock_ops;
rc = __cmd_report(true);
--
2.42.0