From: Christian Ludwig Subject: btrace: Only allow one probe of each type To: Cc: Martin Pieuchot Date: Tue, 3 Dec 2024 09:10:50 +0100 Hi, btrace(8) recognizes dt(4) events by their kernel probe number. That means if a bt(5) script has the same probe multiple times, btrace cannot tell which rules to execute. It simply executes all of them. That is not a problem for regular probes. But it is for 'interval' and 'profile' probes, because the interval is lost. The following script illustrates that: # btrace -e ' interval:hz:1 { print("1"); } interval:hz:10 { print("10"); } ' 1 10 1 10 1 10 ^C This diff disallows to register multiple probes of the same type. That means the script above fails. This really only affects 'interval' and 'profile' probes. For all other probe types, their rules can be merged into one block. - Christian --- regress/usr.sbin/btrace/Makefile | 2 +- regress/usr.sbin/btrace/multiend.bt | 5 +++ regress/usr.sbin/btrace/multiend.ok | 1 + regress/usr.sbin/btrace/multiprobe.bt | 11 +---- regress/usr.sbin/btrace/multiprobe.ok | 3 +- sys/dev/dt/dt_dev.c | 7 +++ usr.sbin/btrace/btrace.c | 82 +++++++++++++++++++++-------------- 7 files changed, 66 insertions(+), 45 deletions(-) diff --git a/regress/usr.sbin/btrace/Makefile b/regress/usr.sbin/btrace/Makefile index 10fc07fd0913..5af18e53b181 100644 --- a/regress/usr.sbin/btrace/Makefile +++ b/regress/usr.sbin/btrace/Makefile @@ -7,7 +7,7 @@ ALLOWDT!= sysctl -n kern.allowdt 2>/dev/null BT_LANG_SCRIPTS= arithm beginend beginend-argn boolean comments \ delete exit histempty if \ map mapclear mapempty mapsyntax mapzero map-unnamed \ - maxoperand min+max+sum multismts nsecs+var \ + maxoperand min+max+sum multiend multismts nsecs+var \ precedence print printf read-map-after-clear \ staticv-empty syntaxerror tuple tupleeval vareval diff --git a/regress/usr.sbin/btrace/multiend.bt b/regress/usr.sbin/btrace/multiend.bt new file mode 100644 index 000000000000..a18395e23c14 --- /dev/null +++ b/regress/usr.sbin/btrace/multiend.bt @@ -0,0 +1,5 @@ +END +{ } + +END +{ } diff --git a/regress/usr.sbin/btrace/multiend.ok b/regress/usr.sbin/btrace/multiend.ok new file mode 100644 index 000000000000..246e6a822256 --- /dev/null +++ b/regress/usr.sbin/btrace/multiend.ok @@ -0,0 +1 @@ +btrace: Cannot register multiple probes of the same type: 'END' diff --git a/regress/usr.sbin/btrace/multiprobe.bt b/regress/usr.sbin/btrace/multiprobe.bt index fbee65916c10..efe0137f48c7 100644 --- a/regress/usr.sbin/btrace/multiprobe.bt +++ b/regress/usr.sbin/btrace/multiprobe.bt @@ -1,10 +1,3 @@ -BEGIN, -interval:hz:50 -{ - printf("multi\n"); -} - +interval:hz:50, interval:hz:100 -{ - exit(); -} +{ } diff --git a/regress/usr.sbin/btrace/multiprobe.ok b/regress/usr.sbin/btrace/multiprobe.ok index 9b34aa063164..5ccc865dc52f 100644 --- a/regress/usr.sbin/btrace/multiprobe.ok +++ b/regress/usr.sbin/btrace/multiprobe.ok @@ -1,2 +1 @@ -multi -multi +btrace: Cannot register multiple probes of the same type: 'interval:hz:100' diff --git a/sys/dev/dt/dt_dev.c b/sys/dev/dt/dt_dev.c index 0d9e72822642..c077b6fb0f36 100644 --- a/sys/dev/dt/dt_dev.c +++ b/sys/dev/dt/dt_dev.c @@ -589,6 +589,7 @@ dt_ioctl_probe_enable(struct dt_softc *sc, struct dtioc_req *dtrq) { struct dt_pcb_list plist; struct dt_probe *dtp; + struct dt_pcb *dp; int error; SIMPLEQ_FOREACH(dtp, &dt_probe_list, dtp_next) { @@ -598,6 +599,12 @@ dt_ioctl_probe_enable(struct dt_softc *sc, struct dtioc_req *dtrq) if (dtp == NULL) return ENOENT; + /* Only allow one probe of each type. */ + TAILQ_FOREACH(dp, &sc->ds_pcbs, dp_snext) { + if (dp->dp_dtp->dtp_pbn == dtrq->dtrq_pbn) + return EEXIST; + } + TAILQ_INIT(&plist); error = dtp->dtp_prov->dtpv_alloc(dtp, sc, &plist, dtrq); if (error) diff --git a/usr.sbin/btrace/btrace.c b/usr.sbin/btrace/btrace.c index e755431be8c8..085b8a3f8db4 100644 --- a/usr.sbin/btrace/btrace.c +++ b/usr.sbin/btrace/btrace.c @@ -69,6 +69,8 @@ struct dtioc_probe_info *dtpi_get_by_value(const char *, const char *, /* * Main loop and rule evaluation. */ +void probe_bail(struct bt_probe *); +const char *probe_name(struct bt_probe *); void rules_do(int); int rules_setup(int); int rules_apply(int, struct dt_evt *); @@ -106,7 +108,6 @@ int ba2dtflags(struct bt_arg *); __dead void xabort(const char *, ...); void debug(const char *, ...); void debugx(const char *, ...); -const char *debug_probe_name(struct bt_probe *); void debug_dump_term(struct bt_arg *); void debug_dump_expr(struct bt_arg *); void debug_dump_filter(struct bt_rule *); @@ -399,6 +400,37 @@ dtpi_get_by_value(const char *prov, const char *func, const char *name) return NULL; } +void +probe_bail(struct bt_probe *bp) +{ + errx(1, "Cannot register multiple probes of the same type: '%s'", + probe_name(bp)); +} + +const char * +probe_name(struct bt_probe *bp) +{ + static char buf[64]; + + if (bp->bp_type == B_PT_BEGIN) + return "BEGIN"; + + if (bp->bp_type == B_PT_END) + return "END"; + + assert(bp->bp_type == B_PT_PROBE); + + if (bp->bp_rate) { + snprintf(buf, sizeof(buf), "%s:%s:%u", bp->bp_prov, + bp->bp_unit, bp->bp_rate); + } else { + snprintf(buf, sizeof(buf), "%s:%s:%s", bp->bp_prov, + bp->bp_unit, bp->bp_name); + } + + return buf; +} + void rules_do(int fd) { @@ -497,7 +529,7 @@ rules_setup(int fd) { struct dtioc_probe_info *dtpi; struct dtioc_req *dtrq; - struct bt_rule *r, *rbegin = NULL; + struct bt_rule *r, *rbegin = NULL, *rend = NULL; struct bt_probe *bp; struct bt_stmt *bs; struct bt_arg *ba; @@ -519,12 +551,20 @@ rules_setup(int fd) evtflags |= rules_action_scan(SLIST_FIRST(&r->br_action)); SLIST_FOREACH(bp, &r->br_probes, bp_next) { - debug("parsed probe '%s'", debug_probe_name(bp)); + debug("parsed probe '%s'", probe_name(bp)); debug_dump_filter(r); if (bp->bp_type != B_PT_PROBE) { - if (bp->bp_type == B_PT_BEGIN) + if (bp->bp_type == B_PT_BEGIN) { + if (rbegin != NULL) + probe_bail(bp); rbegin = r; + } + if (bp->bp_type == B_PT_END) { + if (rend != NULL) + probe_bail(bp); + rend = r; + } continue; } @@ -570,8 +610,11 @@ rules_setup(int fd) continue; dtrq = bp->bp_cookie; - if (ioctl(fd, DTIOCPRBENABLE, dtrq)) + if (ioctl(fd, DTIOCPRBENABLE, dtrq)) { + if (errno == EEXIST) + probe_bail(bp); err(1, "DTIOCPRBENABLE"); + } } } @@ -662,7 +705,7 @@ rule_eval(struct bt_rule *r, struct dt_evt *dtev) struct bt_probe *bp; SLIST_FOREACH(bp, &r->br_probes, bp_next) { - debug("eval rule '%s'", debug_probe_name(bp)); + debug("eval rule '%s'", probe_name(bp)); debug_dump_filter(r); } @@ -2041,33 +2084,6 @@ debug_dump_filter(struct bt_rule *r) debugx("/\n"); } -const char * -debug_probe_name(struct bt_probe *bp) -{ - static char buf[64]; - - if (verbose < 2) - return ""; - - if (bp->bp_type == B_PT_BEGIN) - return "BEGIN"; - - if (bp->bp_type == B_PT_END) - return "END"; - - assert(bp->bp_type == B_PT_PROBE); - - if (bp->bp_rate) { - snprintf(buf, sizeof(buf), "%s:%s:%u", bp->bp_prov, - bp->bp_unit, bp->bp_rate); - } else { - snprintf(buf, sizeof(buf), "%s:%s:%s", bp->bp_prov, - bp->bp_unit, bp->bp_name); - } - - return buf; -} - unsigned long dt_get_offset(pid_t pid) {