Re: [PATCH 10/15] ftrace: trace different functions with a different tracer

Paul E. McKenney
2009-02-17T18:46:14Z

On Tue, Feb 17, 2009 at 12:12:37AM -0500, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Impact: new feature
>
> Currently, the function tracer only gives you an ability to hook
> a tracer to all functions being traced. The dynamic function trace
> allows you to pick and choose which of those functions will be
> traced, but all functions being traced will call all tracers that
> registered with the function tracer.
>
> This patch adds a new feature that allows a tracer to hook to specific
> functions, even when all functions are being traced. It allows for
> different functions to call different tracer hooks.
>
> The way this is accomplished is by a special function that will hook
> to the function tracer and will set up a hash table knowing which
> tracer hook to call with which function. This is the most general
> and easiest method to accomplish this. Later, an arch may choose
> to supply their own method in changing the mcount call of a function
> to call a different tracer. But that will be an exercise for the
> future.
>
> To register a function:
>
> struct ftrace_hook_ops {
> void (*func)(unsigned long ip,
> unsigned long parent_ip,
> void **data);
> int (*callback)(unsigned long ip, void **data);
> void (*free)(void **data);
> };
>
> int register_ftrace_function_hook(char *glob, struct ftrace_hook_ops *ops,
> void *data);
>
> glob is a simple glob to search for the functions to hook.
> ops is a pointer to the operations (listed below)
> data is the default data to be passed to the hook functions when traced
>
> ops:
> func is the hook function to call when the functions are traced
> callback is a callback function that is called when setting up the hash.
> That is, if the tracer needs to do something special for each
> function, that is being traced, and wants to give each function
> its own data. The address of the entry data is passed to this
> callback, so that the callback may wish to update the entry to
> whatever it would like.
> free is a callback for when the entry is freed. In case the tracer
> allocated any data, it is give the chance to free it.
>
> To unregister we have three functions:
>
> void
> unregister_ftrace_function_hook(char *glob, struct ftrace_hook_ops *ops,
> void *data)
>
> This will unregister all hooks that match glob, point to ops, and
> have its data matching data. (note, if glob is NULL, blank or '*',
> all functions will be tested).
>
> void
> unregister_ftrace_function_hook_func(char *glob,
> struct ftrace_hook_ops *ops)
>
> This will unregister all functions matching glob that has an entry
> pointing to ops.
>
> void unregister_ftrace_function_hook_all(char *glob)
>
> This simply unregisters all funcs.

Looks like a nice addition!

One RCU type mismatch called out below.

> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
> include/linux/ftrace.h | 18 ++++
> kernel/trace/ftrace.c | 247 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 265 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index f0a0ecc..13918c4 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -106,6 +106,24 @@ struct ftrace_func_command {
> /* asm/ftrace.h must be defined for archs supporting dynamic ftrace */
> #include <asm/ftrace.h>

[ . . . ]

> +
> +static void
> +__unregister_ftrace_function_hook(char *glob, struct ftrace_hook_ops *ops,
> + void *data, int flags)
> +{
> + struct ftrace_func_hook *entry;
> + struct hlist_node *n, *tmp;
> + char str[KSYM_SYMBOL_LEN];
> + int type = MATCH_FULL;
> + int i, len = 0;
> + char *search;
> +
> + if (glob && (strcmp(glob, "*") || !strlen(glob)))
> + glob = NULL;
> + else {
> + int not;
> +
> + type = ftrace_setup_glob(glob, strlen(glob), &search, &not);
> + len = strlen(search);
> +
> + /* we do not support '!' for function hooks */
> + if (WARN_ON(not))
> + return;
> + }
> +
> + mutex_lock(&ftrace_lock);
> + for (i = 0; i < FTRACE_FUNC_HASHSIZE; i++) {
> + struct hlist_head *hhd = &ftrace_func_hash[i];
> +
> + hlist_for_each_entry_safe(entry, n, tmp, hhd, node) {
> +
> + /* break up if statements for readability */
> + if ((flags & HOOK_TEST_FUNC) && entry->ops != ops)
> + continue;
> +
> + if ((flags & HOOK_TEST_DATA) && entry->data != data)
> + continue;
> +
> + /* do this last, since it is the most expensive */
> + if (glob) {
> + kallsyms_lookup(entry->ip, NULL, NULL,
> + NULL, str);
> + if (!ftrace_match(str, glob, len, type))
> + continue;
> + }
> +
> + hlist_del(&entry->node);
> + call_rcu(&entry->rcu, ftrace_free_entry_rcu);

This should be call_rcu_sched() to match the preemption disabling.

> + }
> + }
> + __disable_ftrace_function_hook();
> + mutex_unlock(&ftrace_lock);
> +}