[PATCH] perf probe: support signedness casting

[PATCH] perf probe: support signedness casting

[Naohiro Aota]

Perf-probe detects a variable's type and use the detected type to add new
probe. Then, kprobes prints its variable in hexadecimal format if the
variable is unsigned and prints in decimal if it is signed.

We sometimes want to see unsigned variable in decimal format (e.g.
sector_t or size_t). In that case, we need to investigate variable's
size manually to specify just signedness.

This patch add signedness casting support. By specifying "s" or "u" as a
type, perf-probe will investigate variable size as usual and use
the specified signedness.

Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
---
 tools/perf/util/probe-finder.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f2d9ff0..5c290c6 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 	char sbuf[STRERR_BUFSIZE];
 	int bsize, boffs, total;
 	int ret;
+	char sign;
 
 	/* TODO: check all types */
-	if (cast && strcmp(cast, "string") != 0) {
+	if (cast && strcmp(cast, "string") != 0 &&
+	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
 		/* Non string type is OK */
+		/* and respect signedness cast */
 		tvar->type = strdup(cast);
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
@@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
 
+	if (cast && (strcmp(cast, "u") == 0))
+		sign = 'u';
+	else if (cast && (strcmp(cast, "s") == 0))
+		sign = 's';
+	else
+		sign = die_is_signed_type(&type) ? 's' : 'u';
+
 	ret = dwarf_bytesize(&type);
 	if (ret <= 0)
 		/* No size ... try to use default type */
@@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
 		ret = MAX_BASIC_TYPE_BITS;
 	}
-	ret = snprintf(buf, 16, "%c%d",
-		       die_is_signed_type(&type) ? 's' : 'u', ret);
+	ret = snprintf(buf, 16, "%c%d", sign, ret);
 
 formatted:
 	if (ret < 0 || ret >= 16) {
-- 
2.7.3

Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH] perf probe: support signedness casting

[Masami Hiramatsu]

On Fri, 5 Aug 2016 14:33:53 +0900
Naohiro Aota <naohiro.aota@hgst.com> wrote:

> Perf-probe detects a variable's type and use the detected type to add new
> probe. Then, kprobes prints its variable in hexadecimal format if the
> variable is unsigned and prints in decimal if it is signed.
> 
> We sometimes want to see unsigned variable in decimal format (e.g.
> sector_t or size_t). In that case, we need to investigate variable's
> size manually to specify just signedness.
> 
> This patch add signedness casting support. By specifying "s" or "u" as a
> type, perf-probe will investigate variable size as usual and use
> the specified signedness.

OK, I could understand what the patch does from code. Please add an
example, and update tools/perf/Documentation/perf-probe.txt too.

Thank you,

> 
> Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> ---
>  tools/perf/util/probe-finder.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f2d9ff0..5c290c6 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  	char sbuf[STRERR_BUFSIZE];
>  	int bsize, boffs, total;
>  	int ret;
> +	char sign;
>  
>  	/* TODO: check all types */
> -	if (cast && strcmp(cast, "string") != 0) {
> +	if (cast && strcmp(cast, "string") != 0 &&
> +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
>  		/* Non string type is OK */
> +		/* and respect signedness cast */
>  		tvar->type = strdup(cast);
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
> @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
>  
> +	if (cast && (strcmp(cast, "u") == 0))
> +		sign = 'u';
> +	else if (cast && (strcmp(cast, "s") == 0))
> +		sign = 's';
> +	else
> +		sign = die_is_signed_type(&type) ? 's' : 'u';
> +
>  	ret = dwarf_bytesize(&type);
>  	if (ret <= 0)
>  		/* No size ... try to use default type */
> @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
>  		ret = MAX_BASIC_TYPE_BITS;
>  	}
> -	ret = snprintf(buf, 16, "%c%d",
> -		       die_is_signed_type(&type) ? 's' : 'u', ret);
> +	ret = snprintf(buf, 16, "%c%d", sign, ret);
>  
>  formatted:
>  	if (ret < 0 || ret >= 16) {
> -- 
> 2.7.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] perf probe: support signedness casting

[Naohiro Aota]

2016-08-05 (金) の 18:10 +0900 に Masami Hiramatsu さんは書きました:
> On Fri, 5 Aug 2016 14:33:53 +0900
> Naohiro Aota <naohiro.aota@hgst.com> wrote:
> 
> > 
> > Perf-probe detects a variable's type and use the detected type to
> > add new
> > probe. Then, kprobes prints its variable in hexadecimal format if
> > the
> > variable is unsigned and prints in decimal if it is signed.
> > 
> > We sometimes want to see unsigned variable in decimal format (e.g.
> > sector_t or size_t). In that case, we need to investigate
> > variable's
> > size manually to specify just signedness.
> > 
> > This patch add signedness casting support. By specifying "s" or "u"
> > as a
> > type, perf-probe will investigate variable size as usual and use
> > the specified signedness.
> OK, I could understand what the patch does from code. Please add an
> example, and update tools/perf/Documentation/perf-probe.txt too.

Thanks for the review. I'm posting updated patch with type
descriptions.

> Thank you,
> 
> > 
> > 
> > Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> > ---
> >  tools/perf/util/probe-finder.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-finder.c
> > b/tools/perf/util/probe-finder.c
> > index f2d9ff0..5c290c6 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die
> > *vr_die,
> >  	char sbuf[STRERR_BUFSIZE];
> >  	int bsize, boffs, total;
> >  	int ret;
> > +	char sign;
> >  
> >  	/* TODO: check all types */
> > -	if (cast && strcmp(cast, "string") != 0) {
> > +	if (cast && strcmp(cast, "string") != 0 &&
> > +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
> >  		/* Non string type is OK */
> > +		/* and respect signedness cast */
> >  		tvar->type = strdup(cast);
> >  		return (tvar->type == NULL) ? -ENOMEM : 0;
> >  	}
> > @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die
> > *vr_die,
> >  		return (tvar->type == NULL) ? -ENOMEM : 0;
> >  	}
> >  
> > +	if (cast && (strcmp(cast, "u") == 0))
> > +		sign = 'u';
> > +	else if (cast && (strcmp(cast, "s") == 0))
> > +		sign = 's';
> > +	else
> > +		sign = die_is_signed_type(&type) ? 's' : 'u';
> > +
> >  	ret = dwarf_bytesize(&type);
> >  	if (ret <= 0)
> >  		/* No size ... try to use default type */
> > @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die
> > *vr_die,
> >  			dwarf_diename(&type),
> > MAX_BASIC_TYPE_BITS);
> >  		ret = MAX_BASIC_TYPE_BITS;
> >  	}
> > -	ret = snprintf(buf, 16, "%c%d",
> > -		       die_is_signed_type(&type) ? 's' : 'u',
> > ret);
> > +	ret = snprintf(buf, 16, "%c%d", sign, ret);
> >  
> >  formatted:
> >  	if (ret < 0 || ret >= 16) {
> > -- 
> > 2.7.3
> > 
> 
Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

[PATCH v2] perf probe: Support signedness casting

[Naohiro Aota]

Perf-probe detects a variable's type and use the detected type to add new
probe. Then, kprobes prints its variable in hexadecimal format if the
variable is unsigned and prints in decimal if it is signed.

We sometimes want to see unsigned variable in decimal format (i.e.
sector_t or size_t). In that case, we need to investigate variable's
size manually to specify just signedness.

This patch add signedness casting support. By specifying "s" or "u" as a
type, perf-probe will investigate variable size as usual and use
the specified signedness.

E.g. without this:

$ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
Added new event:
  probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
You can now use it in all perf tools, such as:
        perf record -e probe:submit_bio -aR sleep 1
$ cat trace_pipe|head
          dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
          dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
          dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
...
// need to investigate the variable size
$ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
Added new event:
  probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
You can now use it in all perf tools, such as:
        perf record -e probe:submit_bio -aR sleep 1

With this:

// just use "s" to cast its signedness
$ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
Added new event:
  probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
You can now use it in all perf tools, such as:
        perf record -e probe:submit_bio -aR sleep 1
$ cat trace_pipe|head
          dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
          dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
          dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208

This commit also update perf-probe.txt to describe "types". Most parts
are based on existing documentation: Documentation/trace/kprobetrace.txt

Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
---
 tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
 tools/perf/util/probe-finder.c          | 15 ++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 736da44..a23b124 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -176,10 +176,18 @@ Each probe argument follows below syntax.
 
 'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
 '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
-'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
+'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield are supported. (see TYPES for detail)
 
 On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
 
+TYPES
+-----
+Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
+String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
+Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
+
+ b<bit-width>@<bit-offset>/<container-size>
+
 LINE SYNTAX
 -----------
 Line range is described by following syntax.
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f2d9ff0..5c290c6 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 	char sbuf[STRERR_BUFSIZE];
 	int bsize, boffs, total;
 	int ret;
+	char sign;
 
 	/* TODO: check all types */
-	if (cast && strcmp(cast, "string") != 0) {
+	if (cast && strcmp(cast, "string") != 0 &&
+	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
 		/* Non string type is OK */
+		/* and respect signedness cast */
 		tvar->type = strdup(cast);
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
@@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
 
+	if (cast && (strcmp(cast, "u") == 0))
+		sign = 'u';
+	else if (cast && (strcmp(cast, "s") == 0))
+		sign = 's';
+	else
+		sign = die_is_signed_type(&type) ? 's' : 'u';
+
 	ret = dwarf_bytesize(&type);
 	if (ret <= 0)
 		/* No size ... try to use default type */
@@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
 		ret = MAX_BASIC_TYPE_BITS;
 	}
-	ret = snprintf(buf, 16, "%c%d",
-		       die_is_signed_type(&type) ? 's' : 'u', ret);
+	ret = snprintf(buf, 16, "%c%d", sign, ret);
 
 formatted:
 	if (ret < 0 || ret >= 16) {
-- 
2.7.3

Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH v2] perf probe: Support signedness casting

[Masami Hiramatsu]

On Fri, 5 Aug 2016 20:53:21 +0900
Naohiro Aota <naohiro.aota@hgst.com> wrote:

> Perf-probe detects a variable's type and use the detected type to add new
> probe. Then, kprobes prints its variable in hexadecimal format if the
> variable is unsigned and prints in decimal if it is signed.
> 
> We sometimes want to see unsigned variable in decimal format (i.e.
> sector_t or size_t). In that case, we need to investigate variable's
> size manually to specify just signedness.
> 
> This patch add signedness casting support. By specifying "s" or "u" as a
> type, perf-probe will investigate variable size as usual and use
> the specified signedness.
> 
> E.g. without this:
> 
> $ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> $ cat trace_pipe|head
>           dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
>           dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
>           dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
> ...
> // need to investigate the variable size
> $ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> 
> With this:
> 
> // just use "s" to cast its signedness
> $ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> $ cat trace_pipe|head
>           dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
>           dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
>           dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208
> 
> This commit also update perf-probe.txt to describe "types". Most parts
> are based on existing documentation: Documentation/trace/kprobetrace.txt
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> ---
>  tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
>  tools/perf/util/probe-finder.c          | 15 ++++++++++++---
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> index 736da44..a23b124 100644
> --- a/tools/perf/Documentation/perf-probe.txt
> +++ b/tools/perf/Documentation/perf-probe.txt
> @@ -176,10 +176,18 @@ Each probe argument follows below syntax.
>  
>  'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
>  '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
> -'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield are supported. (see TYPES for detail)

Hmm, have you added the 's' and 'u' here too ??

>  
>  On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
>  
> +TYPES
> +-----
> +Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.

So, not only the details, but also the brief information about the TYPE, there should be 's' and 'u'.

Other are good to me.

Thanks,

> +String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
> +
> + b<bit-width>@<bit-offset>/<container-size>
> +
>  LINE SYNTAX
>  -----------
>  Line range is described by following syntax.
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f2d9ff0..5c290c6 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  	char sbuf[STRERR_BUFSIZE];
>  	int bsize, boffs, total;
>  	int ret;
> +	char sign;
>  
>  	/* TODO: check all types */
> -	if (cast && strcmp(cast, "string") != 0) {
> +	if (cast && strcmp(cast, "string") != 0 &&
> +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
>  		/* Non string type is OK */
> +		/* and respect signedness cast */
>  		tvar->type = strdup(cast);
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
> @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
>  
> +	if (cast && (strcmp(cast, "u") == 0))
> +		sign = 'u';
> +	else if (cast && (strcmp(cast, "s") == 0))
> +		sign = 's';
> +	else
> +		sign = die_is_signed_type(&type) ? 's' : 'u';
> +
>  	ret = dwarf_bytesize(&type);
>  	if (ret <= 0)
>  		/* No size ... try to use default type */
> @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
>  		ret = MAX_BASIC_TYPE_BITS;
>  	}
> -	ret = snprintf(buf, 16, "%c%d",
> -		       die_is_signed_type(&type) ? 's' : 'u', ret);
> +	ret = snprintf(buf, 16, "%c%d", sign, ret);
>  
>  formatted:
>  	if (ret < 0 || ret >= 16) {


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH v3] perf probe: Support signedness casting

[Naohiro Aota]

Perf-probe detects a variable's type and use the detected type to add new
probe. Then, kprobes prints its variable in hexadecimal format if the
variable is unsigned and prints in decimal if it is signed.

We sometimes want to see unsigned variable in decimal format (i.e.
sector_t or size_t). In that case, we need to investigate variable's
size manually to specify just signedness.

This patch add signedness casting support. By specifying "s" or "u" as a
type, perf-probe will investigate variable size as usual and use
the specified signedness.

E.g. without this:

$ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
Added new event:
  probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
You can now use it in all perf tools, such as:
        perf record -e probe:submit_bio -aR sleep 1
$ cat trace_pipe|head
          dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
          dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
          dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
...
// need to investigate the variable size
$ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
Added new event:
  probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
You can now use it in all perf tools, such as:
        perf record -e probe:submit_bio -aR sleep 1

With this:

// just use "s" to cast its signedness
$ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
Added new event:
  probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
You can now use it in all perf tools, such as:
        perf record -e probe:submit_bio -aR sleep 1
$ cat trace_pipe|head
          dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
          dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
          dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208

This commit also update perf-probe.txt to describe "types". Most parts
are based on existing documentation: Documentation/trace/kprobetrace.txt

Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
---
 tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
 tools/perf/util/probe-finder.c          | 15 ++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 736da44..b303bcd 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -176,10 +176,18 @@ Each probe argument follows below syntax.
 
 'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
 '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
-'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
+'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), signedness casting (u/s), "string" and bitfield are supported. (see TYPES for detail)
 
 On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
 
+TYPES
+-----
+Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
+String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
+Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
+
+ b<bit-width>@<bit-offset>/<container-size>
+
 LINE SYNTAX
 -----------
 Line range is described by following syntax.
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f2d9ff0..5c290c6 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 	char sbuf[STRERR_BUFSIZE];
 	int bsize, boffs, total;
 	int ret;
+	char sign;
 
 	/* TODO: check all types */
-	if (cast && strcmp(cast, "string") != 0) {
+	if (cast && strcmp(cast, "string") != 0 &&
+	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
 		/* Non string type is OK */
+		/* and respect signedness cast */
 		tvar->type = strdup(cast);
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
@@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
 
+	if (cast && (strcmp(cast, "u") == 0))
+		sign = 'u';
+	else if (cast && (strcmp(cast, "s") == 0))
+		sign = 's';
+	else
+		sign = die_is_signed_type(&type) ? 's' : 'u';
+
 	ret = dwarf_bytesize(&type);
 	if (ret <= 0)
 		/* No size ... try to use default type */
@@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
 		ret = MAX_BASIC_TYPE_BITS;
 	}
-	ret = snprintf(buf, 16, "%c%d",
-		       die_is_signed_type(&type) ? 's' : 'u', ret);
+	ret = snprintf(buf, 16, "%c%d", sign, ret);
 
 formatted:
 	if (ret < 0 || ret >= 16) {
-- 
2.7.3

Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH v3] perf probe: Support signedness casting

[Masami Hiramatsu]

On Tue, 9 Aug 2016 11:40:08 +0900
Naohiro Aota <naohiro.aota@hgst.com> wrote:

> Perf-probe detects a variable's type and use the detected type to add new
> probe. Then, kprobes prints its variable in hexadecimal format if the
> variable is unsigned and prints in decimal if it is signed.
> 
> We sometimes want to see unsigned variable in decimal format (i.e.
> sector_t or size_t). In that case, we need to investigate variable's
> size manually to specify just signedness.
> 
> This patch add signedness casting support. By specifying "s" or "u" as a
> type, perf-probe will investigate variable size as usual and use
> the specified signedness.
> 
> E.g. without this:
> 
> $ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> $ cat trace_pipe|head
>           dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
>           dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
>           dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
> ...
> // need to investigate the variable size
> $ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> 
> With this:
> 
> // just use "s" to cast its signedness
> $ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> $ cat trace_pipe|head
>           dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
>           dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
>           dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208
> 
> This commit also update perf-probe.txt to describe "types". Most parts
> are based on existing documentation: Documentation/trace/kprobetrace.txt

This looks very good to me :)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> 
> Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> ---
>  tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
>  tools/perf/util/probe-finder.c          | 15 ++++++++++++---
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> index 736da44..b303bcd 100644
> --- a/tools/perf/Documentation/perf-probe.txt
> +++ b/tools/perf/Documentation/perf-probe.txt
> @@ -176,10 +176,18 @@ Each probe argument follows below syntax.
>  
>  'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
>  '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
> -'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), signedness casting (u/s), "string" and bitfield are supported. (see TYPES for detail)
>  
>  On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
>  
> +TYPES
> +-----
> +Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
> +String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
> +
> + b<bit-width>@<bit-offset>/<container-size>
> +
>  LINE SYNTAX
>  -----------
>  Line range is described by following syntax.
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f2d9ff0..5c290c6 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  	char sbuf[STRERR_BUFSIZE];
>  	int bsize, boffs, total;
>  	int ret;
> +	char sign;
>  
>  	/* TODO: check all types */
> -	if (cast && strcmp(cast, "string") != 0) {
> +	if (cast && strcmp(cast, "string") != 0 &&
> +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
>  		/* Non string type is OK */
> +		/* and respect signedness cast */
>  		tvar->type = strdup(cast);
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
> @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
>  
> +	if (cast && (strcmp(cast, "u") == 0))
> +		sign = 'u';
> +	else if (cast && (strcmp(cast, "s") == 0))
> +		sign = 's';
> +	else
> +		sign = die_is_signed_type(&type) ? 's' : 'u';
> +
>  	ret = dwarf_bytesize(&type);
>  	if (ret <= 0)
>  		/* No size ... try to use default type */
> @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
>  		ret = MAX_BASIC_TYPE_BITS;
>  	}
> -	ret = snprintf(buf, 16, "%c%d",
> -		       die_is_signed_type(&type) ? 's' : 'u', ret);
> +	ret = snprintf(buf, 16, "%c%d", sign, ret);
>  
>  formatted:
>  	if (ret < 0 || ret >= 16) {
> -- 
> 2.7.3
> 
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
> 

BTW,

> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Could you ask your manager to remove this at least on LKML, since here is the place for
very open discussion :) ?

Thanks!


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v3] perf probe: Support signedness casting

[Arnaldo Carvalho de Melo]

Em Tue, Aug 09, 2016 at 11:40:08AM +0900, Naohiro Aota escreveu:
> Perf-probe detects a variable's type and use the detected type to add new
> probe. Then, kprobes prints its variable in hexadecimal format if the
> variable is unsigned and prints in decimal if it is signed.
> 
> We sometimes want to see unsigned variable in decimal format (i.e.
> sector_t or size_t). In that case, we need to investigate variable's
> size manually to specify just signedness.
> 
> This patch add signedness casting support. By specifying "s" or "u" as a
> type, perf-probe will investigate variable size as usual and use
> the specified signedness.

Humm, I tried with :u and got hexadecimal numbers, as before :-\ Can't
we do decimal numbers when :u is used? Just like with :s. We could then
use nothing and get the current behaviour or use :x for hexadecimal
numbers.

Anyway, applied, when using :s this is a nice improvement, thanks!

- Arnaldo
 
> E.g. without this:
> 
> $ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> $ cat trace_pipe|head
>           dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
>           dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
>           dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
> ...
> // need to investigate the variable size
> $ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> 
> With this:
> 
> // just use "s" to cast its signedness
> $ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
> You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> $ cat trace_pipe|head
>           dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
>           dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
>           dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208
> 
> This commit also update perf-probe.txt to describe "types". Most parts
> are based on existing documentation: Documentation/trace/kprobetrace.txt
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> ---
>  tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
>  tools/perf/util/probe-finder.c          | 15 ++++++++++++---
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> index 736da44..b303bcd 100644
> --- a/tools/perf/Documentation/perf-probe.txt
> +++ b/tools/perf/Documentation/perf-probe.txt
> @@ -176,10 +176,18 @@ Each probe argument follows below syntax.
>  
>  'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
>  '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
> -'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), signedness casting (u/s), "string" and bitfield are supported. (see TYPES for detail)
>  
>  On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
>  
> +TYPES
> +-----
> +Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
> +String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
> +
> + b<bit-width>@<bit-offset>/<container-size>
> +
>  LINE SYNTAX
>  -----------
>  Line range is described by following syntax.
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f2d9ff0..5c290c6 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  	char sbuf[STRERR_BUFSIZE];
>  	int bsize, boffs, total;
>  	int ret;
> +	char sign;
>  
>  	/* TODO: check all types */
> -	if (cast && strcmp(cast, "string") != 0) {
> +	if (cast && strcmp(cast, "string") != 0 &&
> +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
>  		/* Non string type is OK */
> +		/* and respect signedness cast */
>  		tvar->type = strdup(cast);
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
> @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
>  
> +	if (cast && (strcmp(cast, "u") == 0))
> +		sign = 'u';
> +	else if (cast && (strcmp(cast, "s") == 0))
> +		sign = 's';
> +	else
> +		sign = die_is_signed_type(&type) ? 's' : 'u';
> +
>  	ret = dwarf_bytesize(&type);
>  	if (ret <= 0)
>  		/* No size ... try to use default type */
> @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
>  		ret = MAX_BASIC_TYPE_BITS;
>  	}
> -	ret = snprintf(buf, 16, "%c%d",
> -		       die_is_signed_type(&type) ? 's' : 'u', ret);
> +	ret = snprintf(buf, 16, "%c%d", sign, ret);
>  
>  formatted:
>  	if (ret < 0 || ret >= 16) {
> -- 
> 2.7.3
> 
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
> 
> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

[tip:perf/urgent] perf probe: Support signedness casting

[tip-bot for Naohiro Aota]

Commit-ID:  19f00b011729417f69e4df53cc3fe5ecc25134a4
Gitweb:     http://git.kernel.org/tip/19f00b011729417f69e4df53cc3fe5ecc25134a4
Author:     Naohiro Aota <naohiro.aota@hgst.com>
AuthorDate: Tue, 9 Aug 2016 11:40:08 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 9 Aug 2016 10:52:22 -0300

perf probe: Support signedness casting

The 'perf probe' tool detects a variable's type and use the detected
type to add a new probe. Then, kprobes prints its variable in
hexadecimal format if the variable is unsigned and prints in decimal if
it is signed.

We sometimes want to see unsigned variable in decimal format (i.e.
sector_t or size_t). In that case, we need to investigate the variable's
size manually to specify just signedness.

This patch add signedness casting support. By specifying "s" or "u" as a
type, perf-probe will investigate variable size as usual and use the
specified signedness.

E.g. without this:

  $ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
  Added new event:
    probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
  You can now use it in all perf tools, such as:
          perf record -e probe:submit_bio -aR sleep 1
  $ cat trace_pipe|head
          dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
          dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
          dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
...
  // need to investigate the variable size
  $ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
  Added new event:
    probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
  You can now use it in all perf tools, such as:
        perf record -e probe:submit_bio -aR sleep 1

  With this:

  // just use "s" to cast its signedness
  $ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
  Added new event:
    probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
  You can now use it in all perf tools, such as:
          perf record -e probe:submit_bio -aR sleep 1
  $ cat trace_pipe|head
          dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
          dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
          dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208

  This commit also update perf-probe.txt to describe "types". Most parts
  are based on existing documentation: Documentation/trace/kprobetrace.txt

Committer note:

Testing using 'perf trace':

  # perf probe -a 'submit_bio bio->bi_iter.bi_sector'
  Added new event:
    probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)

  You can now use it in all perf tools, such as:

	perf record -e probe:submit_bio -aR sleep 1

  # trace --no-syscalls --ev probe:submit_bio
      0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=0xc133c0)
   3181.861 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffb8)
   3181.881 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffc0)
   3184.488 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffc8)
