* [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax
@ 2013-11-18 4:40 Namhyung Kim
2013-11-18 4:40 ` [PATCH 2/2] tracing/probes: Fix basic print type functions Namhyung Kim
2013-11-20 15:19 ` [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax Oleg Nesterov
0 siblings, 2 replies; 6+ messages in thread
From: Namhyung Kim @ 2013-11-18 4:40 UTC (permalink / raw)
To: Steven Rostedt
Cc: Frederic Weisbecker, Ingo Molnar, Masami Hiramatsu, LKML,
Namhyung Kim, Srikar Dronamraju, Oleg Nesterov, zhangwei(Jovi),
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung.kim@lge.com>
The uprobe syntax requires an offset after a file path not a symbol.
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: zhangwei(Jovi) <jovi.zhangwei@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
Documentation/trace/uprobetracer.txt | 10 +++++-----
kernel/trace/trace_uprobe.c | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index d9c3e682312c..8f1a8b8956fc 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -19,15 +19,15 @@ user to calculate the offset of the probepoint in the object.
Synopsis of uprobe_tracer
-------------------------
- p[:[GRP/]EVENT] PATH:SYMBOL[+offs] [FETCHARGS] : Set a uprobe
- r[:[GRP/]EVENT] PATH:SYMBOL[+offs] [FETCHARGS] : Set a return uprobe (uretprobe)
- -:[GRP/]EVENT : Clear uprobe or uretprobe event
+ p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a uprobe
+ r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe)
+ -:[GRP/]EVENT : Clear uprobe or uretprobe event
GRP : Group name. If omitted, "uprobes" is the default value.
EVENT : Event name. If omitted, the event name is generated based
- on SYMBOL+offs.
+ on PATH+OFFSET.
PATH : Path to an executable or a library.
- SYMBOL[+offs] : Symbol+offset where the probe is inserted.
+ OFFSET : Offset where the probe is inserted.
FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 272261b5f94f..a415c5867ec5 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -210,7 +210,7 @@ end:
/*
* Argument syntax:
- * - Add uprobe: p|r[:[GRP/]EVENT] PATH:SYMBOL [FETCHARGS]
+ * - Add uprobe: p|r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS]
*
* - Remove uprobe: -:[GRP/]EVENT
*/
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] tracing/probes: Fix basic print type functions
2013-11-18 4:40 [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax Namhyung Kim
@ 2013-11-18 4:40 ` Namhyung Kim
2013-11-20 15:22 ` Oleg Nesterov
2013-11-20 15:19 ` [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax Oleg Nesterov
1 sibling, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2013-11-18 4:40 UTC (permalink / raw)
To: Steven Rostedt
Cc: Frederic Weisbecker, Ingo Molnar, Masami Hiramatsu, LKML,
Namhyung Kim, Srikar Dronamraju, Oleg Nesterov, zhangwei(Jovi),
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung.kim@lge.com>
The print format of s32 type was "ld" and it's casted to "long". So
it turned out to print 4294967295 for "-1" on 64-bit systems. Not
sure whether it worked well on 32-bit systems.
Anyway, it'd be better if we have exact format and type cast for each
types on both of 32- and 64-bit systems. In fact, the only difference
is on s64/u64 types.
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: zhangwei(Jovi) <jovi.zhangwei@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
kernel/trace/trace_probe.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 412e959709b4..b571e4de0769 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -49,14 +49,19 @@ static __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
} \
static const char PRINT_TYPE_FMT_NAME(type)[] = fmt;
-DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x", unsigned int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%lx", unsigned long)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u8 , "%x", unsigned char)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned short)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%x", unsigned int)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", signed char)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", short)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d", int)
+#if BITS_PER_LONG == 32
DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%llx", unsigned long long)
-DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
+#else /* BITS_PER_LONG == 64 */
+DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%lx", unsigned long)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%ld", long)
+#endif
static inline void *get_rloc_data(u32 *dl)
{
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax
2013-11-18 4:40 [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax Namhyung Kim
2013-11-18 4:40 ` [PATCH 2/2] tracing/probes: Fix basic print type functions Namhyung Kim
@ 2013-11-20 15:19 ` Oleg Nesterov
1 sibling, 0 replies; 6+ messages in thread
From: Oleg Nesterov @ 2013-11-20 15:19 UTC (permalink / raw)
To: Namhyung Kim
Cc: Steven Rostedt, Frederic Weisbecker, Ingo Molnar,
Masami Hiramatsu, LKML, Namhyung Kim, Srikar Dronamraju,
zhangwei(Jovi), Arnaldo Carvalho de Melo
On 11/18, Namhyung Kim wrote:
>
> From: Namhyung Kim <namhyung.kim@lge.com>
>
> The uprobe syntax requires an offset after a file path not a symbol.
>
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: zhangwei(Jovi) <jovi.zhangwei@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tracing/probes: Fix basic print type functions
2013-11-18 4:40 ` [PATCH 2/2] tracing/probes: Fix basic print type functions Namhyung Kim
@ 2013-11-20 15:22 ` Oleg Nesterov
2013-11-22 6:05 ` Namhyung Kim
0 siblings, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2013-11-20 15:22 UTC (permalink / raw)
To: Namhyung Kim
Cc: Steven Rostedt, Frederic Weisbecker, Ingo Molnar,
Masami Hiramatsu, LKML, Namhyung Kim, Srikar Dronamraju,
zhangwei(Jovi), Arnaldo Carvalho de Melo
On 11/18, Namhyung Kim wrote:
>
> -DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x", unsigned int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%lx", unsigned long)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u8 , "%x", unsigned char)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned short)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%x", unsigned int)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", signed char)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", short)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d", int)
> +#if BITS_PER_LONG == 32
> DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%llx", unsigned long long)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
> DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
> +#else /* BITS_PER_LONG == 64 */
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%lx", unsigned long)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%ld", long)
> +#endif
I must have missed something... Not only I do not understand why
do we need "#if BITS_PER_LONG", I can't understand why
DEFINE_BASIC_PRINT_TYPE_FUNC() needs "cast" argument.
IOW, how about the patch below instead?
Oleg.
--- x/kernel/trace/trace_probe.c
+++ x/kernel/trace/trace_probe.c
@@ -40,23 +40,23 @@ const char *reserved_field_names[] = {
#define PRINT_TYPE_FMT_NAME(type) print_type_format_##type
/* Printing in basic type function template */
-#define DEFINE_BASIC_PRINT_TYPE_FUNC(type, fmt, cast) \
+#define DEFINE_BASIC_PRINT_TYPE_FUNC(type, fmt) \
static __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
const char *name, \
- void *data, void *ent)\
+ void *data, void *ent) \
{ \
- return trace_seq_printf(s, " %s=" fmt, name, (cast)*(type *)data);\
+ return trace_seq_printf(s, " %s=" fmt, name, *(type *)data); \
} \
static const char PRINT_TYPE_FMT_NAME(type)[] = fmt;
-DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x", unsigned int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%lx", unsigned long)
-DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%llx", unsigned long long)
-DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
-DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
-DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x")
+DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x")
+DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%x")
+DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%Lx")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
static inline void *get_rloc_data(u32 *dl)
{
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tracing/probes: Fix basic print type functions
2013-11-20 15:22 ` Oleg Nesterov
@ 2013-11-22 6:05 ` Namhyung Kim
2013-11-22 8:10 ` Masami Hiramatsu
0 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2013-11-22 6:05 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Steven Rostedt, Frederic Weisbecker, Ingo Molnar,
Masami Hiramatsu, LKML, Namhyung Kim, Srikar Dronamraju,
zhangwei(Jovi), Arnaldo Carvalho de Melo
Hi Oleg,
On Wed, 20 Nov 2013 16:22:45 +0100, Oleg Nesterov wrote:
> On 11/18, Namhyung Kim wrote:
>>
>> -DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x", unsigned int)
>> -DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned int)
>> -DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%lx", unsigned long)
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u8 , "%x", unsigned char)
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned short)
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%x", unsigned int)
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", signed char)
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", short)
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d", int)
>> +#if BITS_PER_LONG == 32
>> DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%llx", unsigned long long)
>> -DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", int)
>> -DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
>> -DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
>> DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
>> +#else /* BITS_PER_LONG == 64 */
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%lx", unsigned long)
>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%ld", long)
>> +#endif
>
> I must have missed something... Not only I do not understand why
> do we need "#if BITS_PER_LONG", I can't understand why
> DEFINE_BASIC_PRINT_TYPE_FUNC() needs "cast" argument.
>
> IOW, how about the patch below instead?
Looks good to me.
Masami, did you have any issue regarding the casts?
Thanks,
Namhyung
>
> --- x/kernel/trace/trace_probe.c
> +++ x/kernel/trace/trace_probe.c
> @@ -40,23 +40,23 @@ const char *reserved_field_names[] = {
> #define PRINT_TYPE_FMT_NAME(type) print_type_format_##type
>
> /* Printing in basic type function template */
> -#define DEFINE_BASIC_PRINT_TYPE_FUNC(type, fmt, cast) \
> +#define DEFINE_BASIC_PRINT_TYPE_FUNC(type, fmt) \
> static __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
> const char *name, \
> - void *data, void *ent)\
> + void *data, void *ent) \
> { \
> - return trace_seq_printf(s, " %s=" fmt, name, (cast)*(type *)data);\
> + return trace_seq_printf(s, " %s=" fmt, name, *(type *)data); \
> } \
> static const char PRINT_TYPE_FMT_NAME(type)[] = fmt;
>
> -DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x", unsigned int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%lx", unsigned long)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%llx", unsigned long long)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
> -DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x")
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x")
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%x")
> +DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%Lx")
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d")
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
> +DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
>
> static inline void *get_rloc_data(u32 *dl)
> {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH 2/2] tracing/probes: Fix basic print type functions
2013-11-22 6:05 ` Namhyung Kim
@ 2013-11-22 8:10 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2013-11-22 8:10 UTC (permalink / raw)
To: Namhyung Kim
Cc: Oleg Nesterov, Steven Rostedt, Frederic Weisbecker, Ingo Molnar,
LKML, Namhyung Kim, Srikar Dronamraju, zhangwei(Jovi),
Arnaldo Carvalho de Melo
(2013/11/22 15:05), Namhyung Kim wrote:
> Hi Oleg,
>
> On Wed, 20 Nov 2013 16:22:45 +0100, Oleg Nesterov wrote:
>> On 11/18, Namhyung Kim wrote:
>>>
>>> -DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x", unsigned int)
>>> -DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned int)
>>> -DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%lx", unsigned long)
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u8 , "%x", unsigned char)
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned short)
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%x", unsigned int)
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", signed char)
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", short)
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d", int)
>>> +#if BITS_PER_LONG == 32
>>> DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%llx", unsigned long long)
>>> -DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", int)
>>> -DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
>>> -DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
>>> DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
>>> +#else /* BITS_PER_LONG == 64 */
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%lx", unsigned long)
>>> +DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%ld", long)
>>> +#endif
>>
>> I must have missed something... Not only I do not understand why
>> do we need "#if BITS_PER_LONG", I can't understand why
>> DEFINE_BASIC_PRINT_TYPE_FUNC() needs "cast" argument.
>>
>> IOW, how about the patch below instead?
>
> Looks good to me.
>
> Masami, did you have any issue regarding the casts?
Hm, unless compiler doesn't complain about it,I'm OK for that.
And as far as I tested on x86/x86-64/arm(32), this has no problem. :)
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Thank you!
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-22 8:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-18 4:40 [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax Namhyung Kim
2013-11-18 4:40 ` [PATCH 2/2] tracing/probes: Fix basic print type functions Namhyung Kim
2013-11-20 15:22 ` Oleg Nesterov
2013-11-22 6:05 ` Namhyung Kim
2013-11-22 8:10 ` Masami Hiramatsu
2013-11-20 15:19 ` [PATCH 1/2] tracing/uprobes: Fix documentation of uprobe registration syntax Oleg Nesterov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).