Re: [PATCH 11/18] tracing: Add symbol type to function based events

Namhyung Kim
2018-02-08T11:03:41Z

On Fri, Feb 02, 2018 at 06:05:09PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> Add a special type "symbol" that will use %pS to display the field of a
> function based event.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Documentation/trace/function-based-events.rst | 26 +++++++++++++++++++++++++-
> kernel/trace/trace_event_ftrace.c | 13 ++++++++++---
> 2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/trace/function-based-events.rst b/Documentation/trace/function-based-events.rst
> index bdb28f433bfb..f18c8f3ef330 100644
> --- a/Documentation/trace/function-based-events.rst
> +++ b/Documentation/trace/function-based-events.rst
> @@ -98,7 +98,8 @@ as follows:
> ATOM := 'u8' | 'u16' | 'u32' | 'u64' |
> 's8' | 's16' | 's32' | 's64' |
> 'x8' | 'x16' | 'x32' | 'x64' |
> - 'char' | 'short' | 'int' | 'long' | 'size_t'
> + 'char' | 'short' | 'int' | 'long' | 'size_t' |
> + 'symbol'
>
> FIELD := <name> | <name> INDEX | <name> OFFSET | <name> OFFSET INDEX
>
> @@ -243,3 +244,26 @@ The above will take the parameter value, add it by 4, then index it by two
> 8 byte words. It's the same in C as: (u64 *)((void *)param + 4)[2]
>
> Note: "int skb[32]" is the same as "int skb+4[31]".
> +
> +
> +Symbols (function names)
> +========================
> +
> +To display kallsyms "%pS" type of output, use the special type "symbol".
> +
> +Again, using gdb to find the offset of the "func" field of struct work_struct
> +
> +(gdb) printf "%d\n", &((struct work_struct *)0)->func
> +24
> +
> + Both "symbol func[3]" and "symbol func+24[0]" will work.
> +
> + # echo '__queue_work(int cpu, x64 wq, symbol func[3])' > function_events
> +
> + # echo 1 > events/functions/__queue_work/enable
> + # cat trace
> + bash-1641 [007] d..2 6241.171332: queue_work_on->__queue_work(cpu=128, wq=ffff88011a010e00, func=flush_to_ldisc+0x0/0xa0)
> + bash-1641 [007] d..2 6241.171460: queue_work_on->__queue_work(cpu=128, wq=ffff88011a010e00, func=flush_to_ldisc+0x0/0xa0)
> + <idle>-0 [000] dNs3 6241.172004: delayed_work_timer_fn->__queue_work(cpu=128, wq=ffff88011a010800, func=vmstat_shepherd+0x0/0xb0)
> + worker/0:2-1689 [000] d..2 6241.172026: __queue_delayed_work->__queue_work(cpu=7, wq=ffff88011a11da00, func=vmstat_update+0x0/0x70)
> + <idle>-0 [005] d.s3 6241.347996: queue_work_on->__queue_work(cpu=128, wq=ffff88011a011200, func=fb_flashcursor+0x0/0x110 [fb])
> diff --git a/kernel/trace/trace_event_ftrace.c b/kernel/trace/trace_event_ftrace.c
> index 0f2650e97e49..ba10177b9bd6 100644
> --- a/kernel/trace/trace_event_ftrace.c
> +++ b/kernel/trace/trace_event_ftrace.c
> @@ -76,6 +76,7 @@ typedef u64 x64;
> typedef u32 x32;
> typedef u16 x16;
> typedef u8 x8;
> +typedef void * symbol;
>
> #define TYPE_TUPLE(type) \
> { #type, sizeof(type), is_signed_type(type) }
> @@ -97,7 +98,8 @@ typedef u8 x8;
> TYPE_TUPLE(x16), \
> TYPE_TUPLE(u8), \
> TYPE_TUPLE(s8), \
> - TYPE_TUPLE(x8)
> + TYPE_TUPLE(x8), \
> + TYPE_TUPLE(symbol)
>
> static struct func_type {
> char *name;
> @@ -262,7 +264,7 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
> switch (state) {
> case FUNC_STATE_INIT:
> unsign = 0;
> - if (!isalpha(token[0]))
> + if (!isalpha(token[0]) && token[0] != '_')
> break;

Hmm.. it seems that it needs to be moved to the patch 1?

> /* Do not allow wild cards */
> if (strstr(token, "*") || strstr(token, "?"))
> @@ -305,7 +307,7 @@ process_event(struct func_event *fevent, const char *token, enum func_states sta
> return FUNC_STATE_TYPE;
>
> case FUNC_STATE_TYPE:
> - if (!isalpha(token[0]))
> + if (!isalpha(token[0]) || token[0] == '_')
> break;

Why is different that the above? Anyway, '_' is not an alphabet..

Thanks,
Namhyung

> if (WARN_ON(!fevent->last_arg))
> break;
> @@ -472,6 +474,11 @@ static void make_fmt(struct func_arg *arg, char *fmt)
> {
> int c = 0;
>
> + if (arg->func_type == FUNC_TYPE_symbol) {
> + strcpy(fmt, "%pS");
> + return;
> + }
> +
> fmt[c++] = '%';
>
> if (arg->size == 8) {
> --
> 2.15.1
>
>