* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.