lkml.org 
[lkml]   [2008]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
DateSun, 23 Nov 2008 20:35:54 +0100
FromIngo Molnar <>
SubjectRe: [PATCH] tracing/function-return-tracer: don't trace kfree while it frees the return stack

* Steven Rostedt <rostedt@goodmis.org> 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


\
 
 \ /
  Last update: 2008-11-23 20:39    [from the cache]
©2003-2010