Messages in this thread |  | | | Date | Mon, 13 Apr 2009 23:37:45 +0200 | | From | Ingo Molnar <> | | Subject | Re: [PATCH 2/2] ext4: Convert instrumentation from markers to tracepoints |
| |
* Theodore Ts'o <tytso@mit.edu> wrote:
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > --- > fs/ext4/ext4.h | 22 +-- > fs/ext4/ext4_i.h | 12 - > fs/ext4/fsync.c | 8 +- > fs/ext4/ialloc.c | 17 +- > fs/ext4/inode.c | 79 ++--- > fs/ext4/mballoc.c | 84 ++--- > fs/ext4/mballoc.h | 68 +---- > fs/ext4/super.c | 6 +- > include/linux/ext4_trace_types.h | 108 ++++++ > include/trace/ext4.h | 8 + > include/trace/ext4_event_types.h | 690 +++++++++++++++++++++++++++++++++++++ > include/trace/trace_event_types.h | 1 + > include/trace/trace_events.h | 1 + > 13 files changed, 886 insertions(+), 218 deletions(-) > create mode 100644 include/linux/ext4_trace_types.h > create mode 100644 include/trace/ext4.h > create mode 100644 include/trace/ext4_event_types.h
Really nice stuff!
A general, high-level observation: it is visible that the conversion to TRACE_EVENTS() does increase the total linecount, but it also removes code from the most often used source code areas (fs/ext4/*.c) - and using an intuitive function-call-alike single-line tracepoint there. I'd call that a positive effect.
It would still be nice to compact the 690 lines of (out of line) ext4_event_types.h definitions some more - and i think we'll be able to do it without affecting the usage sites.
OTOH, even those bits look fairly readable, and there's a fair amount of bang-for-bucks ratio - these tracepoints give us:
- integrated tracepoints - zero-copy and per-cpu splice() based tracing - binary tracing without printf overhead - structured logging records exposed under /debug/tracing/events - trace events embedded in function tracer output and other plugins - user-defined, per tracepoint filter expressions
... so even if we were to keep the 690 lines of overhead with no further compacting, it still looks like a good deal to me, all things considered.
Ingo
|  |