From: Martin Pieuchot Subject: Re: dt: Trace tracing program To: Christian Ludwig Cc: "tech@openbsd.org" Date: Sat, 30 Mar 2024 09:22:59 +0100 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? 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 *);