Messages in this thread | | | Date | Mon, 8 Jun 2009 17:11:50 -0400 | From | Jason Baron <> | Subject | Re: [PATCH 2/2] convert to syscall tracepoints |
| |
On Mon, Jun 08, 2009 at 10:40:56PM +0200, Ingo Molnar wrote: > * Jason Baron <jbaron@redhat.com> wrote: > > > +#ifdef __NR_time > > +trace_event_syscall(1, time, time_t __user *, tloc); > > +#endif > > + > > +#ifdef __NR_stime > > +trace_event_syscall(1, stime, time_t __user *, tptr); > > +#endif > > + > > +#ifdef __NR_gettimeofday > > +trace_event_syscall(2, gettimeofday, struct timeval __user *, tv, struct timezone __user *, tz); > > +#endif > > This could be reduced to a single line: just add a Kconfig entry > (say TRACE_SYSCALL_TRACEPOINTS) wether an arch supports syscall > tracepoints, enable it on a sane arch, make sure it has all the > syscalls and list them ... > > As more architectures turn on SYSCALL_TRACEPOINTS, they'll have to > resolve any deviations in syscall entry points. Ideally we'd have > one generic table that covers 95% of all syscalls, and the remaining > 5% in some architecture specific #ifdef section. >
true, but this implementation works for all arches now, why would want to slowly add this over time? I think its unnecessary work that could be error prone.
> But, more generally, i'm not at all convinced that we need _any_ of > this enumeration. Look how much the above lines duplicate > DEFINE_SYSCALL macros. Why arent those macros re-used? >
The DEFINE_SYSCALL() are located all over the code in various .c files. Thus, if we define the tracpoints via the DEFINE_SYSCALL() macros we are going to have 'static inline functions' (which is how tracepoints are implemented) defined in all these .c files. Now, I need to call all these 'static inline functions' from ptrace.c. How do I do that? By defining these as I've done in syscalls.h I can include that header file and create an efficient 'case' statement. I've verified in the assembly that the 'case' statments resolve to jumps. Thus, this implementation is pretty efficient. Further, by creating a header file other users of the tracepoints can include that header. If we use DEFINE_SYSCALL() that is not possible. I'd be more than happy to use the DEFINE_SYSCALL() macros, if you could explain how to resolve the above issues...We could write a pre-processor that creates a dynamic header file from the DEFINE_SYSCALL() points? I'm not sure that's simpler...
> We dont need too smart pretty-printing i think - we only want to > know the field size and the field name - nothing else. Duplicating > all those definitions looks outright wrong to me. Do we really, > really, really have to do it? >
In terms of pretty-printing, it should be fairly easy to add size and the field name to what I have without bloating the syscalls.h file at all.
thanks,
-Jason
| |