Messages in this thread | | | Date | Tue, 19 Oct 2010 18:38:43 -0400 | From | Jason Baron <> | Subject | Re: [PATCH] tracing: Cleanup the convoluted softirq tracepoints |
| |
On Tue, Oct 19, 2010 at 11:55:19PM +0200, Thomas Gleixner wrote: > On Tue, 19 Oct 2010, Jason Baron wrote: > > On Tue, Oct 19, 2010 at 09:49:45PM +0200, Thomas Gleixner wrote: > > > > > On Tue, 19 Oct 2010, Steven Rostedt wrote: > > > > > > So it trades a conditional vs. two jumps ? WTF ?? > > > > > > > right, so the 'jmpq' on boot on x86 gets patched with 5 byte no-op > > sequence. So in the disabled case we have no-op followed by a jump > > around the disabled code. > > And that's supposed to be useful ? We do _NOT_ want to jump around > disabled stuff. The noped out case should fall through into the non > traced code. Otherwise that whole jumplabel thing is completely > useless. > > > > I thought that jumplabel magic was supposed to get rid of the jump > > > over the tracing code ? In fact it adds another jump. Whatfor ? > > > > > > > yes, that is the plan. gcc does not yet support hot/cold labels...once > > it does the second jump will go away and the entire tracepoint code will > > be moved to a 'cold' section. It's not quite completely optimal yet, but > > we are getting there. > > Then do not advertise it as the brilliant solution for all tracing > matters. >
I'm not sure I did, the documentation says that we have nop followed by a jmp:
+The new code is a 'nopl' followed by a 'jmp'. Thus: + +nopl - 0f 1f 44 00 00 - 5 bytes +jmp - eb 3e - 2 bytes
http://marc.info/?l=linux-kernel&m=128717355231182&w=2`
> > > Now even worse, when you NOP out the jmpq then your tracepoint is > > > still not enabled. Brilliant ! > > > > > > > The 'jmpq' in the enabled case is patched with a jmpq to the body of the > > tracepoint itself. > > Brilliant. > > > > Did you guys ever look at the assembly output of that insane shite you > > > are advertising with lengthy explanations ? > > > > > > Obviously _NOT_ > > > > > > Come back when you can show me a clean imlementation of all this crap > > > which reproduces with my jumplabel enabled stock compiler. And please > > > just send me a patch w/o the blurb. > > > > > > And sane looks like: > > > > > > jmpq 2f <---- This gets noped out > > > 1: > > > mov %r12,%rdi > > > callq *(%r12) > > > [whatever cleanup it takes ] > > > leaveq > > > retq > > > > > > 2f: > > > [tracing gunk] > > > jmp 1b > > > > > > > yes, this is what the code should look like when we get support for > > hot/cold labels. I've discussed this support with gcc folk, and its the > > next step here. So yes, this is exacatly where we are headed. > > So and at the same time the whole tracing crowd tells me, that this is > already a done deal. See previous advertisments from DrTracing. I'm > seriously grumpy about this especially in the context of a patch which > fixes one of the worst interfaces I've seen in years. > > Thanks, > > tglx
sorry if I mislead anybody about the current state of of 'jump labels'. But we have the same goal in mind, and a clear path to get there. If you don't agree with the approach - I'm all ears. And you are right - the code is not where it should be yet.
thanks,
-Jason
| |