Messages in this thread | | | Subject | Re: [patch 00/37] Linux Kernel Markers instrumentation for sched-devel.git | From | Peter Zijlstra <> | Date | Sun, 27 Apr 2008 12:00:13 +0200 |
| |
On Sat, 2008-04-26 at 16:11 -0400, Mathieu Desnoyers wrote: > * Peter Zijlstra (a.p.zijlstra@chello.nl) wrote: > > On Thu, 2008-04-24 at 11:03 -0400, Mathieu Desnoyers wrote: > > > Hi Ingo, > > > > > > Here is a rather large patchset applying kernel instrumentation to > > > sched-devel.git. It includes, mainly : > > > > I saw this land in sched-devel, how about this: > > > > I think the main reason why those markers are ugly is they have > information meant for use by a general purpose tracer (it will only take > the parameters before the ## signs). > > The other way around is to start specializing the general purpose tracer > to extract the information it needs from the task and rq pointers and > put that in the traces. Actually, that's the approach I use currently > use in LTTng, but Ingo seemed interested to have the union of parameters > needed by specialized and GP tracers, so this is what I did. > > The only thing I dislike with the approach of putting everything in a > separatered header is that it adds a layer of indirection when one try > to to quickly grep for trace_mark() in the kernel tree to see where the > conceptual tracing points are. Therefore, it might be interesting to > simply remove the parameters meant for the general purpose tracer and > let this GP tracer create specialized probes for these instrumentation > sites. It would therefore remove the ugliness without creating a > supplementary indirection layer.
In part its the extra parameters, but to a large part its that darn string. I'm still not getting why we can't just live with trace marks like 'regular' functions much like the ones I used to wrap trace_mark().
> > Index: linux-2.6-2/kernel/sched_trace.h > > =================================================================== > > --- /dev/null > > +++ linux-2.6-2/kernel/sched_trace.h > > @@ -0,0 +1,41 @@ > > +#include <linux/marker.h> > > + > > +static inline void trace_kernel_sched_wait(struct task_struct *p) > > +{ > > + trace_mark(kernel_sched_wait_task, "pid %d state %ld", > > + p->pid, p->state); > > +} > > + > > +static inline > > +void trace_kernel_sched_wakeup(struct rq *rq, struct task_struct *p) > > +{ > > + trace_mark(kernel_sched_wakeup, > > + "pid %d state %ld ## rq %p task %p rq->curr %p", > > + p->pid, p->state, rq, p, rq->curr); > > +} > > + > > +static inline > > +void trace_kernel_sched_wakeup_new(struct rq *rq, struct task_struct *p) > > +{ > > + trace_mark(kernel_sched_wakeup_new, > > + "pid %d state %ld ## rq %p task %p rq->curr %p", > > + p->pid, p->state, rq, p, rq->curr); > > +} > > + > > +static inline void trace_kernel_sched_switch(struct rq *rq, > > + struct task_struct *prev, struct task_struct *next) > > +{ > > + trace_mark(kernel_sched_schedule, > > + "prev_pid %d next_pid %d prev_state %ld " > > + "## rq %p prev %p next %p", > > + prev->pid, next->pid, prev->state, > > + rq, prev, next); > > +} > > + > > +static inline void > > +trace_kernel_sched_migrate_task(struct task_struct *p, int src, int dst) > > +{ > > + trace_mark(kernel_sched_migrate_task, > > + "pid %d state %ld dest_cpu %d", > > + p->pid, p->state, dst); > > +}
One advantage of having these things close together would be that it stimulates consistency between the various trace points. Something that would be sorely needed with such a free form mechanism.
Peter
| |