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

[Frederic Weisbecker]

Ingo Molnar a écrit :
> note that we also need to keep gcc from reordering things here (no 
> matter how unlikely in this particular case).
> 
> (also, small detail: we prefer a newline after variable definitions.)
> 
> Full commit attached below.
> 

I guessed that since tsk->ret_stack is referenced for the last time before the call to kfree,
the registers to memory flushing would be done before the call.

But you're right, I should prevent from possible compiler strange behaviours, thanks!




Ingo Molnar a écrit :
> * Ingo Molnar <mingo@xxxxxxx> wrote:
> 
>>  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;
>> +	/* NULL must become visible to IRQs before we free it: */
>> +	barrier();
>> +
>> +	kfree(ret_stack);
>>  }
> 
> hm, this reminds me - this is the wrong place for the callback - the 
> call stack is freed too early.
> 
> instead of freeing it in do_exit() (like it's done right now), it 
> should be freed when the task struct and thread info is freed: in 
> kernel/fork.c:free_task() - okay?
> 
> The difference is minor but could allow more complete tracing: as 
> right now we'll skip the entries that get generated when we schedule 
> away from a dead task.
> 
> 	Ingo


That's right. Even if the noret code path will not be traced, we are loosing some traces.
That should be fixed with the patch below (didn't tested widely but seems good).
--