<SNIP>
   4717.927 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x4dc7a88)
   4717.970 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x4dc7880)
  ^C[root@jouet ~]#

Now, using this new feature:

[root@jouet ~]# perf probe -a 'submit_bio bio->bi_iter.bi_sector:s'
Added new event:
  probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)

You can now use it in all perf tools, such as:

	perf record -e probe:submit_bio -aR sleep 1

  [root@jouet ~]# trace --no-syscalls --ev probe:submit_bio
     0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145704)
     0.017 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145712)
     0.019 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145720)
     2.567 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145728)
  5631.919 probe:submit_bio:(ffffffffac3aee00) bi_sector=0)
  5631.941 probe:submit_bio:(ffffffffac3aee00) bi_sector=8)
  5631.945 probe:submit_bio:(ffffffffac3aee00) bi_sector=16)
  5631.948 probe:submit_bio:(ffffffffac3aee00) bi_sector=24)
  ^C#

With callchains:

  # trace --no-syscalls --ev probe:submit_bio/max-stack=10/
     0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662544)
                                       submit_bio+0xa8200001 ([kernel.kallsyms])
                                       submit_bh+0xa8200013 ([kernel.kallsyms])
                                       jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
                                       kjournald2+0xa82000ca ([kernel.kallsyms])
                                       kthread+0xa82000d8 ([kernel.kallsyms])
                                       ret_from_fork+0xa820001f ([kernel.kallsyms])
     0.023 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662552)
                                       submit_bio+0xa8200001 ([kernel.kallsyms])
                                       submit_bh+0xa8200013 ([kernel.kallsyms])
                                       jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
                                       kjournald2+0xa82000ca ([kernel.kallsyms])
                                       kthread+0xa82000d8 ([kernel.kallsyms])
                                       ret_from_fork+0xa820001f ([kernel.kallsyms])
     0.027 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662560)
                                       submit_bio+0xa8200001 ([kernel.kallsyms])
                                       submit_bh+0xa8200013 ([kernel.kallsyms])
                                       jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
                                       kjournald2+0xa82000ca ([kernel.kallsyms])
                                       kthread+0xa82000d8 ([kernel.kallsyms])
                                       ret_from_fork+0xa820001f ([kernel.kallsyms])
     2.593 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662568)
                                       submit_bio+0xa8200001 ([kernel.kallsyms])
                                       submit_bh+0xa8200013 ([kernel.kallsyms])
                                       journal_submit_commit_record+0xa82001ac ([kernel.kallsyms])
                                       jbd2_journal_commit_transaction+0xa82012e8 ([kernel.kallsyms])
                                       kjournald2+0xa82000ca ([kernel.kallsyms])
                                       kthread+0xa82000d8 ([kernel.kallsyms])
                                       ret_from_fork+0xa820001f ([kernel.kallsyms])
  ^C#

Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1470710408-23515-1-git-send-email-naohiro.aota@hgst.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
 tools/perf/util/probe-finder.c          | 15 ++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 736da44..b303bcd 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -176,10 +176,18 @@ Each probe argument follows below syntax.
 
 'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
 '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
