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

Steven Rostedt
2009-02-17T18:54:16Z

On Tue, 17 Feb 2009, Paul E. McKenney wrote:
> > 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.

Thanks! I should have CC'd you on that. I thought that was not right.
Will fix.

-- Steve

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