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

[Masami Hiramatsu <mhiramat@kernel.org>]

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>