-'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
+'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), signedness casting (u/s), "string" and bitfield are supported. (see TYPES for detail)
 
 On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
 
+TYPES
+-----
+Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
+String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
+Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
+
+ b<bit-width>@<bit-offset>/<container-size>
+
 LINE SYNTAX
 -----------
 Line range is described by following syntax.
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f2d9ff0..5c290c6 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 	char sbuf[STRERR_BUFSIZE];
 	int bsize, boffs, total;
 	int ret;
+	char sign;
 
 	/* TODO: check all types */
-	if (cast && strcmp(cast, "string") != 0) {
+	if (cast && strcmp(cast, "string") != 0 &&
+	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
 		/* Non string type is OK */
+		/* and respect signedness cast */
 		tvar->type = strdup(cast);
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
@@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		return (tvar->type == NULL) ? -ENOMEM : 0;
 	}
 
+	if (cast && (strcmp(cast, "u") == 0))
+		sign = 'u';
+	else if (cast && (strcmp(cast, "s") == 0))
+		sign = 's';
+	else
+		sign = die_is_signed_type(&type) ? 's' : 'u';
+
 	ret = dwarf_bytesize(&type);
 	if (ret <= 0)
 		/* No size ... try to use default type */
@@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
 		ret = MAX_BASIC_TYPE_BITS;
 	}
