Messages in this thread | | | Date | Tue, 17 Jan 2012 03:22:31 +0100 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 8/9] stacktrace: implement save_stack_trace_quick() |
| |
On Wed, Jan 11, 2012 at 08:38:26AM -0800, Tejun Heo wrote: > Hello, Frederic. > > On Wed, Jan 11, 2012 at 05:26:44PM +0100, Frederic Weisbecker wrote: > > On Tue, Jan 10, 2012 at 10:28:25AM -0800, Tejun Heo wrote: > > > Implement save_stack_trace_quick() which only considers the usual > > > contexts (ie. thread and irq) and doesn't handle links between > > > different contexts - if %current is in irq context, only backtrace in > > > the irq stack is considered. > > > > The thing I don't like is the duplication that involves not only on > > stack unwinding but also on the safety checks. > > I'm not entirely convinced that this is necessary or we can just add > more features to the existing backtrace facility (and maybe make that > more efficient) and be done with it.
Yeah probably we can do that.
> > > > This is subset of dump_trace() done in much simpler way. It's > > > intended to be used in hot paths where the overhead of dump_trace() > > > can be too heavy. > > > > Is it? Have you found a measurable impact (outside the fact you record only > > one stack). > > As I wrote in the head message, I haven't done comparative test yet > but in the preliminary tests the CPU overhead against memory backed > device is quite visible (roughly ~20%), so I expect it to matter. > Note that testing against memory backed device is actually relevant, > on faster SSDs, CPU is already a bottleneck. > > It would be best if we can extend the existing one to cover all the > cases with acceptable overhead. I needed to write this minimal > version anyway for comparison so it's posted together but no matter > how it turns out switching them isn't difficult.
Right. So there are two major differences that may affect performances between save_stack_trace() and save_stack_trace_quick():
- save_stack_trace() does a full walk through the stack, but it rejects unreliable entries. So to begin with, it should use print_context_stack_bp() that does a frame pointer walk only (in CONFIG_FRAME_POINTER case).
- It links between stacks. Doing the ->stack() that returns a value should help in this regard.
- And dump_stack() does various more checks, perhaps we can simplify it a bit.
| |