Re: Question about a patch for stable

From: Greg Kroah-Hartman
Date: Wed Mar 13 2013 - 22:47:26 EST


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
--
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/