-	ret = snprintf(buf, 16, "%c%d",
-		       die_is_signed_type(&type) ? 's' : 'u', ret);
+	ret = snprintf(buf, 16, "%c%d", sign, ret);
 
 formatted:
 	if (ret < 0 || ret >= 16) {

Re: [tip:perf/urgent] perf probe: Support signedness casting

[Masami Hiramatsu]

Hi Ingo,

Could you add my Acked-by for this patch?
(I thought I've sent it...)

Thank you,

On Tue, 9 Aug 2016 12:19:02 -0700
tip-bot for Naohiro Aota <tipbot@zytor.com> wrote:

> Commit-ID:  19f00b011729417f69e4df53cc3fe5ecc25134a4
> Gitweb:     http://git.kernel.org/tip/19f00b011729417f69e4df53cc3fe5ecc25134a4
> Author:     Naohiro Aota <naohiro.aota@hgst.com>
> AuthorDate: Tue, 9 Aug 2016 11:40:08 +0900
> Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
> CommitDate: Tue, 9 Aug 2016 10:52:22 -0300
> 
> perf probe: Support signedness casting
> 
> The 'perf probe' tool detects a variable's type and use the detected
> type to add a new probe. Then, kprobes prints its variable in
> hexadecimal format if the variable is unsigned and prints in decimal if
> it is signed.
> 
> We sometimes want to see unsigned variable in decimal format (i.e.
> sector_t or size_t). In that case, we need to investigate the variable's
> size manually to specify just signedness.
> 
> This patch add signedness casting support. By specifying "s" or "u" as a
> type, perf-probe will investigate variable size as usual and use the
> specified signedness.
> 
> E.g. without this:
> 
>   $ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
>   Added new event:
>     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
>   You can now use it in all perf tools, such as:
>           perf record -e probe:submit_bio -aR sleep 1
>   $ cat trace_pipe|head
>           dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
>           dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
>           dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
> ...
>   // need to investigate the variable size
>   $ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
>   Added new event:
>     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
>   You can now use it in all perf tools, such as:
>         perf record -e probe:submit_bio -aR sleep 1
> 
>   With this:
> 
>   // just use "s" to cast its signedness
>   $ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
>   Added new event:
>     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
>   You can now use it in all perf tools, such as:
>           perf record -e probe:submit_bio -aR sleep 1
>   $ cat trace_pipe|head
>           dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
>           dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
>           dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208
> 
>   This commit also update perf-probe.txt to describe "types". Most parts
>   are based on existing documentation: Documentation/trace/kprobetrace.txt
> 
> Committer note:
> 
> Testing using 'perf trace':
> 
>   # perf probe -a 'submit_bio bio->bi_iter.bi_sector'
>   Added new event:
>     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
> 
>   You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:submit_bio -aR sleep 1
> 
>   # trace --no-syscalls --ev probe:submit_bio
>       0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=0xc133c0)
>    3181.861 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffb8)
>    3181.881 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffc0)
>    3184.488 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffc8)
> <SNIP>
>    4717.927 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x4dc7a88)
>    4717.970 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x4dc7880)
>   ^C[root@jouet ~]#
> 
> Now, using this new feature:
> 
> [root@jouet ~]# perf probe -a 'submit_bio bio->bi_iter.bi_sector:s'
> Added new event:
>   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:submit_bio -aR sleep 1
> 
>   [root@jouet ~]# trace --no-syscalls --ev probe:submit_bio
>      0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145704)
>      0.017 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145712)
>      0.019 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145720)
>      2.567 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145728)
>   5631.919 probe:submit_bio:(ffffffffac3aee00) bi_sector=0)
>   5631.941 probe:submit_bio:(ffffffffac3aee00) bi_sector=8)
>   5631.945 probe:submit_bio:(ffffffffac3aee00) bi_sector=16)
>   5631.948 probe:submit_bio:(ffffffffac3aee00) bi_sector=24)
>   ^C#
> 
> With callchains:
> 
>   # trace --no-syscalls --ev probe:submit_bio/max-stack=10/
>      0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662544)
>                                        submit_bio+0xa8200001 ([kernel.kallsyms])
>                                        submit_bh+0xa8200013 ([kernel.kallsyms])
>                                        jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
>                                        kjournald2+0xa82000ca ([kernel.kallsyms])
>                                        kthread+0xa82000d8 ([kernel.kallsyms])
>                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
>      0.023 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662552)
>                                        submit_bio+0xa8200001 ([kernel.kallsyms])
>                                        submit_bh+0xa8200013 ([kernel.kallsyms])
>                                        jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
>                                        kjournald2+0xa82000ca ([kernel.kallsyms])
>                                        kthread+0xa82000d8 ([kernel.kallsyms])
>                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
>      0.027 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662560)
>                                        submit_bio+0xa8200001 ([kernel.kallsyms])
>                                        submit_bh+0xa8200013 ([kernel.kallsyms])
>                                        jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
>                                        kjournald2+0xa82000ca ([kernel.kallsyms])
>                                        kthread+0xa82000d8 ([kernel.kallsyms])
>                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
>      2.593 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662568)
>                                        submit_bio+0xa8200001 ([kernel.kallsyms])
>                                        submit_bh+0xa8200013 ([kernel.kallsyms])
>                                        journal_submit_commit_record+0xa82001ac ([kernel.kallsyms])
>                                        jbd2_journal_commit_transaction+0xa82012e8 ([kernel.kallsyms])
>                                        kjournald2+0xa82000ca ([kernel.kallsyms])
>                                        kthread+0xa82000d8 ([kernel.kallsyms])
>                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
>   ^C#
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Wang Nan <wangnan0@huawei.com>
> Link: http://lkml.kernel.org/r/1470710408-23515-1-git-send-email-naohiro.aota@hgst.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
>  tools/perf/util/probe-finder.c          | 15 ++++++++++++---
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> index 736da44..b303bcd 100644
> --- a/tools/perf/Documentation/perf-probe.txt
> +++ b/tools/perf/Documentation/perf-probe.txt
> @@ -176,10 +176,18 @@ Each probe argument follows below syntax.
>  
>  'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
>  '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
> -'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), signedness casting (u/s), "string" and bitfield are supported. (see TYPES for detail)
>  
>  On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
>  
> +TYPES
> +-----
> +Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
> +String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> +Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
> +
> + b<bit-width>@<bit-offset>/<container-size>
> +
>  LINE SYNTAX
>  -----------
>  Line range is described by following syntax.
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f2d9ff0..5c290c6 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  	char sbuf[STRERR_BUFSIZE];
>  	int bsize, boffs, total;
>  	int ret;
> +	char sign;
>  
>  	/* TODO: check all types */
> -	if (cast && strcmp(cast, "string") != 0) {
> +	if (cast && strcmp(cast, "string") != 0 &&
> +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
>  		/* Non string type is OK */
> +		/* and respect signedness cast */
>  		tvar->type = strdup(cast);
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
> @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  		return (tvar->type == NULL) ? -ENOMEM : 0;
>  	}
>  
> +	if (cast && (strcmp(cast, "u") == 0))
> +		sign = 'u';
> +	else if (cast && (strcmp(cast, "s") == 0))
> +		sign = 's';
> +	else
> +		sign = die_is_signed_type(&type) ? 's' : 'u';
> +
>  	ret = dwarf_bytesize(&type);
>  	if (ret <= 0)
>  		/* No size ... try to use default type */
> @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
>  			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
>  		ret = MAX_BASIC_TYPE_BITS;
>  	}
> -	ret = snprintf(buf, 16, "%c%d",
> -		       die_is_signed_type(&type) ? 's' : 'u', ret);
> +	ret = snprintf(buf, 16, "%c%d", sign, ret);
>  
>  formatted:
>  	if (ret < 0 || ret >= 16) {


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v3] perf probe: Support signedness casting

