[DTrace-devel] [PATCH] dtprobed: handle multiple providers in a single piece of DOF
Nick Alcock
nick.alcock at oracle.com
Thu Nov 14 16:03:52 UTC 2024
A given ELF object can of course contain as many providers as you like. The
DOF stash format, DOF stash management code and DTrace itself are all set up
fine to handle DOF with multiple providers in: but, alas, the code that
reads the DOF from the parser child in response to an ioctl() assumes that
one provider is all it will ever see.
This is not ideal, but rejigging it introduces an interesting problem: when
do you *stop* reading providers from the parser stream? Right now,
dof_parser.c loops over all of its providers and emits them until all are
done, then just stops: there is no indication of how many providers there
are or that this is the last provider or anything (though if there are none,
it does try to send an empty provider to avoid the caller blocking waiting
for a reply that will never come).
It's a bit annoying to try to figure out how many providers there are in
advance, so instead of that, take a route that's easier both to emit and
parse, drop the empty provider stuff, and simply add a DIT_EOF record which
indicates "no more providers expected", which is always sent last. (We
stuff this in before DIT_ARGS_* in the enum because it makes more conceptual
sense right after DIT_ERR, and DIT_ARGS_* hasn't released anywhere yet.)
With that in place it's a simple matter to loop adding parsed DOF to the
stash's accumulator until we hit an EOF record. Memory management is
complicated a little, because a single parsed buffer (reply from dof_parser)
is now owned by multiple accumulator records in the stash: but every one of
those is terminated by a DIT_EOF, which appears nowhere else, so we can just
not free a parsed buffer unless it's of type DIT_EOF.
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
dtprobed/dof_stash.c | 12 +++-
dtprobed/dtprobed.c | 63 ++++++++++++++-------
libcommon/dof_parser.c | 23 +++-----
libcommon/dof_parser.h | 8 ++-
test/triggers/Build | 6 +-
test/triggers/usdt-tst-multiprovider-prov.d | 12 ++++
test/triggers/usdt-tst-multiprovider.c | 22 +++++++
test/unittest/usdt/tst.multiprovider.r | 3 +
test/unittest/usdt/tst.multiprovider.r.p | 2 +
test/unittest/usdt/tst.multiprovider.sh | 15 +++++
10 files changed, 124 insertions(+), 42 deletions(-)
create mode 100644 test/triggers/usdt-tst-multiprovider-prov.d
create mode 100644 test/triggers/usdt-tst-multiprovider.c
create mode 100644 test/unittest/usdt/tst.multiprovider.r
create mode 100755 test/unittest/usdt/tst.multiprovider.r.p
create mode 100755 test/unittest/usdt/tst.multiprovider.sh
diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
index 82fdd3174759d..296987ad41e93 100644
--- a/dtprobed/dof_stash.c
+++ b/dtprobed/dof_stash.c
@@ -360,7 +360,13 @@ dof_stash_free(dt_list_t *accum)
for (accump = dt_list_next(accum); accump != NULL;
accump = dt_list_next(accump)) {
dt_list_delete(accum, accump);
- free(accump->parsed);
+
+ /*
+ * All parsed memory regions are terminated by an EOF, so once
+ * we encounter the EOF, this region can safely be freed.
+ */
+ if (accump->parsed->type == DIT_EOF)
+ free(accump->parsed);
free(last_accump);
last_accump = accump;
}
@@ -656,6 +662,10 @@ dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum)
fuse_log(FUSE_LOG_ERR, "dtprobed: parser error: %s\n",
accump->parsed->err.err);
goto err_provider_close;
+ /* EOF: nothing to do. No need to record this parser-stream
+ * implementation detail into the parsed representation. */
+ case DIT_EOF:
+ break;
default:
/*
diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index 2ca39b26f88a9..86865eb467b67 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -772,41 +772,60 @@ process_dof(pid_t pid, int out, int in, dev_t dev, ino_t inum, dev_t exec_dev,
dof_parser_tidy(1);
continue;
}
- if (provider->type != DIT_PROVIDER)
+ if (provider->type != DIT_PROVIDER && provider->type != DIT_EOF)
goto err;
break;
} while (!provider);
- if (dof_stash_push_parsed(&accum, provider) < 0)
- goto oom;
-
- for (i = 0; i < provider->provider.nprobes; i++) {
- dof_parsed_t *probe = dof_read(pid, in);
- size_t j;
-
- errmsg = "no probes in this provider, or parse state corrupt";
- if (!probe || probe->type != DIT_PROBE)
- goto err;
-
- if (dof_stash_push_parsed(&accum, probe) < 0)
+ while (provider->type != DIT_EOF) {
+ if (dof_stash_push_parsed(&accum, provider) < 0)
goto oom;
- j = 0;
- do {
- dof_parsed_t *tp = dof_read(pid, in);
+ fuse_log(FUSE_LOG_DEBUG, "Parser read: provider %s, %i probes\n",
+ provider->provider.name, provider->provider.nprobes);
- errmsg = "no tracepoints in a probe, or parse state corrupt";
- if (!tp || tp->type == DIT_PROVIDER || tp->type == DIT_PROBE)
+ for (i = 0; i < provider->provider.nprobes; i++) {
+ dof_parsed_t *probe = dof_read(pid, in);
+ size_t j;
+
+ errmsg = "no probes in this provider, or parse state corrupt";
+ if (!probe || probe->type != DIT_PROBE)
goto err;
- if (dof_stash_push_parsed(&accum, tp) < 0)
+ if (dof_stash_push_parsed(&accum, probe) < 0)
goto oom;
- if (tp->type == DIT_TRACEPOINT)
- j++;
- } while (j < probe->probe.ntp);
+ j = 0;
+ do {
+ dof_parsed_t *tp = dof_read(pid, in);
+
+ errmsg = "no tracepoints in a probe, or parse state corrupt";
+ if (!tp || tp->type == DIT_PROVIDER ||
+ tp->type == DIT_PROBE || tp->type == DIT_EOF)
+ goto err;
+
+ fuse_log(FUSE_LOG_DEBUG, "Parser read: adding %s:%s to stash\n",
+ provider->provider.name, probe->probe.name);
+
+ if (dof_stash_push_parsed(&accum, tp) < 0)
+ goto oom;
+
+ if (tp->type == DIT_TRACEPOINT)
+ j++;
+ } while (j < probe->probe.ntp);
+ }
+
+ errmsg = "subsequent provider read failed, or stream not properly terminated";
+ provider = dof_read(pid, in);
+ if (!provider)
+ goto err;
}
+ /* Push the EOF on. */
+
+ if (dof_stash_push_parsed(&accum, provider) < 0)
+ goto oom;
+
if (!reparsing)
if ((gen = dof_stash_add(pid, dev, inum, exec_dev, exec_inum, dh,
in_buf, in_bufsz)) < 0)
diff --git a/libcommon/dof_parser.c b/libcommon/dof_parser.c
index 1792a8bfcc849..ae5abe5220929 100644
--- a/libcommon/dof_parser.c
+++ b/libcommon/dof_parser.c
@@ -1145,7 +1145,7 @@ dof_parse(int out, dof_helper_t *dhp, dof_hdr_t *dof)
{
int i, rv;
uintptr_t daddr = (uintptr_t)dof;
- int count = 0;
+ dof_parsed_t eof;
dt_dbg_dof("DOF 0x%p from helper {'%s', %p, %p}...\n",
dof, dhp ? dhp->dofhp_mod : "<none>", dhp, dof);
@@ -1157,8 +1157,7 @@ dof_parse(int out, dof_helper_t *dhp, dof_hdr_t *dof)
}
/*
- * Look for helper providers, validate their descriptions, and
- * parse them.
+ * Look for providers, validate their descriptions, and parse them.
*/
if (dhp != NULL) {
dt_dbg_dof(" DOF 0x%p Validating and parsing providers...\n", dof);
@@ -1177,25 +1176,19 @@ dof_parse(int out, dof_helper_t *dhp, dof_hdr_t *dof)
dof_destroy(dhp, dof);
return;
}
- count++;
emit_provider(out, dhp, dof, sec);
}
}
/*
- * If nothing was written, emit an empty result to wake up
- * the caller.
+ * Always emit an EOF, to wake up the caller if nothing else, but also
+ * to notify the caller that there are no more providers to read.
*/
- if (count == 0) {
- dof_parsed_t empty;
+ memset(&eof, 0, sizeof(dof_parsed_t));
- memset(&empty, 0, sizeof(dof_parsed_t));
-
- empty.size = offsetof(dof_parsed_t, provider.name);
- empty.type = DIT_PROVIDER;
- empty.provider.nprobes = 0;
- dof_parser_write_one(out, &empty, empty.size);
- }
+ eof.size = offsetof(dof_parsed_t, provider.nprobes);
+ eof.type = DIT_EOF;
+ dof_parser_write_one(out, &eof, eof.size);
dof_destroy(dhp, dof);
}
diff --git a/libcommon/dof_parser.h b/libcommon/dof_parser.h
index 8f42d00551e3f..75aa3082161b7 100644
--- a/libcommon/dof_parser.h
+++ b/libcommon/dof_parser.h
@@ -24,6 +24,7 @@
* DIT_ARGS_XLAT (1, optional)
* DIT_ARGS_MAP (1, optional)
* DIT_TRACEPOINT (any number >= 1)
+ * DIT_EOF (no more providers, last record)
*
* The dof_parsed.provider.flags word indicates the presence of the
* various optional args records in the following stream (you can rely on
@@ -37,9 +38,10 @@ typedef enum dof_parsed_info {
DIT_PROBE = 1,
DIT_TRACEPOINT = 2,
DIT_ERR = 3,
- DIT_ARGS_NATIVE = 4,
- DIT_ARGS_XLAT = 5,
- DIT_ARGS_MAP = 6,
+ DIT_EOF = 4,
+ DIT_ARGS_NATIVE = 5,
+ DIT_ARGS_XLAT = 6,
+ DIT_ARGS_MAP = 7,
} dof_parsed_info_t;
/*
diff --git a/test/triggers/Build b/test/triggers/Build
index 107b3b4d1f2e8..c50c265ccec7d 100644
--- a/test/triggers/Build
+++ b/test/triggers/Build
@@ -13,7 +13,8 @@ EXTERNAL_64BIT_TRIGGERS = testprobe readwholedir mmap bogus-ioctl open delaydie
ustack-tst-spin ustack-tst-mtspin \
visible-constructor visible-constructor-static visible-constructor-static-unstripped
-EXTERNAL_64BIT_SDT_TRIGGERS = usdt-tst-argmap usdt-tst-args usdt-tst-forker usdt-tst-defer usdt-tst-special
+EXTERNAL_64BIT_SDT_TRIGGERS = usdt-tst-argmap usdt-tst-args usdt-tst-forker usdt-tst-defer \
+ usdt-tst-multiprovider usdt-tst-special
EXTERNAL_64BIT_TRIGGERS += $(EXTERNAL_64BIT_SDT_TRIGGERS)
EXTERNAL_32BIT_TRIGGERS := visible-constructor-32
@@ -206,6 +207,9 @@ usdt-tst-forker_PROV := usdt-tst-forker-prov.d
# usdt-tst-defer calls USDT probes based on dtrace -h
usdt-tst-defer_PROV := usdt-tst-defer-prov.d
+# usdt-tst-multiprovider calls USDT probes based on dtrace -h
+usdt-tst-multiprovider_PROV := usdt-tst-multiprovider-prov.d
+
# usdt-tst-special calls USDT probes based on dtrace -h
usdt-tst-special_CFLAGS := -fno-inline -O2
usdt-tst-special_PROV := usdt-tst-special-prov.d
diff --git a/test/triggers/usdt-tst-multiprovider-prov.d b/test/triggers/usdt-tst-multiprovider-prov.d
new file mode 100644
index 0000000000000..cfa8fab23233e
--- /dev/null
+++ b/test/triggers/usdt-tst-multiprovider-prov.d
@@ -0,0 +1,12 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/* @@skip: provider declaration - not a test */
+
+provider prova { probe entrya(); };
+provider provb { probe entryb(); };
+provider provc { probe entryc(); };
diff --git a/test/triggers/usdt-tst-multiprovider.c b/test/triggers/usdt-tst-multiprovider.c
new file mode 100644
index 0000000000000..430c0cdd2cf4d
--- /dev/null
+++ b/test/triggers/usdt-tst-multiprovider.c
@@ -0,0 +1,22 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/*
+ * A trigger with multiple providers in it.
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include "usdt-tst-multiprovider-prov.h"
+
+int main(int argc, char **argv)
+{
+ PROVA_ENTRYA();
+ PROVB_ENTRYB();
+ PROVC_ENTRYC();
+ usleep(5 * 1000 * 1000);
+ return 0;
+}
diff --git a/test/unittest/usdt/tst.multiprovider.r b/test/unittest/usdt/tst.multiprovider.r
new file mode 100644
index 0000000000000..b83f559a7e7ff
--- /dev/null
+++ b/test/unittest/usdt/tst.multiprovider.r
@@ -0,0 +1,3 @@
+ID provaPID usdt-tst-multiprovider main entrya
+ID provbPID usdt-tst-multiprovider main entryb
+ID provcPID usdt-tst-multiprovider main entryc
diff --git a/test/unittest/usdt/tst.multiprovider.r.p b/test/unittest/usdt/tst.multiprovider.r.p
new file mode 100755
index 0000000000000..ae8493e3d5be4
--- /dev/null
+++ b/test/unittest/usdt/tst.multiprovider.r.p
@@ -0,0 +1,2 @@
+#!/bin/sh
+grep -v '^ *ID' | sed 's,^[0-9]*,ID,; s,prov\(.\)[0-9]*,prov\1PID,; s, *, ,g'
diff --git a/test/unittest/usdt/tst.multiprovider.sh b/test/unittest/usdt/tst.multiprovider.sh
new file mode 100755
index 0000000000000..d5b72c2be77ec
--- /dev/null
+++ b/test/unittest/usdt/tst.multiprovider.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+# Licensed under the Universal Permissive License v 1.0 as shown at
+# http://oss.oracle.com/licenses/upl.
+#
+if [ $# != 1 ]; then
+ echo expected one argument: '<'dtrace-path'>'
+ exit 2
+fi
+
+dtrace=$1
+
+exec $dtrace $dt_flags -l -P 'prov*' -c `pwd`/test/triggers/usdt-tst-multiprovider
base-commit: 68d14f30b5ffaaea405354a2edf44279127d209a
--
2.46.0.278.g36e3a12567
More information about the DTrace-devel
mailing list