[V2,1/3] perf parse-regs: Split parse_regs
diff mbox series

Message ID 1557865174-56264-1-git-send-email-kan.liang@linux.intel.com
State New
Headers show
Series
  • [V2,1/3] perf parse-regs: Split parse_regs
Related show

Commit Message

Liang, Kan May 14, 2019, 8:19 p.m. UTC
From: Kan Liang <kan.liang@linux.intel.com>

The available registers for --int-regs and --user-regs may be different,
e.g. XMM registers.

Split parse_regs into two dedicated functions for --int-regs and
--user-regs respectively.

Modify the warning message. "--user-regs=?" should be applied to show
the available registers for --user-regs.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

No changes since V1

 tools/perf/builtin-record.c          |  4 ++--
 tools/perf/util/parse-regs-options.c | 19 ++++++++++++++++---
 tools/perf/util/parse-regs-options.h |  3 ++-
 3 files changed, 20 insertions(+), 6 deletions(-)

Comments

Ravi Bangoria May 15, 2019, 6:49 a.m. UTC | #1
On 5/15/19 1:49 AM, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> The available registers for --int-regs and --user-regs may be different,
> e.g. XMM registers.
> 
> Split parse_regs into two dedicated functions for --int-regs and
> --user-regs respectively.
> 
> Modify the warning message. "--user-regs=?" should be applied to show
> the available registers for --user-regs.
> 
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---

For patch 1 and 2,
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>

Minor neat. Should we update document as well? May be something like:

  tools/perf/Documentation/perf-record.txt

  --user-regs::
  Similar to -I, but capture user registers at sample time. To list the available
  user registers use --user-regs=\?.

Ravi
Liang, Kan May 15, 2019, 2:15 p.m. UTC | #2
On 5/15/2019 2:49 AM, Ravi Bangoria wrote:
> 
> On 5/15/19 1:49 AM, kan.liang@linux.intel.com wrote:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> The available registers for --int-regs and --user-regs may be different,
>> e.g. XMM registers.
>>
>> Split parse_regs into two dedicated functions for --int-regs and
>> --user-regs respectively.
>>
>> Modify the warning message. "--user-regs=?" should be applied to show
>> the available registers for --user-regs.
>>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
> 
> For patch 1 and 2,
> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> 
> Minor neat. Should we update document as well? May be something like:
> 
>    tools/perf/Documentation/perf-record.txt
> 
>    --user-regs::
>    Similar to -I, but capture user registers at sample time. To list the available
>    user registers use --user-regs=\?.
>
Hi Ravi,

Thanks for test.

The change of document looks good.

Thanks,
Kan

Patch
diff mbox series

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 386e665..45172bb 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2029,10 +2029,10 @@  static struct option __record_options[] = {
 		    "use per-thread mmaps"),
 	OPT_CALLBACK_OPTARG('I', "intr-regs", &record.opts.sample_intr_regs, NULL, "any register",
 		    "sample selected machine registers on interrupt,"
-		    " use '-I?' to list register names", parse_regs),
+		    " use '-I?' to list register names", parse_intr_regs),
 	OPT_CALLBACK_OPTARG(0, "user-regs", &record.opts.sample_user_regs, NULL, "any register",
 		    "sample selected machine registers on interrupt,"
-		    " use '-I?' to list register names", parse_regs),
+		    " use '--user-regs=?' to list register names", parse_user_regs),
 	OPT_BOOLEAN(0, "running-time", &record.opts.running_time,
 		    "Record running/enabled time of read (:S) events"),
 	OPT_CALLBACK('k', "clockid", &record.opts,
diff --git a/tools/perf/util/parse-regs-options.c b/tools/perf/util/parse-regs-options.c
index 9cb187a..b21617f 100644
--- a/tools/perf/util/parse-regs-options.c
+++ b/tools/perf/util/parse-regs-options.c
@@ -5,8 +5,8 @@ 
 #include <subcmd/parse-options.h>
 #include "util/parse-regs-options.h"
 
-int
-parse_regs(const struct option *opt, const char *str, int unset)
+static int
+__parse_regs(const struct option *opt, const char *str, int unset, bool intr)
 {
 	uint64_t *mode = (uint64_t *)opt->value;
 	const struct sample_reg *r;
@@ -48,7 +48,8 @@  parse_regs(const struct option *opt, const char *str, int unset)
 					break;
 			}
 			if (!r->name) {
-				ui__warning("Unknown register \"%s\", check man page or run \"perf record -I?\"\n", s);
+				ui__warning("Unknown register \"%s\", check man page or run \"perf record %s?\"\n",
+					    s, intr ? "-I" : "--user-regs=");
 				goto error;
 			}
 
@@ -69,3 +70,15 @@  parse_regs(const struct option *opt, const char *str, int unset)
 	free(os);
 	return ret;
 }
+
+int
+parse_user_regs(const struct option *opt, const char *str, int unset)
+{
+	return __parse_regs(opt, str, unset, false);
+}
+
+int
+parse_intr_regs(const struct option *opt, const char *str, int unset)
+{
+	return __parse_regs(opt, str, unset, true);
+}
diff --git a/tools/perf/util/parse-regs-options.h b/tools/perf/util/parse-regs-options.h
index cdefb1a..2b23d25 100644
--- a/tools/perf/util/parse-regs-options.h
+++ b/tools/perf/util/parse-regs-options.h
@@ -2,5 +2,6 @@ 
 #ifndef _PERF_PARSE_REGS_OPTIONS_H
 #define _PERF_PARSE_REGS_OPTIONS_H 1
 struct option;
-int parse_regs(const struct option *opt, const char *str, int unset);
+int parse_user_regs(const struct option *opt, const char *str, int unset);
+int parse_intr_regs(const struct option *opt, const char *str, int unset);
 #endif /* _PERF_PARSE_REGS_OPTIONS_H */