[Masami Hiramatsu]

On Tue, 9 Aug 2016 11:05:28 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Tue, Aug 09, 2016 at 11:40:08AM +0900, Naohiro Aota escreveu:
> > Perf-probe detects a variable's type and use the detected type to add new
> > probe. Then, kprobes prints its variable in hexadecimal format if the
> > variable is unsigned and prints in decimal if it is signed.
> > 
> > We sometimes want to see unsigned variable in decimal format (i.e.
> > sector_t or size_t). In that case, we need to investigate variable's
> > size manually to specify just signedness.
> > 
> > This patch add signedness casting support. By specifying "s" or "u" as a
> > type, perf-probe will investigate variable size as usual and use
> > the specified signedness.
> 
> Humm, I tried with :u and got hexadecimal numbers, as before :-\ Can't
> we do decimal numbers when :u is used? Just like with :s. We could then
> use nothing and get the current behaviour or use :x for hexadecimal
> numbers.

Hmm, would you mean we'll change the format in ftrace or perf?
u8/16/32/64 is already used for showing hexadecimal numbers, and
how it is shown, is decided by lib/traceevent. I think we can add
specifying format string as a option in addition to the type cast for
ftrace. But for perf, I'm not sure how it is decided to show the data.
Does it follow the ftrace's printf format?

