Re: [PATCH] tracing/function-return-tracer: don't trace kfreewhile it frees the return stack

From: Ingo Molnar
Date: Sun Nov 23 2008 - 14:36:18 EST



* Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

>
> On Sun, 23 Nov 2008, Ingo Molnar wrote:
> > >
> > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > > index 90d99fb..ffff7ec 100644
> > > --- a/kernel/trace/ftrace.c
> > > +++ b/kernel/trace/ftrace.c
> > > @@ -1628,8 +1628,9 @@ void ftrace_retfunc_init_task(struct task_struct *t)
> > >
> > > void ftrace_retfunc_exit_task(struct task_struct *t)
> > > {
> > > - kfree(t->ret_stack);
> > > + struct ftrace_ret_stack *ret_stack = t->ret_stack;
> > > t->ret_stack = NULL;
> > > + kfree(ret_stack);
> >
> > heh, nice one :)
> >
> > note that we also need to keep gcc from reordering things here (no
> > matter how unlikely in this particular case).
>
> I first thought that too, but thinking about it, if gcc does do that, then
> it will break the logic for a correct C program.
>
> t is passed in as a pointer, then it modifies the contents of t
> (which could be a global pointer), then it calls a external
> function, that might also reference the global pointer.
>
> This means that if it were to reorder the two, it would break C,
> because the compiler can not assume that the called function will
> read the global pointer either.
>
> In other words, the compiler should not need to worry about SMP or
> modifications done by interrupts or other threads. But the compiler
> should always preserve the order that is assumed by a single
> context.

Correct, but this assumes that kfree is a C function. Which it might
not necessarily be: it could be optimized via an inline in certain
cases, etc. It's best to document such cases explicitly.

In any case, the real solution is what i suggested in the previous
mail, to do the freeing from the task-struct freeing path in
kernel/fork.c:free_task() - that has other advantages as well.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/