* [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 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
* 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
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).