Thanks,


> 
> Anyway, applied, when using :s this is a nice improvement, thanks!
> 
> - Arnaldo
>  
> > E.g. without this:
> > 
> > $ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
> > Added new event:
> >   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
> > You can now use it in all perf tools, such as:
> >         perf record -e probe:submit_bio -aR sleep 1
> > $ cat trace_pipe|head
> >           dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
> >           dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
> >           dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
> > ...
> > // need to investigate the variable size
> > $ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
> > Added new event:
> >   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
> > You can now use it in all perf tools, such as:
> >         perf record -e probe:submit_bio -aR sleep 1
> > 
> > With this:
> > 
> > // just use "s" to cast its signedness
> > $ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
> > Added new event:
> >   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
> > You can now use it in all perf tools, such as:
> >         perf record -e probe:submit_bio -aR sleep 1
> > $ cat trace_pipe|head
> >           dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
> >           dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
> >           dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208
> > 
> > This commit also update perf-probe.txt to describe "types". Most parts
> > are based on existing documentation: Documentation/trace/kprobetrace.txt
> > 
> > Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> > ---
> >  tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
> >  tools/perf/util/probe-finder.c          | 15 ++++++++++++---
> >  2 files changed, 21 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> > index 736da44..b303bcd 100644
> > --- a/tools/perf/Documentation/perf-probe.txt
> > +++ b/tools/perf/Documentation/perf-probe.txt
> > @@ -176,10 +176,18 @@ Each probe argument follows below syntax.
> >  
> >  'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
> >  '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
> > -'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> > +'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), signedness casting (u/s), "string" and bitfield are supported. (see TYPES for detail)
> >  
> >  On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
> >  
> > +TYPES
> > +-----
> > +Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
> > +String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> > +Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
> > +
> > + b<bit-width>@<bit-offset>/<container-size>
> > +
> >  LINE SYNTAX
> >  -----------
> >  Line range is described by following syntax.
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index f2d9ff0..5c290c6 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
> >  	char sbuf[STRERR_BUFSIZE];
> >  	int bsize, boffs, total;
> >  	int ret;
> > +	char sign;
> >  
> >  	/* TODO: check all types */
> > -	if (cast && strcmp(cast, "string") != 0) {
> > +	if (cast && strcmp(cast, "string") != 0 &&
> > +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
> >  		/* Non string type is OK */
> > +		/* and respect signedness cast */
> >  		tvar->type = strdup(cast);
> >  		return (tvar->type == NULL) ? -ENOMEM : 0;
> >  	}
> > @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
> >  		return (tvar->type == NULL) ? -ENOMEM : 0;
> >  	}
> >  
> > +	if (cast && (strcmp(cast, "u") == 0))
> > +		sign = 'u';
> > +	else if (cast && (strcmp(cast, "s") == 0))
> > +		sign = 's';
> > +	else
> > +		sign = die_is_signed_type(&type) ? 's' : 'u';
> > +
> >  	ret = dwarf_bytesize(&type);
> >  	if (ret <= 0)
> >  		/* No size ... try to use default type */
> > @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
> >  			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
> >  		ret = MAX_BASIC_TYPE_BITS;
> >  	}
> > -	ret = snprintf(buf, 16, "%c%d",
> > -		       die_is_signed_type(&type) ? 's' : 'u', ret);
> > +	ret = snprintf(buf, 16, "%c%d", sign, ret);
> >  
> >  formatted:
> >  	if (ret < 0 || ret >= 16) {
> > -- 
> > 2.7.3
> > 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v3] perf probe: Support signedness casting

[Arnaldo Carvalho de Melo]

Em Wed, Aug 10, 2016 at 07:38:28AM +0900, Masami Hiramatsu escreveu:
> On Tue, 9 Aug 2016 11:05:28 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Tue, Aug 09, 2016 at 11:40:08AM +0900, Naohiro Aota escreveu:
> > > This patch add signedness casting support. By specifying "s" or "u" as a
> > > type, perf-probe will investigate variable size as usual and use
> > > the specified signedness.

> > Humm, I tried with :u and got hexadecimal numbers, as before :-\ Can't
> > we do decimal numbers when :u is used? Just like with :s. We could then
> > use nothing and get the current behaviour or use :x for hexadecimal
> > numbers.
> 
> Hmm, would you mean we'll change the format in ftrace or perf?
> u8/16/32/64 is already used for showing hexadecimal numbers, and
> how it is shown, is decided by lib/traceevent. I think we can add
> specifying format string as a option in addition to the type cast for
> ftrace. But for perf, I'm not sure how it is decided to show the data.
> Does it follow the ftrace's printf format?

