Index | Thread | Search

From:
Christian Ludwig <cludwig@genua.de>
Subject:
btrace: Support additional interval/profile units
To:
<tech@openbsd.org>
Cc:
Martin Pieuchot <mpi@grenadille.net>
Date:
Mon, 6 Jan 2025 16:54:33 +0100

Download raw body.

Thread
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	<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