Re: Question about a patch for stable

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Wed, Mar 13, 2013 at 10:09:05PM -0400, Steven Rostedt wrote:
> Hi Greg,
> 
> I was bringing Documentation/trace/ftrace.txt up to date, and obviously
> I found some minor bugs in the code while doing so :-)  One of the
> things I've discovered, is that the new -mfentry option for gcc used for
> function tracing (for only x86 and gcc >= 4.6.0), the stack tracer gives
> some bogus results:
> 
>      # cat stack_trace
>             Depth    Size   Location    (48 entries)
>             -----    ----   --------
>       0)     4824     216   ftrace_call+0x5/0x2f
>       1)     4608     112   ____cache_alloc+0xb7/0x22d
>       2)     4496      80   kmem_cache_alloc+0x63/0x12f
> [...]
> 
> One thing is that it shows the ftrace_call label of the function tracer
> instead of the top function and its stack. This is due to fentry being
> called as the first operation of a function instead of the way mcount is
> called, which is after the stack frame is set up. Because the function
> is traced before the stack frame was set up, we lose what function
> called the current function. Not only that, the stack frame size (216)
> is a combination of that function we missed as well as the ftrace_call
> stack size used to save registers.
> 
> I have a couple of small fixes that make this more correct:
> 
>      # cat stack_trace
>             Depth    Size   Location    (14 entries)
>             -----    ----   --------
>       0)     2640      48   update_group_power+0x26/0x187
>       1)     2592     224   update_sd_lb_stats+0x2a5/0x4ac
>       2)     2368     160   find_busiest_group+0x31/0x1f1
>       3)     2208     256   load_balance+0xd9/0x662
> 
> The bug only affects that first entry. Do you think its worth adding to
> stable. The changes are solely contained in kernel/trace/trace_stack.c
> and do not affect anything else.
> 
> I'm not sure I'm going to even submit this for 3.9, and just let it go
> into 3.9.1.
> 
> I also noticed that the contents of the file stack_max_size doesn't
> match the depth (2640 from above), because it also includes the stack
> size of the overhead of the stack tracer itself. I have a fix for that
> too, but I believe this has always been broken (with and without
> -mfentry) and I'm not sure that deserves to be back ported. I'm going to
> queue the fix for 3.10. I'm not sure people care about this one or not.
> 
> What's your thoughts?

I always defer to the maintainer of the subsystem, so it's up to you.

greg k-h