Humm, what I asked was: why using :s makes it appears as signed decimal
while :u makes it appear as unsigned _hexa_decimal?

Someone reading the announce for this patch is lead to think that 's'
and 'u' are just for signedness. So having another letter ('x') to
specify how to format it seems like a valid expectation.

If ftrace would use it? I don't know, I think it should.

And perf currently uses libtraceevent to do most of the field pretty
printing (symbol resolving is done using perf's symbol.c and friends,
for instance).

- Arnaldo 
 
> > Anyway, applied, when using :s this is a nice improvement, thanks!

Re: [tip:perf/urgent] perf probe: Support signedness casting

[Arnaldo Carvalho de Melo]

Em Wed, Aug 10, 2016 at 07:28:08AM +0900, Masami Hiramatsu escreveu:
> Hi Ingo,
> 
> Could you add my Acked-by for this patch?
> (I thought I've sent it...)

Sorry about that, I usually put those acks diligently, but this one
slipped by... I saw the discussion, waited for it to settle and you ack
to be provided, but then, while testing it and editing the log, I forgot
to collect the ack.

- Arnaldo
 
> Thank you,
> 
> On Tue, 9 Aug 2016 12:19:02 -0700
> tip-bot for Naohiro Aota <tipbot@zytor.com> wrote:
> 
> > Commit-ID:  19f00b011729417f69e4df53cc3fe5ecc25134a4
> > Gitweb:     http://git.kernel.org/tip/19f00b011729417f69e4df53cc3fe5ecc25134a4
> > Author:     Naohiro Aota <naohiro.aota@hgst.com>
> > AuthorDate: Tue, 9 Aug 2016 11:40:08 +0900
> > Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
> > CommitDate: Tue, 9 Aug 2016 10:52:22 -0300
> > 
> > perf probe: Support signedness casting
> > 
> > The 'perf probe' tool detects a variable's type and use the detected
> > type to add a new probe. Then, kprobes prints its variable in
> > hexadecimal format if the variable is unsigned and prints in decimal if
> > it is signed.
> > 
> > We sometimes want to see unsigned variable in decimal format (i.e.
> > sector_t or size_t). In that case, we need to investigate the variable's
> > size manually to specify just signedness.
> > 
> > This patch add signedness casting support. By specifying "s" or "u" as a
> > type, perf-probe will investigate variable size as usual and use the
> > specified signedness.
> > 
> > E.g. without this:
> > 
> >   $ perf probe -a 'submit_bio bio->bi_iter.bi_sector'
> >   Added new event:
> >     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
> >   You can now use it in all perf tools, such as:
> >           perf record -e probe:submit_bio -aR sleep 1
> >   $ cat trace_pipe|head
> >           dbench-9692  [003] d..1   971.096633: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d00
> >           dbench-9692  [003] d..1   971.096685: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x1a3d80
> >           dbench-9692  [003] d..1   971.096687: submit_bio: (submit_bio+0x0/0x140) bi_sector=0x3a3d80
> > ...
> >   // need to investigate the variable size
> >   $ perf probe -a 'submit_bio bio->bi_iter.bi_sector:s64'
> >   Added new event:
> >     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s64)
> >   You can now use it in all perf tools, such as:
> >         perf record -e probe:submit_bio -aR sleep 1
> > 
> >   With this:
> > 
> >   // just use "s" to cast its signedness
> >   $ perf probe -v -a 'submit_bio bio->bi_iter.bi_sector:s'
> >   Added new event:
> >     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
> >   You can now use it in all perf tools, such as:
> >           perf record -e probe:submit_bio -aR sleep 1
> >   $ cat trace_pipe|head
> >           dbench-9689  [001] d..1  1212.391237: submit_bio: (submit_bio+0x0/0x140) bi_sector=128
> >           dbench-9689  [001] d..1  1212.391252: submit_bio: (submit_bio+0x0/0x140) bi_sector=131072
> >           dbench-9697  [006] d..1  1212.398611: submit_bio: (submit_bio+0x0/0x140) bi_sector=30208
> > 
> >   This commit also update perf-probe.txt to describe "types". Most parts
> >   are based on existing documentation: Documentation/trace/kprobetrace.txt
> > 
> > Committer note:
> > 
> > Testing using 'perf trace':
> > 
> >   # perf probe -a 'submit_bio bio->bi_iter.bi_sector'
> >   Added new event:
> >     probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe:submit_bio -aR sleep 1
> > 
> >   # trace --no-syscalls --ev probe:submit_bio
> >       0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=0xc133c0)
> >    3181.861 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffb8)
> >    3181.881 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffc0)
> >    3184.488 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x6cffc8)
> > <SNIP>
> >    4717.927 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x4dc7a88)
> >    4717.970 probe:submit_bio:(ffffffffac3aee00) bi_sector=0x4dc7880)
> >   ^C[root@jouet ~]#
> > 
> > Now, using this new feature:
> > 
> > [root@jouet ~]# perf probe -a 'submit_bio bio->bi_iter.bi_sector:s'
> > Added new event:
> >   probe:submit_bio     (on submit_bio with bi_sector=bio->bi_iter.bi_sector:s)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe:submit_bio -aR sleep 1
> > 
> >   [root@jouet ~]# trace --no-syscalls --ev probe:submit_bio
> >      0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145704)
> >      0.017 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145712)
> >      0.019 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145720)
> >      2.567 probe:submit_bio:(ffffffffac3aee00) bi_sector=7145728)
> >   5631.919 probe:submit_bio:(ffffffffac3aee00) bi_sector=0)
> >   5631.941 probe:submit_bio:(ffffffffac3aee00) bi_sector=8)
> >   5631.945 probe:submit_bio:(ffffffffac3aee00) bi_sector=16)
> >   5631.948 probe:submit_bio:(ffffffffac3aee00) bi_sector=24)
> >   ^C#
> > 
> > With callchains:
> > 
> >   # trace --no-syscalls --ev probe:submit_bio/max-stack=10/
> >      0.000 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662544)
> >                                        submit_bio+0xa8200001 ([kernel.kallsyms])
> >                                        submit_bh+0xa8200013 ([kernel.kallsyms])
> >                                        jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
> >                                        kjournald2+0xa82000ca ([kernel.kallsyms])
> >                                        kthread+0xa82000d8 ([kernel.kallsyms])
> >                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
> >      0.023 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662552)
> >                                        submit_bio+0xa8200001 ([kernel.kallsyms])
> >                                        submit_bh+0xa8200013 ([kernel.kallsyms])
> >                                        jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
> >                                        kjournald2+0xa82000ca ([kernel.kallsyms])
> >                                        kthread+0xa82000d8 ([kernel.kallsyms])
> >                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
> >      0.027 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662560)
> >                                        submit_bio+0xa8200001 ([kernel.kallsyms])
> >                                        submit_bh+0xa8200013 ([kernel.kallsyms])
> >                                        jbd2_journal_commit_transaction+0xa8200691 ([kernel.kallsyms])
> >                                        kjournald2+0xa82000ca ([kernel.kallsyms])
> >                                        kthread+0xa82000d8 ([kernel.kallsyms])
> >                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
> >      2.593 probe:submit_bio:(ffffffffac3aee00) bi_sector=50662568)
> >                                        submit_bio+0xa8200001 ([kernel.kallsyms])
> >                                        submit_bh+0xa8200013 ([kernel.kallsyms])
> >                                        journal_submit_commit_record+0xa82001ac ([kernel.kallsyms])
> >                                        jbd2_journal_commit_transaction+0xa82012e8 ([kernel.kallsyms])
> >                                        kjournald2+0xa82000ca ([kernel.kallsyms])
> >                                        kthread+0xa82000d8 ([kernel.kallsyms])
> >                                        ret_from_fork+0xa820001f ([kernel.kallsyms])
> >   ^C#
> > 
> > Signed-off-by: Naohiro Aota <naohiro.aota@hgst.com>
> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Wang Nan <wangnan0@huawei.com>
> > Link: http://lkml.kernel.org/r/1470710408-23515-1-git-send-email-naohiro.aota@hgst.com
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/Documentation/perf-probe.txt | 10 +++++++++-
> >  tools/perf/util/probe-finder.c          | 15 ++++++++++++---
> >  2 files changed, 21 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> > index 736da44..b303bcd 100644
> > --- a/tools/perf/Documentation/perf-probe.txt
> > +++ b/tools/perf/Documentation/perf-probe.txt
> > @@ -176,10 +176,18 @@ Each probe argument follows below syntax.
> >  
> >  'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
> >  '$vars' and '$params' special arguments are also available for NAME, '$vars' is expanded to the local variables (including function parameters) which can access at given probe point. '$params' is expanded to only the function parameters.
> > -'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> > +'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo. Currently, basic types (u8/u16/u32/u64/s8/s16/s32/s64), signedness casting (u/s), "string" and bitfield are supported. (see TYPES for detail)
> >  
> >  On x86 systems %REG is always the short form of the register: for example %AX. %RAX or %EAX is not valid.
> >  
> > +TYPES
> > +-----
> > +Basic types (u8/u16/u32/u64/s8/s16/s32/s64) are integer types. Prefix 's' and 'u' means those types are signed and unsigned respectively. Traced arguments are shown in decimal (signed) or hex (unsigned). You can also use 's' or 'u' to specify only signedness and leave its size auto-detected by perf probe.
> > +String type is a special type, which fetches a "null-terminated" string from kernel space. This means it will fail and store NULL if the string container has been paged out. You can specify 'string' type only for the local variable or structure member which is an array of or a pointer to 'char' or 'unsigned char' type.
> > +Bitfield is another special type, which takes 3 parameters, bit-width, bit-offset, and container-size (usually 32). The syntax is;
> > +
> > + b<bit-width>@<bit-offset>/<container-size>
> > +
> >  LINE SYNTAX
> >  -----------
> >  Line range is described by following syntax.
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index f2d9ff0..5c290c6 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -297,10 +297,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
> >  	char sbuf[STRERR_BUFSIZE];
> >  	int bsize, boffs, total;
> >  	int ret;
> > +	char sign;
> >  
> >  	/* TODO: check all types */
> > -	if (cast && strcmp(cast, "string") != 0) {
> > +	if (cast && strcmp(cast, "string") != 0 &&
> > +	    strcmp(cast, "s") != 0 && strcmp(cast, "u") != 0) {
> >  		/* Non string type is OK */
> > +		/* and respect signedness cast */
> >  		tvar->type = strdup(cast);
> >  		return (tvar->type == NULL) ? -ENOMEM : 0;
> >  	}
> > @@ -361,6 +364,13 @@ static int convert_variable_type(Dwarf_Die *vr_die,
> >  		return (tvar->type == NULL) ? -ENOMEM : 0;
> >  	}
> >  
> > +	if (cast && (strcmp(cast, "u") == 0))
> > +		sign = 'u';
> > +	else if (cast && (strcmp(cast, "s") == 0))
> > +		sign = 's';
> > +	else
> > +		sign = die_is_signed_type(&type) ? 's' : 'u';
> > +
> >  	ret = dwarf_bytesize(&type);
> >  	if (ret <= 0)
> >  		/* No size ... try to use default type */
> > @@ -373,8 +383,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
> >  			dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
> >  		ret = MAX_BASIC_TYPE_BITS;
> >  	}
> > -	ret = snprintf(buf, 16, "%c%d",
> > -		       die_is_signed_type(&type) ? 's' : 'u', ret);
> > +	ret = snprintf(buf, 16, "%c%d", sign, ret);
> >  
> >  formatted:
> >  	if (ret < 0 || ret >= 16) {
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v3] perf probe: Support signedness casting

