tracing: Remove per event trace registering
in [Kernel]
|
Prev: [PATCH v5 6/9] KVM MMU: support keeping sp live while it's out of protection
Next: net: Improve ks8851 snl transmit performance
From: Frederic Weisbecker on 7 May 2010 00:30 On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote: > From: Steven Rostedt <srostedt(a)redhat.com> > > This patch removes the register functions of TRACE_EVENT() to enable > and disable tracepoints. The registering of a event is now down > directly in the trace_events.c file. The tracepoint_probe_register() > is now called directly. > > The prototypes are no longer type checked, but this should not be > an issue since the tracepoints are created automatically by the > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then > other macros will catch it. Agreed. Typechecking matters for human code but not in this context. Considering that the tracepoint and the probe are created by the same CPP code, bugs will be tracked down quickly and located to a single place. > > The trace_event_class structure now holds the probes to be called > by the callbacks. This removes needing to have each event have > a separate pointer for the probe. > > To handle kprobes and syscalls, since they register probes in a > different manner, a "reg" field is added to the ftrace_event_class > structure. If the "reg" field is assigned, then it will be called for > enabling and disabling of the probe for either ftrace or perf. To let > the reg function know what is happening, a new enum (trace_reg) is > created that has the type of control that is needed. > > With this new rework, the 82 kernel events and 616 syscall events > has their footprint dramatically lowered: > > text data bss dec hex filename > 5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig > 5792282 1333796 9351592 16477670 fb6de6 vmlinux.class > 5793448 1333780 9351592 16478820 fb7264 vmlinux.tracepoint > 5796926 1337748 9351592 16486266 fb8f7a vmlinux.data > 5774316 1306580 9351592 16432488 fabd68 vmlinux.regs > > The size went from 16477030 to 16432488, that's a total of 44K > in savings. With tracepoints being continuously added, this is > critical that the footprint becomes minimal. > > v2: Changed the callback probes to pass void * and typecast the > value within the function. > > Signed-off-by: Steven Rostedt <rostedt(a)goodmis.org> Very nice!! Acked-by: Frederic Weisbecker <fweisbec(a)gmail.com> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mathieu Desnoyers on 7 May 2010 11:00 * Frederic Weisbecker (fweisbec(a)gmail.com) wrote: > On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote: > > From: Steven Rostedt <srostedt(a)redhat.com> > > > > This patch removes the register functions of TRACE_EVENT() to enable > > and disable tracepoints. The registering of a event is now down > > directly in the trace_events.c file. The tracepoint_probe_register() > > is now called directly. > > > > The prototypes are no longer type checked, but this should not be > > an issue since the tracepoints are created automatically by the > > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then > > other macros will catch it. > > > > Agreed. Typechecking matters for human code but not in this context. > Considering that the tracepoint and the probe are created by the same > CPP code, bugs will be tracked down quickly and located to a single > place. So it seems that I am the only one asking for extra type-checking and caring about problems that can appear subtily on architectures where the number of caller/callee arguments must match. And also the only one considering that passing more arguments to a callback that does not expect all of them might be a problem on some architectures. Am I the only one thinking there is something fishy there ? I might be entirely over-paranoid, but this approach has rarely failed me in the past. Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mathieu Desnoyers on 7 May 2010 11:40 * Steven Rostedt (rostedt(a)goodmis.org) wrote: > On Fri, 2010-05-07 at 10:54 -0400, Mathieu Desnoyers wrote: > > * Frederic Weisbecker (fweisbec(a)gmail.com) wrote: > > > On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote: > > > > From: Steven Rostedt <srostedt(a)redhat.com> > > > > > > > > This patch removes the register functions of TRACE_EVENT() to enable > > > > and disable tracepoints. The registering of a event is now down > > > > directly in the trace_events.c file. The tracepoint_probe_register() > > > > is now called directly. > > > > > > > > The prototypes are no longer type checked, but this should not be > > > > an issue since the tracepoints are created automatically by the > > > > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then > > > > other macros will catch it. > > > > > > > > > > > > Agreed. Typechecking matters for human code but not in this context. > > > Considering that the tracepoint and the probe are created by the same > > > CPP code, bugs will be tracked down quickly and located to a single > > > place. > > > > So it seems that I am the only one asking for extra type-checking and > > caring about problems that can appear subtily on architectures where the > > number of caller/callee arguments must match. And also the only one > > considering that passing more arguments to a callback that does not > > expect all of them might be a problem on some architectures. > > > > Am I the only one thinking there is something fishy there ? I might be > > entirely over-paranoid, but this approach has rarely failed me in the > > past. > > I think you are the only one not realizing that the caller and callee > are created automatically with the same data. There is no human > intervention here. > > You are asking to add a check that I can not see helping. The only way > to add a check, is to use the automated process to check the automation. > If the automated process fails, it is very likely the check will also be > broken and will not catch the bug either. Tell me where to fetch the git head, I'll add it myself. ;) Thanks, Mathieu > > -- Steve > > -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 7 May 2010 14:10 On Fri, May 07, 2010 at 10:54:38AM -0400, Mathieu Desnoyers wrote: > * Frederic Weisbecker (fweisbec(a)gmail.com) wrote: > > On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote: > > > From: Steven Rostedt <srostedt(a)redhat.com> > > > > > > This patch removes the register functions of TRACE_EVENT() to enable > > > and disable tracepoints. The registering of a event is now down > > > directly in the trace_events.c file. The tracepoint_probe_register() > > > is now called directly. > > > > > > The prototypes are no longer type checked, but this should not be > > > an issue since the tracepoints are created automatically by the > > > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then > > > other macros will catch it. > > > > > > > > Agreed. Typechecking matters for human code but not in this context. > > Considering that the tracepoint and the probe are created by the same > > CPP code, bugs will be tracked down quickly and located to a single > > place. > > So it seems that I am the only one asking for extra type-checking and > caring about problems that can appear subtily on architectures where the > number of caller/callee arguments must match. And also the only one > considering that passing more arguments to a callback that does not > expect all of them might be a problem on some architectures. > > Am I the only one thinking there is something fishy there ? I might be > entirely over-paranoid, but this approach has rarely failed me in the > past. You are confusing two different and unrelated issues here. One is the lost typechecking when we _register_ a probe because we now use directly tracepoint_probe_register() for that. I don't care personally because both tracepoint and probe are created by the same automation. There is very few risk of callback type mess and if there is, it will be located in a small and isolated code. The second is this extra parameter passed whether or not it is needed. And although we suppose it is safe, I don't feel comfortable with it. So if we can find a more proper way to avoid it, I'm all for it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 7 May 2010 16:10
On Fri, May 07, 2010 at 03:08:25PM -0400, Steven Rostedt wrote: > On Fri, 2010-05-07 at 20:01 +0200, Frederic Weisbecker wrote: > > On Fri, May 07, 2010 at 10:54:38AM -0400, Mathieu Desnoyers wrote: > > > The second is this extra parameter passed whether or not it is needed. > > And although we suppose it is safe, I don't feel comfortable with it. > > So if we can find a more proper way to avoid it, I'm all for it. > > Now I'm making the extra parameter mandatory for all tracepoint > probes. ;-) > > But this time, it will be at the start not the end. > > void probe(void *data, proto); > > > Unfortunately we can't avoid it. In order to remove the extra code > (registering and unregistering) and even share the probe among several > events, we need a way to pass the data to the probe to let the probe > know what event it is dealing with (to put in the event id into the > buffer, to let the tracer output code know what event this data is for). > > The current method is that only the proto that the tracepoint uses is > passed to the probe. This gives us no way to add any more information. > > This new method allows data to be assigned at probe register, and the > probe gets this data as the first parameter. > > The register_* functions will still do typechecking of the probes, they > just add the "void *" at the beginning. > > Actually, here is a place that I can see where Mathieu's check does come > in handy. If we add the check test to each probe, and the tracepoint > proto changes, it will flag it. > > Mathieu, you've been explaining this wrong ;-) > > I'm not worried about changes to ftrace.h breaking things. I'm worried > about changes to tracepoint.h breaking ftrace.h. This is where your > check comes in. As I change the void *data from the end to the start, > I'm nervous about catching all the probes that are registered this way. > (ftrace events, syscalls, kprobes, and perf) Yeah right, I see the point. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |