Download raw body.
btrace: Support additional interval/profile units
On 06/01/25(Mon) 16:54, Christian Ludwig wrote:
> 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.
ok mpi@
> ---
> 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 <v.i> ERROR ENDFILT
> %token <v.i> OP_EQ OP_NE OP_LE OP_LT OP_GE OP_GT OP_LAND OP_LOR
> /* Builtins */
> -%token <v.i> BUILTIN BEGIN ELSE END HZ IF STR
> +%token <v.i> BUILTIN BEGIN ELSE END IF STR
> /* Functions and Map operators */
> %token <v.i> F_DELETE F_PRINT
> %token <v.i> 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
>
btrace: Support additional interval/profile units