Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1572283 > unrolled thread

[PATCH 3/5] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/5] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/ Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2017-02-02 12:20 +0100
    Re: [PATCH 3/5] perf/sdt/x86: Move OP parser to  tools/perf/arch/x86/ Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-07 04:20 +0100
      Re: [PATCH 3/5] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/ Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2017-02-07 06:30 +0100

#1572283 — [PATCH 3/5] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/

SDT marker argument is in N@OP format. N is the size of argument and
OP is the actual assembly operand. OP is arch dependent component and
hence it's parsing logic also should be placed under tools/perf/arch/.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/arch/x86/util/perf_regs.c |  93 ++++++++++++++++++++++++-
 tools/perf/util/perf_regs.c          |   9 ++-
 tools/perf/util/perf_regs.h          |   7 +-
 tools/perf/util/probe-file.c         | 127 +++++++++--------------------------
 4 files changed, 134 insertions(+), 102 deletions(-)

diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index d8a8dcf..34fcb0d 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -3,6 +3,7 @@
 #include "../../perf.h"
 #include "../../util/util.h"
 #include "../../util/perf_regs.h"
+#include "../../util/debug.h"
 
 const struct sample_reg sample_reg_masks[] = {
 	SMPL_REG(AX, PERF_REG_X86_AX),
@@ -87,7 +88,16 @@ static const struct sdt_name_reg sdt_reg_renamings[] = {
 	SDT_NAME_REG_END,
 };
 
-int sdt_rename_register(char **pdesc, char *old_name)
+bool arch_sdt_probe_arg_supp(void)
+{
+	return true;
+}
+
+/*
+ * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
+ * registers before filling the uprobe tracer interface.
+ */
+static int sdt_rename_register(char **pdesc, char *old_name)
 {
 	const struct sdt_name_reg *rnames = sdt_reg_renamings;
 	char *new_desc, *old_desc = *pdesc;
@@ -129,3 +139,84 @@ int sdt_rename_register(char **pdesc, char *old_name)
 
 	return 0;
 }
+
+/*
+ * x86 specific implementation
+ * return value:
+ *	<0 : error
+ *	 0 : success
+ *	>0 : skip
+ */
+int arch_sdt_probe_parse_op(char **desc, const char **prefix)
+{
+	char *tmp;
+	int ret = 0;
+
+	/*
+	 * The uprobe tracer format does not support all the addressing
+	 * modes (notably: in x86 the scaled mode); so, we detect ','
+	 * characters, if there is just one, there is no use converting
+	 * the sdt arg into a uprobe one.
+	 *
+	 * Also it does not support constants; if we find one in the
+	 * current argument, let's skip the argument.
+	 */
+	if (strchr(*desc, ',') || strchr(*desc, '$')) {
+		pr_debug4("Skipping unsupported SDT argument; %s\n", *desc);
+		return 1;
+	}
+
+	/*
+	 * If the argument addressing mode is indirect, we must check
+	 * a few things...
+	 */
+	tmp = strchr(*desc, '(');
+	if (tmp) {
+		int j;
+
+		/*
+		 * ...if the addressing mode is indirect with a
+		 * positive offset (ex.: "1608(%ax)"), we need to add
+		 * a '+' prefix so as to be compliant with uprobe
+		 * format.
+		 */
+		if ((*desc)[0] != '+' && (*desc)[0] != '-')
+			*prefix = ((*desc)[0] == '(') ? "+0" : "+";
+
+		/*
+		 * ...or if the addressing mode is indirect with a symbol
+		 * as offset, the argument will not be supported by
+		 * the uprobe tracer format; so, let's skip this one.
+		 */
+		for (j = 0; j < tmp - *desc; j++) {
+			if ((*desc)[j] != '+' && (*desc)[j] != '-' &&
+			    !isdigit((*desc)[j])) {
+				pr_debug4("Skipping unsupported SDT argument; "
+					"%s\n", *desc);
+				return 1;
+			}
+		}
+	}
+
+	/*
+	 * The uprobe parser does not support all gas register names;
+	 * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+	 * the loop below looks for the register names (starting with
+	 * a '%' and tries to perform the needed renamings.
+	 */
+	tmp = strchr(*desc, '%');
+	while (tmp) {
+		size_t offset = tmp - *desc;
+
+		ret = sdt_rename_register(desc, *desc + offset);
+		if (ret < 0)
+			return ret;
+
+		/*
+		 * The *desc pointer might have changed; so, let's not
+		 * try to reuse tmp for next lookup
+		 */
+		tmp = strchr(*desc + offset + 1, '%');
+	}
+	return 0;
+}
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index a37e593..f2b3d0d 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -6,8 +6,13 @@ const struct sample_reg __weak sample_reg_masks[] = {
 	SMPL_REG_END
 };
 
-int __weak sdt_rename_register(char **pdesc __maybe_unused,
-			char *old_name __maybe_unused)
+bool __weak arch_sdt_probe_arg_supp(void)
+{
+	return false;
+}
+
+int __weak arch_sdt_probe_parse_op(char **op_ptr __maybe_unused,
+				   const char **prefix __maybe_unused)
 {
 	return 0;
 }
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 7544a15..86a2961 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -15,11 +15,8 @@ struct sample_reg {
 
 extern const struct sample_reg sample_reg_masks[];
 
-/*
- * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
- * registers before filling the uprobe tracer interface.
- */
-int sdt_rename_register(char **pdesc, char *old_name);
+bool arch_sdt_probe_arg_supp(void);
+int arch_sdt_probe_parse_op(char **op_ptr, const char **prefix);
 
 #ifdef HAVE_PERF_REGS_SUPPORT
 #include <perf_regs.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index d8a169e..38eca3c 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -693,6 +693,25 @@ static const char * const type_to_suffix[] = {
 	"", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
 };
 
+/*
+ * Isolate the string number and convert it into a decimal value;
+ * this will be an index to get suffix of the uprobe name (defining
+ * the type)
+ */
+static int sdt_probe_parse_n(char *n_ptr, const char **suffix)
+{
+	long type_idx;
+
+	type_idx = strtol(n_ptr, NULL, 10);
+	if (type_idx < -8 || type_idx > 8) {
+		pr_debug4("Failed to get a valid sdt type\n");
+		return -1;
+	}
+
+	*suffix = type_to_suffix[type_idx + 8];
+	return 0;
+}
+
 static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
 {
 	char *tmp, *desc = strdup(arg);
@@ -704,109 +723,29 @@ static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
 		return ret;
 	}
 
+	/*
+	 * Argument is in N@OP format. N is size of the argument and OP is
+	 * the actual assembly operand. N can be omitted; in that case
+	 * argument is just OP(without @).
+	 */
 	tmp = strchr(desc, '@');
 	if (tmp) {
-		long type_idx;
-		/*
-		 * Isolate the string number and convert it into a
-		 * binary value; this will be an index to get suffix
-		 * of the uprobe name (defining the type)
-		 */
 		tmp[0] = '\0';
-		type_idx = strtol(desc, NULL, 10);
-		/* Check that the conversion went OK */
-		if (type_idx == LONG_MIN || type_idx == LONG_MAX) {
-			pr_debug4("Failed to parse sdt type\n");
-			goto error;
-		}
-		/* Check that the converted value is OK */
-		if (type_idx < -8 || type_idx > 8) {
-			pr_debug4("Failed to get a valid sdt type\n");
-			goto error;
-		}
-		suffix = type_to_suffix[type_idx + 8];
-		/* Get rid of the sdt prefix which is now useless */
 		tmp++;
-		memmove(desc, tmp, strlen(tmp) + 1);
-	}
 
-	/*
-	 * The uprobe tracer format does not support all the
-	 * addressing modes (notably: in x86 the scaled mode); so, we
-	 * detect ',' characters, if there is just one, there is no
-	 * use converting the sdt arg into a uprobe one.
-	 */
-	if (strchr(desc, ',')) {
-		pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
-		goto out;
-	}
-
-	/*
-	 * If the argument addressing mode is indirect, we must check
-	 * a few things...
-	 */
-	tmp = strchr(desc, '(');
-	if (tmp) {
-		int j;
-
-		/*
-		 * ...if the addressing mode is indirect with a
-		 * positive offset (ex.: "1608(%ax)"), we need to add
-		 * a '+' prefix so as to be compliant with uprobe
-		 * format.
-		 */
-		if (desc[0] != '+' && desc[0] != '-')
-			prefix = "+";
-
-		/*
-		 * ...or if the addressing mode is indirect with a symbol
-		 * as offset, the argument will not be supported by
-		 * the uprobe tracer format; so, let's skip this one.
-		 */
-		for (j = 0; j < tmp - desc; j++) {
-			if (desc[j] != '+' && desc[j] != '-' &&
-				!isdigit(desc[j])) {
-				pr_debug4("Skipping unsupported SDT argument; "
-					"%s\n", desc);
-				goto out;
-			}
-		}
-	}
-
-	/*
-	 * The uprobe tracer format does not support constants; if we
-	 * find one in the current argument, let's skip the argument.
-	 */
-	if (strchr(desc, '$')) {
-		pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
-		goto out;
-	}
-
-	/*
-	 * The uprobe parser does not support all gas register names;
-	 * so, we have to replace them (ex. for x86_64: %rax -> %ax);
-	 * the loop below looks for the register names (starting with
-	 * a '%' and tries to perform the needed renamings.
-	 */
-	tmp = strchr(desc, '%');
-	while (tmp) {
-		size_t offset = tmp - desc;
-
-		ret = sdt_rename_register(&desc, desc + offset);
-		if (ret < 0)
+		if (sdt_probe_parse_n(desc, &suffix))
 			goto error;
-
-		/*
-		 * The desc pointer might have changed; so, let's not
-		 * try to reuse tmp for next lookup
-		 */
-		tmp = strchr(desc + offset + 1, '%');
+		memmove(desc, tmp, strlen(tmp) + 1);
 	}
 
-	if (strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
+	ret = arch_sdt_probe_parse_op(&desc, &prefix);
+	if (ret < 0)
+		goto error;
+
+	if (ret == 0 &&
+	    strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
 		goto error;
 
-out:
 	ret = 0;
 error:
 	free(desc);
@@ -829,7 +768,7 @@ static char *synthesize_sdt_probe_command(struct sdt_note *note,
 				sdt_note__get_addr(note)) < 0)
 		goto error;
 
-	if (!note->args)
+	if (!note->args || !arch_sdt_probe_arg_supp())
 		goto out;
 
 	if (note->args) {
-- 
2.9.3

[toc] | [next] | [standalone]

#1575353 — Re: [PATCH 3/5] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/

On Thu,  2 Feb 2017 16:41:41 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> SDT marker argument is in N@OP format. N is the size of argument and
> OP is the actual assembly operand. OP is arch dependent component and
> hence it's parsing logic also should be placed under tools/perf/arch/.
> 

Ok, I have one question. Is there any possibility that N is different
size of OP? e.g. 8@dil, in this case we will record whole rdi.
is that OK?

Thanks,

> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/arch/x86/util/perf_regs.c |  93 ++++++++++++++++++++++++-
>  tools/perf/util/perf_regs.c          |   9 ++-
>  tools/perf/util/perf_regs.h          |   7 +-
>  tools/perf/util/probe-file.c         | 127 +++++++++--------------------------
>  4 files changed, 134 insertions(+), 102 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> index d8a8dcf..34fcb0d 100644
> --- a/tools/perf/arch/x86/util/perf_regs.c
> +++ b/tools/perf/arch/x86/util/perf_regs.c
> @@ -3,6 +3,7 @@
>  #include "../../perf.h"
>  #include "../../util/util.h"
>  #include "../../util/perf_regs.h"
> +#include "../../util/debug.h"
>  
>  const struct sample_reg sample_reg_masks[] = {
>  	SMPL_REG(AX, PERF_REG_X86_AX),
> @@ -87,7 +88,16 @@ static const struct sdt_name_reg sdt_reg_renamings[] = {
>  	SDT_NAME_REG_END,
>  };
>  
> -int sdt_rename_register(char **pdesc, char *old_name)
> +bool arch_sdt_probe_arg_supp(void)
> +{
> +	return true;
> +}
> +
> +/*
> + * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
> + * registers before filling the uprobe tracer interface.
> + */
> +static int sdt_rename_register(char **pdesc, char *old_name)
>  {
>  	const struct sdt_name_reg *rnames = sdt_reg_renamings;
>  	char *new_desc, *old_desc = *pdesc;
> @@ -129,3 +139,84 @@ int sdt_rename_register(char **pdesc, char *old_name)
>  
>  	return 0;
>  }
> +
> +/*
> + * x86 specific implementation
> + * return value:
> + *	<0 : error
> + *	 0 : success
> + *	>0 : skip
> + */
> +int arch_sdt_probe_parse_op(char **desc, const char **prefix)
> +{
> +	char *tmp;
> +	int ret = 0;
> +
> +	/*
> +	 * The uprobe tracer format does not support all the addressing
> +	 * modes (notably: in x86 the scaled mode); so, we detect ','
> +	 * characters, if there is just one, there is no use converting
> +	 * the sdt arg into a uprobe one.
> +	 *
> +	 * Also it does not support constants; if we find one in the
> +	 * current argument, let's skip the argument.
> +	 */
> +	if (strchr(*desc, ',') || strchr(*desc, '$')) {
> +		pr_debug4("Skipping unsupported SDT argument; %s\n", *desc);
> +		return 1;
> +	}
> +
> +	/*
> +	 * If the argument addressing mode is indirect, we must check
> +	 * a few things...
> +	 */
> +	tmp = strchr(*desc, '(');
> +	if (tmp) {
> +		int j;
> +
> +		/*
> +		 * ...if the addressing mode is indirect with a
> +		 * positive offset (ex.: "1608(%ax)"), we need to add
> +		 * a '+' prefix so as to be compliant with uprobe
> +		 * format.
> +		 */
> +		if ((*desc)[0] != '+' && (*desc)[0] != '-')
> +			*prefix = ((*desc)[0] == '(') ? "+0" : "+";
> +
> +		/*
> +		 * ...or if the addressing mode is indirect with a symbol
> +		 * as offset, the argument will not be supported by
> +		 * the uprobe tracer format; so, let's skip this one.
> +		 */
> +		for (j = 0; j < tmp - *desc; j++) {
> +			if ((*desc)[j] != '+' && (*desc)[j] != '-' &&
> +			    !isdigit((*desc)[j])) {
> +				pr_debug4("Skipping unsupported SDT argument; "
> +					"%s\n", *desc);
> +				return 1;
> +			}
> +		}
> +	}
> +
> +	/*
> +	 * The uprobe parser does not support all gas register names;
> +	 * so, we have to replace them (ex. for x86_64: %rax -> %ax);
> +	 * the loop below looks for the register names (starting with
> +	 * a '%' and tries to perform the needed renamings.
> +	 */
> +	tmp = strchr(*desc, '%');
> +	while (tmp) {
> +		size_t offset = tmp - *desc;
> +
> +		ret = sdt_rename_register(desc, *desc + offset);
> +		if (ret < 0)
> +			return ret;
> +
> +		/*
> +		 * The *desc pointer might have changed; so, let's not
> +		 * try to reuse tmp for next lookup
> +		 */
> +		tmp = strchr(*desc + offset + 1, '%');
> +	}
> +	return 0;
> +}
> diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> index a37e593..f2b3d0d 100644
> --- a/tools/perf/util/perf_regs.c
> +++ b/tools/perf/util/perf_regs.c
> @@ -6,8 +6,13 @@ const struct sample_reg __weak sample_reg_masks[] = {
>  	SMPL_REG_END
>  };
>  
> -int __weak sdt_rename_register(char **pdesc __maybe_unused,
> -			char *old_name __maybe_unused)
> +bool __weak arch_sdt_probe_arg_supp(void)
> +{
> +	return false;
> +}
> +
> +int __weak arch_sdt_probe_parse_op(char **op_ptr __maybe_unused,
> +				   const char **prefix __maybe_unused)
>  {
>  	return 0;
>  }
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index 7544a15..86a2961 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -15,11 +15,8 @@ struct sample_reg {
>  
>  extern const struct sample_reg sample_reg_masks[];
>  
> -/*
> - * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
> - * registers before filling the uprobe tracer interface.
> - */
> -int sdt_rename_register(char **pdesc, char *old_name);
> +bool arch_sdt_probe_arg_supp(void);
> +int arch_sdt_probe_parse_op(char **op_ptr, const char **prefix);
>  
>  #ifdef HAVE_PERF_REGS_SUPPORT
>  #include <perf_regs.h>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index d8a169e..38eca3c 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -693,6 +693,25 @@ static const char * const type_to_suffix[] = {
>  	"", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
>  };
>  
> +/*
> + * Isolate the string number and convert it into a decimal value;
> + * this will be an index to get suffix of the uprobe name (defining
> + * the type)
> + */
> +static int sdt_probe_parse_n(char *n_ptr, const char **suffix)
> +{
> +	long type_idx;
> +
> +	type_idx = strtol(n_ptr, NULL, 10);
> +	if (type_idx < -8 || type_idx > 8) {
> +		pr_debug4("Failed to get a valid sdt type\n");
> +		return -1;
> +	}
> +
> +	*suffix = type_to_suffix[type_idx + 8];
> +	return 0;
> +}
> +
>  static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
>  {
>  	char *tmp, *desc = strdup(arg);
> @@ -704,109 +723,29 @@ static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
>  		return ret;
>  	}
>  
> +	/*
> +	 * Argument is in N@OP format. N is size of the argument and OP is
> +	 * the actual assembly operand. N can be omitted; in that case
> +	 * argument is just OP(without @).
> +	 */
>  	tmp = strchr(desc, '@');
>  	if (tmp) {
> -		long type_idx;
> -		/*
> -		 * Isolate the string number and convert it into a
> -		 * binary value; this will be an index to get suffix
> -		 * of the uprobe name (defining the type)
> -		 */
>  		tmp[0] = '\0';
> -		type_idx = strtol(desc, NULL, 10);
> -		/* Check that the conversion went OK */
> -		if (type_idx == LONG_MIN || type_idx == LONG_MAX) {
> -			pr_debug4("Failed to parse sdt type\n");
> -			goto error;
> -		}
> -		/* Check that the converted value is OK */
> -		if (type_idx < -8 || type_idx > 8) {
> -			pr_debug4("Failed to get a valid sdt type\n");
> -			goto error;
> -		}
> -		suffix = type_to_suffix[type_idx + 8];
> -		/* Get rid of the sdt prefix which is now useless */
>  		tmp++;
> -		memmove(desc, tmp, strlen(tmp) + 1);
> -	}
>  
> -	/*
> -	 * The uprobe tracer format does not support all the
> -	 * addressing modes (notably: in x86 the scaled mode); so, we
> -	 * detect ',' characters, if there is just one, there is no
> -	 * use converting the sdt arg into a uprobe one.
> -	 */
> -	if (strchr(desc, ',')) {
> -		pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
> -		goto out;
> -	}
> -
> -	/*
> -	 * If the argument addressing mode is indirect, we must check
> -	 * a few things...
> -	 */
> -	tmp = strchr(desc, '(');
> -	if (tmp) {
> -		int j;
> -
> -		/*
> -		 * ...if the addressing mode is indirect with a
> -		 * positive offset (ex.: "1608(%ax)"), we need to add
> -		 * a '+' prefix so as to be compliant with uprobe
> -		 * format.
> -		 */
> -		if (desc[0] != '+' && desc[0] != '-')
> -			prefix = "+";
> -
> -		/*
> -		 * ...or if the addressing mode is indirect with a symbol
> -		 * as offset, the argument will not be supported by
> -		 * the uprobe tracer format; so, let's skip this one.
> -		 */
> -		for (j = 0; j < tmp - desc; j++) {
> -			if (desc[j] != '+' && desc[j] != '-' &&
> -				!isdigit(desc[j])) {
> -				pr_debug4("Skipping unsupported SDT argument; "
> -					"%s\n", desc);
> -				goto out;
> -			}
> -		}
> -	}
> -
> -	/*
> -	 * The uprobe tracer format does not support constants; if we
> -	 * find one in the current argument, let's skip the argument.
> -	 */
> -	if (strchr(desc, '$')) {
> -		pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
> -		goto out;
> -	}
> -
> -	/*
> -	 * The uprobe parser does not support all gas register names;
> -	 * so, we have to replace them (ex. for x86_64: %rax -> %ax);
> -	 * the loop below looks for the register names (starting with
> -	 * a '%' and tries to perform the needed renamings.
> -	 */
> -	tmp = strchr(desc, '%');
> -	while (tmp) {
> -		size_t offset = tmp - desc;
> -
> -		ret = sdt_rename_register(&desc, desc + offset);
> -		if (ret < 0)
> +		if (sdt_probe_parse_n(desc, &suffix))
>  			goto error;
> -
> -		/*
> -		 * The desc pointer might have changed; so, let's not
> -		 * try to reuse tmp for next lookup
> -		 */
> -		tmp = strchr(desc + offset + 1, '%');
> +		memmove(desc, tmp, strlen(tmp) + 1);
>  	}
>  
> -	if (strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
> +	ret = arch_sdt_probe_parse_op(&desc, &prefix);
> +	if (ret < 0)
> +		goto error;
> +
> +	if (ret == 0 &&
> +	    strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
>  		goto error;
>  
> -out:
>  	ret = 0;
>  error:
>  	free(desc);
> @@ -829,7 +768,7 @@ static char *synthesize_sdt_probe_command(struct sdt_note *note,
>  				sdt_note__get_addr(note)) < 0)
>  		goto error;
>  
> -	if (!note->args)
> +	if (!note->args || !arch_sdt_probe_arg_supp())
>  		goto out;
>  
>  	if (note->args) {
> -- 
> 2.9.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1575393

Thanks Masami for the review.

On Tuesday 07 February 2017 08:41 AM, Masami Hiramatsu wrote:
> On Thu,  2 Feb 2017 16:41:41 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> SDT marker argument is in N@OP format. N is the size of argument and
>> OP is the actual assembly operand. OP is arch dependent component and
>> hence it's parsing logic also should be placed under tools/perf/arch/.
>>
> Ok, I have one question. Is there any possibility that N is different
> size of OP? e.g. 8@dil, in this case we will record whole rdi.
> is that OK?

By looking at list of markers on my x86 Fedora25 box, yes, it's possible
for case when register size used in OP is more than size specified by N.
For example, -4@68(%rbx). But I don't see any argument which specifies
higher size in N compared to size of register in OP, like you mentioned
in your example.

Ravi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web