Download raw body.
dt: Trace tracing program
On Sat, Mar 30, 2024 at 09:22:59AM +0100, Martin Pieuchot wrote:
> On 16/02/24(Fri) 15:21, Christian Ludwig wrote:
> > Hi,
> >
> > sorry for the late reply.
> >
> > On Mon, 2024-02-05 at 16:14 +0100, Martin Pieuchot wrote:
> > > On 02/02/24(Fri) 16:24, Christian Ludwig wrote:
> > > > For the sake of visibility, also trace the tracing program itself.
> > > > If you really wanted to hide it, add a filter to your probes, like:
> > >
> > > I'm not sure about that. Why do you want to see the tracing program?
> > > What are you trying to monitor?
> >
> > I am trying to monitor interrupt events using the TRACEPOINT macro. But
> > in the current form, the TRACEPOINT macro simply ignores the tracing
> > program, which leads to seemingly "lost" interrupts. Hence the diff.
> >
> > I think it's not worth adding another macro, TRACEPOINT does everything
> > I need, once it's adapted to be safe to call in all interrupt contexts.
> > I am working on that.
> >
> > OTOH exposing the tracing program also makes it easier to better
> > connect the dots concering side effects introduced by the tracer. And
> > if you really want to hide it you can use a filter explicitly.
>
> I believe now is the right time to do this change. I'd like commit the
> more complete diff below which completely remove kernel filtering.
> Kernel filtering was used in the early days of dt(4) but now all
> filtering is done is userland.
>
> ok?
But this adds a lot of extra code to drop unwanted messages and as a
result you may drop messages because the ring overflowed.
So I'm not sure if this is the right move.
> Index: dev/dt/dt_dev.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
> diff -u -p -r1.32 dt_dev.c
> --- dev/dt/dt_dev.c 29 Feb 2024 00:18:48 -0000 1.32
> +++ dev/dt/dt_dev.c 30 Mar 2024 08:13:08 -0000
> @@ -348,30 +348,6 @@ dtlookup(int unit)
> }
>
> int
> -dtioc_req_isvalid(struct dtioc_req *dtrq)
> -{
> - switch (dtrq->dtrq_filter.dtf_operand) {
> - case DT_OP_NONE:
> - case DT_OP_EQ:
> - case DT_OP_NE:
> - break;
> - default:
> - return 0;
> - }
> -
> - switch (dtrq->dtrq_filter.dtf_variable) {
> - case DT_FV_NONE:
> - case DT_FV_PID:
> - case DT_FV_TID:
> - break;
> - default:
> - return 0;
> - }
> -
> - return 1;
> -}
> -
> -int
> dt_ioctl_list_probes(struct dt_softc *sc, struct dtioc_probe *dtpr)
> {
> struct dtioc_probe_info info, *dtpi;
> @@ -552,9 +528,6 @@ dt_ioctl_probe_enable(struct dt_softc *s
> struct dt_probe *dtp;
> int error;
>
> - if (!dtioc_req_isvalid(dtrq))
> - return EINVAL;
> -
> SIMPLEQ_FOREACH(dtp, &dt_probe_list, dtp_next) {
> if (dtp->dtp_pbn == dtrq->dtrq_pbn)
> break;
> @@ -582,9 +555,6 @@ dt_ioctl_probe_disable(struct dt_softc *
> struct dt_probe *dtp;
> int error;
>
> - if (!dtioc_req_isvalid(dtrq))
> - return EINVAL;
> -
> SIMPLEQ_FOREACH(dtp, &dt_probe_list, dtp_next) {
> if (dtp->dtp_pbn == dtrq->dtrq_pbn)
> break;
> @@ -711,48 +681,6 @@ dt_pcb_purge(struct dt_pcb_list *plist)
> }
> }
>
> -int
> -dt_pcb_filter(struct dt_pcb *dp)
> -{
> - struct dt_filter *dtf = &dp->dp_filter;
> - struct proc *p = curproc;
> - unsigned int var = 0;
> - int match = 1;
> -
> - /* Filter out tracing program. */
> - if (dp->dp_sc->ds_pid == p->p_p->ps_pid)
> - return 1;
> -
> - switch (dtf->dtf_variable) {
> - case DT_FV_PID:
> - var = p->p_p->ps_pid;
> - break;
> - case DT_FV_TID:
> - var = p->p_tid + THREAD_PID_OFFSET;
> - break;
> - case DT_FV_NONE:
> - break;
> - default:
> - KASSERT(0);
> - }
> -
> - switch (dtf->dtf_operand) {
> - case DT_OP_EQ:
> - match = !!(var == dtf->dtf_value);
> - break;
> - case DT_OP_NE:
> - match = !!(var != dtf->dtf_value);
> - break;
> - case DT_OP_NONE:
> - break;
> - default:
> - KASSERT(0);
> - }
> -
> - return !match;
> -}
> -
> -
> /*
> * Get a reference to the next free event state from the ring.
> */
> @@ -762,9 +690,6 @@ dt_pcb_ring_get(struct dt_pcb *dp, int p
> struct proc *p = curproc;
> struct dt_evt *dtev;
> int distance;
> -
> - if (dt_pcb_filter(dp))
> - return NULL;
>
> mtx_enter(&dp->dp_mtx);
> distance = dp->dp_prod - dp->dp_cons;
> Index: dev/dt/dt_prov_profile.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/dt/dt_prov_profile.c,v
> diff -u -p -r1.7 dt_prov_profile.c
> --- dev/dt/dt_prov_profile.c 13 Mar 2024 13:13:57 -0000 1.7
> +++ dev/dt/dt_prov_profile.c 30 Mar 2024 08:19:15 -0000
> @@ -72,7 +72,6 @@ dt_prov_profile_alloc(struct dt_probe *d
> CPU_INFO_ITERATOR cii;
> extern int hz;
>
> - KASSERT(dtioc_req_isvalid(dtrq));
> KASSERT(TAILQ_EMPTY(plist));
> KASSERT(dtp == dtpp_profile || dtp == dtpp_interval);
>
> @@ -92,7 +91,6 @@ dt_prov_profile_alloc(struct dt_probe *d
> dp->dp_nsecs = SEC_TO_NSEC(1) / dtrq->dtrq_rate;
> dp->dp_cpu = ci;
>
> - dp->dp_filter = dtrq->dtrq_filter;
> dp->dp_evtflags = dtrq->dtrq_evtflags & DTEVT_PROV_PROFILE;
> TAILQ_INSERT_HEAD(plist, dp, dp_snext);
> }
> Index: dev/dt/dt_prov_static.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/dt/dt_prov_static.c,v
> diff -u -p -r1.22 dt_prov_static.c
> --- dev/dt/dt_prov_static.c 28 Aug 2023 14:50:01 -0000 1.22
> +++ dev/dt/dt_prov_static.c 30 Mar 2024 08:19:00 -0000
> @@ -179,14 +179,12 @@ dt_prov_static_alloc(struct dt_probe *dt
> {
> struct dt_pcb *dp;
>
> - KASSERT(dtioc_req_isvalid(dtrq));
> KASSERT(TAILQ_EMPTY(plist));
>
> dp = dt_pcb_alloc(dtp, sc);
> if (dp == NULL)
> return ENOMEM;
>
> - dp->dp_filter = dtrq->dtrq_filter;
> dp->dp_evtflags = dtrq->dtrq_evtflags;
> TAILQ_INSERT_HEAD(plist, dp, dp_snext);
>
> Index: dev/dt/dt_prov_syscall.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/dt/dt_prov_syscall.c,v
> diff -u -p -r1.8 dt_prov_syscall.c
> --- dev/dt/dt_prov_syscall.c 13 Apr 2023 02:19:05 -0000 1.8
> +++ dev/dt/dt_prov_syscall.c 30 Mar 2024 08:19:29 -0000
> @@ -101,7 +101,6 @@ dt_prov_syscall_alloc(struct dt_probe *d
> {
> struct dt_pcb *dp;
>
> - KASSERT(dtioc_req_isvalid(dtrq));
> KASSERT(TAILQ_EMPTY(plist));
> KASSERT(dtp->dtp_prov == &dt_prov_syscall);
> KASSERT((dtp->dtp_sysnum >= 0) && (dtp->dtp_sysnum < dtps_nsysent));
> @@ -110,7 +109,6 @@ dt_prov_syscall_alloc(struct dt_probe *d
> if (dp == NULL)
> return ENOMEM;
>
> - dp->dp_filter = dtrq->dtrq_filter;
> dp->dp_evtflags = dtrq->dtrq_evtflags & DTEVT_PROV_SYSCALL;
> TAILQ_INSERT_HEAD(plist, dp, dp_snext);
>
> Index: dev/dt/dtvar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/dt/dtvar.h,v
> diff -u -p -r1.18 dtvar.h
> --- dev/dt/dtvar.h 9 Feb 2024 17:42:18 -0000 1.18
> +++ dev/dt/dtvar.h 30 Mar 2024 08:15:56 -0000
> @@ -82,26 +82,6 @@ struct dt_evt {
> "\003KSTACK" \
> "\004FUNCARGS" \
>
> -/*
> - * Each PCB can have a filter attached to itself. A filter do not
> - * prevent an enabled probe to fire, but when that happens, event
> - * states are only recorded if it is matched.
> - */
> -struct dt_filter {
> - enum dt_operand {
> - DT_OP_NONE = 0,
> - DT_OP_EQ,
> - DT_OP_NE,
> - } dtf_operand;
> - enum dt_filtervar {
> - DT_FV_NONE = 0,
> - DT_FV_PID,
> - DT_FV_TID,
> - } dtf_variable /* what should be filtered */;
> - unsigned int dtf_value; /* PID or TID to filter */
> -};
> -
> -
> struct dtioc_probe_info {
> uint32_t dtpi_pbn; /* probe number */
> uint8_t dtpi_nargs; /* # of arguments */
> @@ -129,7 +109,6 @@ struct dtioc_arg {
>
> struct dtioc_req {
> uint32_t dtrq_pbn; /* probe number */
> - struct dt_filter dtrq_filter; /* probe filter */
> uint32_t dtrq_rate; /* number of ticks */
> uint64_t dtrq_evtflags; /* states to record */
> };
> @@ -165,8 +144,6 @@ struct dtioc_getaux {
>
> struct dt_softc;
>
> -int dtioc_req_isvalid(struct dtioc_req *);
> -
> /*
> * Probe control block, possibly per-CPU.
> *
> @@ -195,7 +172,6 @@ struct dt_pcb {
> struct dt_softc *dp_sc; /* [I] related softc */
> struct dt_probe *dp_dtp; /* [I] related probe */
> uint64_t dp_evtflags; /* [I] event states to record */
> - struct dt_filter dp_filter; /* [I] filter to match */
>
> /* Provider specific fields. */
> struct clockintr dp_clockintr; /* [D] profiling handle */
> @@ -211,7 +187,6 @@ TAILQ_HEAD(dt_pcb_list, dt_pcb);
> struct dt_pcb *dt_pcb_alloc(struct dt_probe *, struct dt_softc *);
> void dt_pcb_free(struct dt_pcb *);
> void dt_pcb_purge(struct dt_pcb_list *);
> -int dt_pcb_filter(struct dt_pcb *);
>
> struct dt_evt *dt_pcb_ring_get(struct dt_pcb *, int);
> void dt_pcb_ring_consume(struct dt_pcb *, struct dt_evt *);
>
--
:wq Claudio
dt: Trace tracing program