Re: bt(5), btrace(8): execute END probe and print maps after exit() statement
On Sat, Oct 21, 2023 at 07:17:05PM +0200, Martin Pieuchot wrote:
> On 18/10/23(Wed) 12:56, Scott Cheloha wrote:
> > Hi,
> >
> > A bt(5) exit() statement causes the btrace(8) interpreter to exit(3)
> > immediately.
> >
> > A BPFtrace exit() statement is more nuanced: the END probe is executed
> > and the contents of all maps are printed before the interpreter exits.
> >
> > This patch adds a halting check after the execution of each bt(5)
> > statement. If a statement causes the program to halt, the halt
> > bubbles up to the top-level rule evaluation loop and terminates
> > execution. rules_teardown() then runs, just as if the program had
> > received SIGTERM.
> >
> > Two edge-like cases:
> >
> > 1. You can exit() from BEGIN. rules_setup() returns non-zero if this
> >happens so the main loop knows to halt immediately.
> >
> > 2. You can exit() from END. This is just an early-return: the END probe
> >doesn't run again.
> >
> > Thoughts?
>
> Makes sense to ease the transition from bpftrace scripts. Ok with me if
> you make sure the regression tests still pass. Some outputs might
> depend on the actual behavior and would need to be updated.
Agh, my mistake, there are two tests that depend on the current
behavior. I have updated them below.
ok with the test fixes?
Index: usr.sbin/btrace/btrace.c
===
RCS file: /cvs/src/usr.sbin/btrace/btrace.c,v
retrieving revision 1.79
diff -u -p -r1.79 btrace.c
--- usr.sbin/btrace/btrace.c12 Oct 2023 15:16:44 - 1.79
+++ usr.sbin/btrace/btrace.c22 Oct 2023 01:21:21 -
@@ -71,10 +71,10 @@ struct dtioc_probe_info *dtpi_get_by_val
* Main loop and rule evaluation.
*/
voidrules_do(int);
-voidrules_setup(int);
-voidrules_apply(int, struct dt_evt *);
+int rules_setup(int);
+int rules_apply(int, struct dt_evt *);
voidrules_teardown(int);
-voidrule_eval(struct bt_rule *, struct dt_evt *);
+int rule_eval(struct bt_rule *, struct dt_evt *);
voidrule_printmaps(struct bt_rule *);
/*
@@ -84,7 +84,7 @@ uint64_t builtin_nsecs(struct dt_evt *
const char *builtin_kstack(struct dt_evt *);
const char *builtin_arg(struct dt_evt *, enum bt_argtype);
struct bt_arg *fn_str(struct bt_arg *, struct dt_evt *, char *);
-voidstmt_eval(struct bt_stmt *, struct dt_evt *);
+int stmt_eval(struct bt_stmt *, struct dt_evt *);
voidstmt_bucketize(struct bt_stmt *, struct dt_evt *);
voidstmt_clear(struct bt_stmt *);
voidstmt_delete(struct bt_stmt *, struct dt_evt *);
@@ -405,6 +405,7 @@ void
rules_do(int fd)
{
struct sigaction sa;
+ int halt = 0;
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
@@ -415,9 +416,9 @@ rules_do(int fd)
if (sigaction(SIGTERM, &sa, NULL))
err(1, "sigaction");
- rules_setup(fd);
+ halt = rules_setup(fd);
- while (!quit_pending && g_nprobes > 0) {
+ while (!quit_pending && !halt && g_nprobes > 0) {
static struct dt_evt devtbuf[64];
ssize_t rlen;
size_t i;
@@ -434,8 +435,11 @@ rules_do(int fd)
if ((rlen % sizeof(struct dt_evt)) != 0)
err(1, "incorrect read");
- for (i = 0; i < rlen / sizeof(struct dt_evt); i++)
- rules_apply(fd, &devtbuf[i]);
+ for (i = 0; i < rlen / sizeof(struct dt_evt); i++) {
+ halt = rules_apply(fd, &devtbuf[i]);
+ if (halt)
+ break;
+ }
}
rules_teardown(fd);
@@ -484,7 +488,7 @@ rules_action_scan(struct bt_stmt *bs)
return evtflags;
}
-void
+int
rules_setup(int fd)
{
struct dtioc_probe_info *dtpi;
@@ -493,7 +497,7 @@ rules_setup(int fd)
struct bt_probe *bp;
struct bt_stmt *bs;
struct bt_arg *ba;
- int dokstack = 0, on = 1;
+ int dokstack = 0, halt = 0, on = 1;
uint64_t evtflags;
TAILQ_FOREACH(r, &g_rules, br_next) {
@@ -553,7 +557,7 @@ rules_setup(int fd)
clock_gettime(CLOCK_REALTIME, &bt_devt.dtev_tsp);
if (rbegin)
- rule_eval(rbegin, &bt_devt);
+ halt = rule_eval(rbegin, &bt_devt);
/* Enable all probes */
TAILQ_FOREACH(r, &g_rules, br_next) {
@@ -571,9 +575,14 @@ rules_setup(int fd)
if (ioctl(fd, DTIOCRECORD, &on))
err(1, "DTIOCRECORD");
}
+
+ return halt;
}
-void
+/*
+ * Returns non-zero if the program should halt.
+ */
+int
rules_apply(int fd, struct dt_evt *dtev)
{
struct bt_rule *r;
Re: bt(5), btrace(8): execute END probe and print maps after exit() statement
On 18/10/23(Wed) 12:56, Scott Cheloha wrote:
> Hi,
>
> A bt(5) exit() statement causes the btrace(8) interpreter to exit(3)
> immediately.
>
> A BPFtrace exit() statement is more nuanced: the END probe is executed
> and the contents of all maps are printed before the interpreter exits.
>
> This patch adds a halting check after the execution of each bt(5)
> statement. If a statement causes the program to halt, the halt
> bubbles up to the top-level rule evaluation loop and terminates
> execution. rules_teardown() then runs, just as if the program had
> received SIGTERM.
>
> Two edge-like cases:
>
> 1. You can exit() from BEGIN. rules_setup() returns non-zero if this
>happens so the main loop knows to halt immediately.
>
> 2. You can exit() from END. This is just an early-return: the END probe
>doesn't run again.
>
> Thoughts?
Makes sense to ease the transition from bpftrace scripts. Ok with me if
you make sure the regression tests still pass. Some outputs might
depend on the actual behavior and would need to be updated.
>
> $ btrace -e '
> BEGIN {
> @[probe] = "reached";
> exit();
> @[probe] = "not reached";
> }
> END {
> @[probe] = "reached";
> exit();
> @[probe] = "not reached";
> }'
>
> Index: btrace.c
> ===
> RCS file: /cvs/src/usr.sbin/btrace/btrace.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 btrace.c
> --- btrace.c 12 Oct 2023 15:16:44 - 1.79
> +++ btrace.c 18 Oct 2023 17:54:16 -
> @@ -71,10 +71,10 @@ struct dtioc_probe_info *dtpi_get_by_val
> * Main loop and rule evaluation.
> */
> void rules_do(int);
> -void rules_setup(int);
> -void rules_apply(int, struct dt_evt *);
> +int rules_setup(int);
> +int rules_apply(int, struct dt_evt *);
> void rules_teardown(int);
> -void rule_eval(struct bt_rule *, struct dt_evt *);
> +int rule_eval(struct bt_rule *, struct dt_evt *);
> void rule_printmaps(struct bt_rule *);
>
> /*
> @@ -84,7 +84,7 @@ uint64_t builtin_nsecs(struct dt_evt *
> const char *builtin_kstack(struct dt_evt *);
> const char *builtin_arg(struct dt_evt *, enum bt_argtype);
> struct bt_arg*fn_str(struct bt_arg *, struct dt_evt *, char
> *);
> -void stmt_eval(struct bt_stmt *, struct dt_evt *);
> +int stmt_eval(struct bt_stmt *, struct dt_evt *);
> void stmt_bucketize(struct bt_stmt *, struct dt_evt *);
> void stmt_clear(struct bt_stmt *);
> void stmt_delete(struct bt_stmt *, struct dt_evt *);
> @@ -405,6 +405,7 @@ void
> rules_do(int fd)
> {
> struct sigaction sa;
> + int halt = 0;
>
> memset(&sa, 0, sizeof(sa));
> sigemptyset(&sa.sa_mask);
> @@ -415,9 +416,9 @@ rules_do(int fd)
> if (sigaction(SIGTERM, &sa, NULL))
> err(1, "sigaction");
>
> - rules_setup(fd);
> + halt = rules_setup(fd);
>
> - while (!quit_pending && g_nprobes > 0) {
> + while (!quit_pending && !halt && g_nprobes > 0) {
> static struct dt_evt devtbuf[64];
> ssize_t rlen;
> size_t i;
> @@ -434,8 +435,11 @@ rules_do(int fd)
> if ((rlen % sizeof(struct dt_evt)) != 0)
> err(1, "incorrect read");
>
> - for (i = 0; i < rlen / sizeof(struct dt_evt); i++)
> - rules_apply(fd, &devtbuf[i]);
> + for (i = 0; i < rlen / sizeof(struct dt_evt); i++) {
> + halt = rules_apply(fd, &devtbuf[i]);
> + if (halt)
> + break;
> + }
> }
>
> rules_teardown(fd);
> @@ -484,7 +488,7 @@ rules_action_scan(struct bt_stmt *bs)
> return evtflags;
> }
>
> -void
> +int
> rules_setup(int fd)
> {
> struct dtioc_probe_info *dtpi;
> @@ -493,7 +497,7 @@ rules_setup(int fd)
> struct bt_probe *bp;
> struct bt_stmt *bs;
> struct bt_arg *ba;
> - int dokstack = 0, on = 1;
> + int dokstack = 0, halt = 0, on = 1;
> uint64_t evtflags;
>
> TAILQ_FOREACH(r, &g_rules, br_next) {
> @@ -553,7 +557,7 @@ rules_setup(int fd)
> clock_gettime(CLOCK_REALTIME, &bt_devt.dtev_tsp);
>
> if (rbegin)
> - rule_eval(rbegin, &bt_devt);
> + halt = rule_eval(rbegin, &bt_devt);
>
> /* Enable all probes */
> TAILQ_FOREACH(r, &g_rules, br_next) {
> @@ -571,9 +575,14 @@ rules_setup(int fd)
> if (ioctl(fd, DTIOCRECORD, &on))
> err(1, "DTIOCRECORD");
> }
> +
> + return halt;
> }
>
> -void
> +/*
> + * Returns non-zero if the program should halt.
> + */
> +int
> rules_apply(int fd, struct dt_evt *dtev)
> {
> struct bt_r
bt(5), btrace(8): execute END probe and print maps after exit() statement
Hi,
A bt(5) exit() statement causes the btrace(8) interpreter to exit(3)
immediately.
A BPFtrace exit() statement is more nuanced: the END probe is executed
and the contents of all maps are printed before the interpreter exits.
This patch adds a halting check after the execution of each bt(5)
statement. If a statement causes the program to halt, the halt
bubbles up to the top-level rule evaluation loop and terminates
execution. rules_teardown() then runs, just as if the program had
received SIGTERM.
Two edge-like cases:
1. You can exit() from BEGIN. rules_setup() returns non-zero if this
happens so the main loop knows to halt immediately.
2. You can exit() from END. This is just an early-return: the END probe
doesn't run again.
Thoughts?
$ btrace -e '
BEGIN {
@[probe] = "reached";
exit();
@[probe] = "not reached";
}
END {
@[probe] = "reached";
exit();
@[probe] = "not reached";
}'
Index: btrace.c
===
RCS file: /cvs/src/usr.sbin/btrace/btrace.c,v
retrieving revision 1.79
diff -u -p -r1.79 btrace.c
--- btrace.c12 Oct 2023 15:16:44 - 1.79
+++ btrace.c18 Oct 2023 17:54:16 -
@@ -71,10 +71,10 @@ struct dtioc_probe_info *dtpi_get_by_val
* Main loop and rule evaluation.
*/
voidrules_do(int);
-voidrules_setup(int);
-voidrules_apply(int, struct dt_evt *);
+int rules_setup(int);
+int rules_apply(int, struct dt_evt *);
voidrules_teardown(int);
-voidrule_eval(struct bt_rule *, struct dt_evt *);
+int rule_eval(struct bt_rule *, struct dt_evt *);
voidrule_printmaps(struct bt_rule *);
/*
@@ -84,7 +84,7 @@ uint64_t builtin_nsecs(struct dt_evt *
const char *builtin_kstack(struct dt_evt *);
const char *builtin_arg(struct dt_evt *, enum bt_argtype);
struct bt_arg *fn_str(struct bt_arg *, struct dt_evt *, char *);
-voidstmt_eval(struct bt_stmt *, struct dt_evt *);
+int stmt_eval(struct bt_stmt *, struct dt_evt *);
voidstmt_bucketize(struct bt_stmt *, struct dt_evt *);
voidstmt_clear(struct bt_stmt *);
voidstmt_delete(struct bt_stmt *, struct dt_evt *);
@@ -405,6 +405,7 @@ void
rules_do(int fd)
{
struct sigaction sa;
+ int halt = 0;
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
@@ -415,9 +416,9 @@ rules_do(int fd)
if (sigaction(SIGTERM, &sa, NULL))
err(1, "sigaction");
- rules_setup(fd);
+ halt = rules_setup(fd);
- while (!quit_pending && g_nprobes > 0) {
+ while (!quit_pending && !halt && g_nprobes > 0) {
static struct dt_evt devtbuf[64];
ssize_t rlen;
size_t i;
@@ -434,8 +435,11 @@ rules_do(int fd)
if ((rlen % sizeof(struct dt_evt)) != 0)
err(1, "incorrect read");
- for (i = 0; i < rlen / sizeof(struct dt_evt); i++)
- rules_apply(fd, &devtbuf[i]);
+ for (i = 0; i < rlen / sizeof(struct dt_evt); i++) {
+ halt = rules_apply(fd, &devtbuf[i]);
+ if (halt)
+ break;
+ }
}
rules_teardown(fd);
@@ -484,7 +488,7 @@ rules_action_scan(struct bt_stmt *bs)
return evtflags;
}
-void
+int
rules_setup(int fd)
{
struct dtioc_probe_info *dtpi;
@@ -493,7 +497,7 @@ rules_setup(int fd)
struct bt_probe *bp;
struct bt_stmt *bs;
struct bt_arg *ba;
- int dokstack = 0, on = 1;
+ int dokstack = 0, halt = 0, on = 1;
uint64_t evtflags;
TAILQ_FOREACH(r, &g_rules, br_next) {
@@ -553,7 +557,7 @@ rules_setup(int fd)
clock_gettime(CLOCK_REALTIME, &bt_devt.dtev_tsp);
if (rbegin)
- rule_eval(rbegin, &bt_devt);
+ halt = rule_eval(rbegin, &bt_devt);
/* Enable all probes */
TAILQ_FOREACH(r, &g_rules, br_next) {
@@ -571,9 +575,14 @@ rules_setup(int fd)
if (ioctl(fd, DTIOCRECORD, &on))
err(1, "DTIOCRECORD");
}
+
+ return halt;
}
-void
+/*
+ * Returns non-zero if the program should halt.
+ */
+int
rules_apply(int fd, struct dt_evt *dtev)
{
struct bt_rule *r;
@@ -586,9 +595,11 @@ rules_apply(int fd, struct dt_evt *dtev)
continue;
dtai_cache(fd, &dt_dtpis[dtev->dtev_pbn - 1]);
- rule_eval(r, dtev);
+ if (rule_eval(r, dtev))
+ return 1;
}
}
+ return 0;
}
void
@@ -637,7 +648,10 @@ rules
