From: Christian Ludwig Subject: btrace: Support additional interval/profile units To: Cc: Martin Pieuchot Date: Mon, 6 Jan 2025 16:54:33 +0100 Hi, this is an update to cheloha@'s diff at https://marc.info/?l=openbsd-tech&m=170917477404018 and allows arbitrary periods for interval/profile probes in btrace(8). It implements additional units for these probes: s, ms, us. Notable changes from the original diff: * Kill dtrq_rate, use dtrq_nsecs everywhere. * Adjust parser errors / debug messages. * A regression test. Tests and feedback welcome. - Christian --- regress/usr.sbin/btrace/Makefile | 3 +- regress/usr.sbin/btrace/interval.bt | 8 ++++ regress/usr.sbin/btrace/interval.ok | 3 ++ sys/dev/dt/dt_prov_profile.c | 9 ++-- sys/dev/dt/dtvar.h | 2 +- usr.sbin/btrace/bt_parse.y | 67 +++++++++++++++++++++-------- usr.sbin/btrace/bt_parser.h | 4 +- usr.sbin/btrace/btrace.c | 65 ++++++++++++++++++---------- 8 files changed, 114 insertions(+), 47 deletions(-) create mode 100644 regress/usr.sbin/btrace/interval.bt create mode 100644 regress/usr.sbin/btrace/interval.ok diff --git a/regress/usr.sbin/btrace/Makefile b/regress/usr.sbin/btrace/Makefile index 9214f221f44e..05a33b933dff 100644 --- a/regress/usr.sbin/btrace/Makefile +++ b/regress/usr.sbin/btrace/Makefile @@ -14,7 +14,8 @@ BT_LANG_SCRIPTS= arithm beginend beginend-argn boolean comments \ BT_ARG_LANG_SCRIPTS= staticv str # scripts that use kernel probes -BT_KERN_SCRIPTS= argn empty-stmts filters mapoverwrite multiprobe +BT_KERN_SCRIPTS= argn empty-stmts filters interval mapoverwrite \ + multiprobe REGRESS_EXPECTED_FAILURES= run-maxoperand diff --git a/regress/usr.sbin/btrace/interval.bt b/regress/usr.sbin/btrace/interval.bt new file mode 100644 index 000000000000..c975d0a3187a --- /dev/null +++ b/regress/usr.sbin/btrace/interval.bt @@ -0,0 +1,8 @@ +interval:ms:300 { + @count = @count + 1; + printf("Probe: %d\n", @count); +} + +profile:s:1 { + exit(); +} diff --git a/regress/usr.sbin/btrace/interval.ok b/regress/usr.sbin/btrace/interval.ok new file mode 100644 index 000000000000..a2b653b166d3 --- /dev/null +++ b/regress/usr.sbin/btrace/interval.ok @@ -0,0 +1,3 @@ +Probe: 1 +Probe: 2 +Probe: 3 diff --git a/sys/dev/dt/dt_prov_profile.c b/sys/dev/dt/dt_prov_profile.c index 394cda9cd284..68fd258ddef3 100644 --- a/sys/dev/dt/dt_prov_profile.c +++ b/sys/dev/dt/dt_prov_profile.c @@ -67,16 +67,17 @@ int dt_prov_profile_alloc(struct dt_probe *dtp, struct dt_softc *sc, struct dt_pcb_list *plist, struct dtioc_req *dtrq) { + uint64_t nsecs; struct dt_pcb *dp; struct cpu_info *ci; CPU_INFO_ITERATOR cii; - extern int hz; KASSERT(TAILQ_EMPTY(plist)); KASSERT(dtp == dtpp_profile || dtp == dtpp_interval); - if (dtrq->dtrq_rate <= 0 || dtrq->dtrq_rate > hz) - return EOPNOTSUPP; + nsecs = dtrq->dtrq_nsecs; + if (nsecs < USEC_TO_NSEC(200) || nsecs > SEC_TO_NSEC(UINT32_MAX)) + return EINVAL; CPU_INFO_FOREACH(cii, ci) { if (!CPU_IS_PRIMARY(ci) && (dtp == dtpp_interval)) @@ -88,7 +89,7 @@ dt_prov_profile_alloc(struct dt_probe *dtp, struct dt_softc *sc, return ENOMEM; } - dp->dp_nsecs = SEC_TO_NSEC(1) / dtrq->dtrq_rate; + dp->dp_nsecs = nsecs; dp->dp_cpu = ci; dp->dp_evtflags = dtrq->dtrq_evtflags & DTEVT_PROV_PROFILE; diff --git a/sys/dev/dt/dtvar.h b/sys/dev/dt/dtvar.h index 1ca135d5eb49..00c84fb20510 100644 --- a/sys/dev/dt/dtvar.h +++ b/sys/dev/dt/dtvar.h @@ -109,7 +109,7 @@ struct dtioc_arg { struct dtioc_req { uint32_t dtrq_pbn; /* probe number */ - uint32_t dtrq_rate; /* number of ticks */ + uint64_t dtrq_nsecs; /* execution period */ uint64_t dtrq_evtflags; /* states to record */ }; diff --git a/usr.sbin/btrace/bt_parse.y b/usr.sbin/btrace/bt_parse.y index 6e23b7bcd2da..33062420b84c 100644 --- a/usr.sbin/btrace/bt_parse.y +++ b/usr.sbin/btrace/bt_parse.y @@ -62,7 +62,7 @@ struct bt_arg g_maxba = BA_INITIALIZER(LONG_MAX, B_AT_LONG); struct bt_rule *br_new(struct bt_probe *, struct bt_filter *, struct bt_stmt *); -struct bt_probe *bp_new(const char *, const char *, const char *, int32_t); +struct bt_probe *bp_new(const char *, const char *, const char *, long); struct bt_arg *ba_append(struct bt_arg *, struct bt_arg *); struct bt_arg *ba_op(enum bt_argtype, struct bt_arg *, struct bt_arg *); struct bt_stmt *bs_new(enum bt_action, struct bt_arg *, struct bt_var *); @@ -119,7 +119,7 @@ static int beflag = 0; /* BEGIN/END parsing context flag */ %token ERROR ENDFILT %token OP_EQ OP_NE OP_LE OP_LT OP_GE OP_GT OP_LAND OP_LOR /* Builtins */ -%token BUILTIN BEGIN ELSE END HZ IF STR +%token BUILTIN BEGIN ELSE END IF STR /* Functions and Map operators */ %token F_DELETE F_PRINT %token MFUNC FUNC0 FUNC1 FUNCN OP1 OP2 OP4 MOP0 MOP1 @@ -155,7 +155,7 @@ probe : { pflag = 1; } pname { $$ = $2; pflag = 0; } ; pname : STRING ':' STRING ':' STRING { $$ = bp_new($1, $3, $5, 0); } - | STRING ':' HZ ':' NUMBER { $$ = bp_new($1, "hz", NULL, $5); } + | STRING ':' STRING ':' NUMBER { $$ = bp_new($1, $3, NULL, $5); } ; mentry : GVAR '[' vargs ']' { $$ = bm_find($1, $3); } @@ -358,18 +358,55 @@ bt_new(struct bt_arg *ba, struct bt_stmt *condbs, struct bt_stmt *elsebs) return bs_new(B_AC_TEST, bop, (struct bt_var *)bc); } +/* + * interval and profile support the same units. + */ +static uint64_t +bp_unit_to_nsec(const char *unit, long value) +{ + static const struct { + const char *name; + enum { UNIT_HZ, UNIT_US, UNIT_MS, UNIT_S } id; + long long max; + } units[] = { + { .name = "hz", .id = UNIT_HZ, .max = 1000000LL }, + { .name = "us", .id = UNIT_US, .max = LLONG_MAX / 1000 }, + { .name = "ms", .id = UNIT_MS, .max = LLONG_MAX / 1000000 }, + { .name = "s", .id = UNIT_S, .max = LLONG_MAX / 1000000000 }, + }; + size_t i; + + for (i = 0; i < nitems(units); i++) { + if (strcmp(units[i].name, unit) == 0) { + if (value < 1) + yyerror("Number is invalid: %ld", value); + if (value > units[i].max) + yyerror("Number is too large: %ld", value); + switch (units[i].id) { + case UNIT_HZ: + return (1000000000LLU / value); + case UNIT_US: + return (value * 1000LLU); + case UNIT_MS: + return (value * 1000000LLU); + case UNIT_S: + return (value * 1000000000LLU); + } + } + } + yyerror("Invalid unit: %s", unit); + return 0; +} + /* Create a new probe */ struct bt_probe * -bp_new(const char *prov, const char *func, const char *name, int32_t rate) +bp_new(const char *prov, const char *func, const char *name, long number) { struct bt_probe *bp; enum bt_ptype ptype; - if (rate < 0 || rate > INT32_MAX) - errx(1, "only positive values permitted"); - if (prov == NULL && func == NULL && name == NULL) - ptype = rate; /* BEGIN or END */ + ptype = number; /* BEGIN or END */ else ptype = B_PT_PROBE; @@ -379,7 +416,8 @@ bp_new(const char *prov, const char *func, const char *name, int32_t rate) bp->bp_prov = prov; bp->bp_func = func; bp->bp_name = name; - bp->bp_rate = rate; + if (ptype == B_PT_PROBE && name == NULL) + bp->bp_nsecs = bp_unit_to_nsec(func, number); bp->bp_type = ptype; return bp; @@ -726,7 +764,6 @@ lookup(char *s) { "else", ELSE, 0 }, { "exit", FUNC0, B_AC_EXIT }, { "hist", OP1, 0 }, - { "hz", HZ, 0 }, { "if", IF, 0 }, { "kstack", BUILTIN, B_AT_BI_KSTACK }, { "lhist", OP4, 0 }, @@ -1077,14 +1114,10 @@ again: /* * Probe lexer backdoor, interpret the token as a string * rather than a keyword. Otherwise, reserved keywords - * would conflict with syscall names. The exception to - * this is 'hz', which hopefully will never be a - * syscall. + * would conflict with syscall names. */ - if (kwp->token != HZ) { - yylval.v.string = kwp->word; - return STRING; - } + yylval.v.string = kwp->word; + return STRING; } else if (beflag) { /* Interpret tokens in a BEGIN/END context. */ if (kwp->type >= B_AT_BI_ARG0 && diff --git a/usr.sbin/btrace/bt_parser.h b/usr.sbin/btrace/bt_parser.h index 081c0045c2f7..579ae314a181 100644 --- a/usr.sbin/btrace/bt_parser.h +++ b/usr.sbin/btrace/bt_parser.h @@ -31,7 +31,7 @@ * * "provider:function:name" * or - * "provider:time_unit:rate" + * "provider:time_unit:number" * * Multiple probes can be associated to the same action. */ @@ -40,7 +40,7 @@ struct bt_probe { const char *bp_prov; /* provider */ const char *bp_func; /* function or time unit */ const char *bp_name; - uint32_t bp_rate; + uint64_t bp_nsecs; #define bp_unit bp_func enum bt_ptype { B_PT_BEGIN = 1, diff --git a/usr.sbin/btrace/btrace.c b/usr.sbin/btrace/btrace.c index 7c2e3dcdc22c..8cbc8ce9eadd 100644 --- a/usr.sbin/btrace/btrace.c +++ b/usr.sbin/btrace/btrace.c @@ -363,12 +363,6 @@ dtpi_func(struct dtioc_probe_info *dtpi) return syscallnames[idx]; } -int -dtpi_is_unit(const char *unit) -{ - return !strncmp("hz", unit, sizeof("hz")); -} - struct dtioc_probe_info * dtpi_get_by_value(const char *prov, const char *func, const char *name) { @@ -377,20 +371,16 @@ dtpi_get_by_value(const char *prov, const char *func, const char *name) dtpi = dt_dtpis; for (i = 0; i < dt_ndtpi; i++, dtpi++) { - if (prov != NULL && - strncmp(prov, dtpi->dtpi_prov, DTNAMESIZE)) - continue; - - if (func != NULL) { - if (dtpi_is_unit(func)) - return dtpi; - - if (strncmp(func, dtpi_func(dtpi), DTNAMESIZE)) + if (prov != NULL) { + if (strncmp(prov, dtpi->dtpi_prov, DTNAMESIZE)) + continue; + } + if (func != NULL && name != NULL) { + if (strncmp(func, dtpi_func(dtpi), DTNAMESIZE)) + continue; + if (strncmp(name, dtpi->dtpi_name, DTNAMESIZE)) continue; } - - if (strncmp(name, dtpi->dtpi_name, DTNAMESIZE)) - continue; debug("matched probe %s:%s:%s\n", dtpi->dtpi_prov, dtpi_func(dtpi), dtpi->dtpi_name); @@ -400,6 +390,37 @@ dtpi_get_by_value(const char *prov, const char *func, const char *name) return NULL; } +static uint64_t +bp_nsecs_to_unit(struct bt_probe *bp) +{ + static const struct { + const char *name; + enum { UNIT_HZ, UNIT_US, UNIT_MS, UNIT_S } id; + } units[] = { + { .name = "hz", .id = UNIT_HZ }, + { .name = "us", .id = UNIT_US }, + { .name = "ms", .id = UNIT_MS }, + { .name = "s", .id = UNIT_S }, + }; + size_t i; + + for (i = 0; i < nitems(units); i++) { + if (strcmp(units[i].name, bp->bp_unit) == 0) { + switch (units[i].id) { + case UNIT_HZ: + return (1000000000LLU / bp->bp_nsecs); + case UNIT_US: + return (bp->bp_nsecs / 1000LLU); + case UNIT_MS: + return (bp->bp_nsecs / 1000000LLU); + case UNIT_S: + return (bp->bp_nsecs / 1000000000LLU); + } + } + } + return 0; +} + void probe_bail(struct bt_probe *bp) { @@ -420,9 +441,9 @@ probe_name(struct bt_probe *bp) 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); + if (bp->bp_nsecs) { + snprintf(buf, sizeof(buf), "%s:%s:%llu", bp->bp_prov, + bp->bp_unit, bp_nsecs_to_unit(bp)); } else { snprintf(buf, sizeof(buf), "%s:%s:%s", bp->bp_prov, bp->bp_unit, bp->bp_name); @@ -582,7 +603,7 @@ rules_setup(int fd) bp->bp_pbn = dtpi->dtpi_pbn; dtrq->dtrq_pbn = dtpi->dtpi_pbn; - dtrq->dtrq_rate = bp->bp_rate; + dtrq->dtrq_nsecs = bp->bp_nsecs; dtrq->dtrq_evtflags = evtflags; if (dtrq->dtrq_evtflags & DTEVT_KSTACK) dokstack = 1; -- 2.34.1