[Masami Hiramatsu]

On Wed, 10 Aug 2016 10:04:40 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, Aug 10, 2016 at 07:38:28AM +0900, Masami Hiramatsu escreveu:
> > On Tue, 9 Aug 2016 11:05:28 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > Em Tue, Aug 09, 2016 at 11:40:08AM +0900, Naohiro Aota escreveu:
> > > > This patch add signedness casting support. By specifying "s" or "u" as a
> > > > type, perf-probe will investigate variable size as usual and use
> > > > the specified signedness.
> 
> > > Humm, I tried with :u and got hexadecimal numbers, as before :-\ Can't
> > > we do decimal numbers when :u is used? Just like with :s. We could then
> > > use nothing and get the current behaviour or use :x for hexadecimal
> > > numbers.
> > 
> > Hmm, would you mean we'll change the format in ftrace or perf?
> > u8/16/32/64 is already used for showing hexadecimal numbers, and
> > how it is shown, is decided by lib/traceevent. I think we can add
> > specifying format string as a option in addition to the type cast for
> > ftrace. But for perf, I'm not sure how it is decided to show the data.
> > Does it follow the ftrace's printf format?
> 
> Humm, what I asked was: why using :s makes it appears as signed decimal
> while :u makes it appear as unsigned _hexa_decimal?

IIRC, I decided that. Since I would like to use kprobes mainly for debug,
show vars in hexadecimal by default(like debug dump). But for the signed
value the decimal should be used (%x implies unsigned).
Actually, without any type casting, kprobe argument is printed in hexadecimal
by default (because it is just a dump of register/memory). So, signed value
is special.

> Someone reading the announce for this patch is lead to think that 's'
> and 'u' are just for signedness. So having another letter ('x') to
> specify how to format it seems like a valid expectation.

I see, I think when we start supporting debuginfo by perf, I should have
come up with that. At this point, I'm just considering backward compatibility
and extensibility, since if 'u' means decimal and 'x' means hexadecimal,
'u8','u16','u32','u64' should also be changed to decimal. And also, we might
consider the case if someone asks to show vars in octal (like file permission)
or in fixed-width zero-filled hexadecimal (like address). It seems not so much
variety, so it may be possible to introduce 'oXX' for octal and 'p' for address
etc. (but is that type casting...?)

> If ftrace would use it? I don't know, I think it should.
> 
> And perf currently uses libtraceevent to do most of the field pretty
> printing (symbol resolving is done using perf's symbol.c and friends,
> for instance).

Hm, OK, it seems using ftrace format string, so I think I just need to
change it.

> 
> - Arnaldo 
>  
> > > Anyway, applied, when using :s this is a nice improvement, thanks!


-- 
Masami Hiramatsu <mhiramat@kernel.org>