[DTrace-devel] [PATCH v2 10/19] Remove the is-enabled provider
eugene.loh at oracle.com
eugene.loh at oracle.com
Sat Oct 26 01:16:19 UTC 2024
From: Eugene Loh <eugene.loh at oracle.com>
The trampoline for the is-enabled provider is unnecessarily complicated.
We do not need dt_cg_tramp_copy_regs() since the copied values will not
be used. Nor do we need the (second) copy of regs[arg0] to mst->arg[0].
We can inline copyout_val().
Actually, we can simply consolidate the USDT and is-enabled providers.
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
libdtrace/dt_prov_uprobe.c | 165 ++++-------------------
test/unittest/usdt/tst.enable_pid.r | 45 +++++++
test/unittest/usdt/tst.enable_pid.r.p | 7 +
test/unittest/usdt/tst.enable_pid.sh | 183 ++++++++++++++++++++++++++
4 files changed, 261 insertions(+), 139 deletions(-)
create mode 100644 test/unittest/usdt/tst.enable_pid.r
create mode 100755 test/unittest/usdt/tst.enable_pid.r.p
create mode 100755 test/unittest/usdt/tst.enable_pid.sh
diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index 54e115f51..dcec76956 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -43,7 +43,6 @@
/* Provider name for the underlying probes. */
static const char prvname[] = "uprobe";
-static const char prvname_is_enabled[] = "uprobe__is_enabled";
#define PP_IS_RETURN 1
#define PP_IS_FUNCALL 2
@@ -77,7 +76,6 @@ static const dtrace_pattr_t pattr = {
{ DTRACE_STABILITY_PRIVATE, DTRACE_STABILITY_PRIVATE, DTRACE_CLASS_UNKNOWN },
};
-dt_provimpl_t dt_uprobe_is_enabled;
dt_provimpl_t dt_pid;
dt_provimpl_t dt_usdt;
@@ -85,8 +83,6 @@ static int populate(dtrace_hdl_t *dtp)
{
if (dt_provider_create(dtp, dt_uprobe.name, &dt_uprobe, &pattr,
NULL) == NULL ||
- dt_provider_create(dtp, dt_uprobe_is_enabled.name,
- &dt_uprobe_is_enabled, &pattr, NULL) == NULL ||
dt_provider_create(dtp, dt_pid.name, &dt_pid, &pattr,
NULL) == NULL)
return -1; /* errno already set */
@@ -464,7 +460,6 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
dtrace_probedesc_t pd;
dt_probe_t *uprp;
dt_uprobe_t *upp;
- int is_enabled = 0;
/*
* The underlying probes (uprobes) represent the tracepoints that pid
@@ -487,8 +482,6 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
strcpy(prb, "return");
break;
case DTPPT_IS_ENABLED:
- is_enabled = 1;
- /* Fallthrough. */
case DTPPT_ENTRY:
case DTPPT_OFFSETS:
snprintf(prb, sizeof(prb), "%lx", psp->pps_off);
@@ -499,7 +492,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
}
pd.id = DTRACE_IDNONE;
- pd.prv = is_enabled ? prvname_is_enabled : prvname;
+ pd.prv = prvname;
pd.mod = mod;
pd.fun = psp->pps_fun;
pd.prb = prb;
@@ -691,21 +684,6 @@ static void enable(dtrace_hdl_t *dtp, dt_probe_t *prp, int is_usdt)
dt_probe_enable(dtp, uprp);
}
- /*
- * If necessary, we need to enable is-enabled probes too (if they
- * exist).
- */
- if (is_usdt) {
- dtrace_probedesc_t pd;
- dt_probe_t *iep;
-
- memcpy(&pd, &prp->desc, sizeof(pd));
- pd.prv = prvname_is_enabled;
- iep = dt_probe_lookup(dtp, &pd);
- if (iep != NULL)
- dt_probe_enable(dtp, iep);
- }
-
/*
* Finally, ensure we're in the list of enablings as well.
* (This ensures that, among other things, the probes map
@@ -850,6 +828,29 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_lookup_elem));
emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_exit));
+ if (upp->flags & PP_IS_ENABLED) {
+ /*
+ * Generate a BPF trampoline for an is-enabled probe. The is-enabled probe
+ * prototype looks like:
+ *
+ * int is_enabled(int *arg)
+ *
+ * The trampoline writes 1 into the location pointed to by the passed-in arg.
+ */
+ emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0), 1));
+ emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_8, PT_REGS_ARG0));
+ emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_FP));
+ emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0)));
+ emit(dlp, BPF_MOV_IMM(BPF_REG_3, sizeof(uint32_t)));
+ emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_write_user));
+
+ goto out;
+ }
+
+ /*
+ * Continue with normal USDT probes.
+ */
+
/* Read the PRID from the table lookup and store to mst->prid. */
emit(dlp, BPF_LOAD(BPF_W, BPF_REG_1, BPF_REG_0, 0));
emit(dlp, BPF_STORE(BPF_W, BPF_REG_7, DMST_PRID, BPF_REG_1));
@@ -906,110 +907,11 @@ out:
return 0;
}
-/*
- * Copy the given immediate value into the address given by the specified probe
- * argument.
- */
-static void
-copyout_val(dt_pcb_t *pcb, uint_t lbl, uint32_t val, int arg)
-{
- dt_regset_t *drp = pcb->pcb_regs;
- dt_irlist_t *dlp = &pcb->pcb_ir;
-
- emitl(dlp, lbl, BPF_STORE_IMM(BPF_DW, BPF_REG_FP, DT_TRAMP_SP_SLOT(0),
- val));
-
- if (dt_regset_xalloc_args(drp) == -1)
- longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
- emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_7, DMST_ARG(arg)));
- emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_FP));
- emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0)));
- emit(dlp, BPF_MOV_IMM(BPF_REG_3, sizeof(uint32_t)));
- dt_regset_xalloc(drp, BPF_REG_0);
- emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_write_user));
-
- /* XXX any point error-checking here? What can we possibly do? */
- dt_regset_free(drp, BPF_REG_0);
- dt_regset_free_args(drp);
-}
-
-/*
- * Generate a BPF trampoline for an is-enabled probe. The is-enabled probe
- * prototype looks like:
- *
- * int is_enabled(int *arg)
- *
- * The trampoline dereferences the passed-in arg and writes 1 into it if this is
- * one of the processes for which the probe is enabled.
- */
-static int trampoline_is_enabled(dt_pcb_t *pcb, uint_t exitlbl)
-{
- dt_irlist_t *dlp = &pcb->pcb_ir;
- const dt_probe_t *uprp = pcb->pcb_probe;
- dt_ident_t *usdt_prids = dt_dlib_get_map(pcb->pcb_hdl, "usdt_prids");
-
- assert(usdt_prids != NULL);
-
- dt_cg_tramp_prologue(pcb);
-
- /*
- * After the dt_cg_tramp_prologue() call, we have:
- * // (%r7 = dctx->mst)
- * // (%r8 = dctx->ctx)
- */
- dt_cg_tramp_copy_regs(pcb);
-
- /*
- * Copy in the first function argument, a pointer value to which
- * the is-enabled state of the probe will be written (necessarily
- * 1 if this probe is running at all).
- */
- emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, PT_REGS_ARG0));
- emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(0), BPF_REG_0));
-
- /*
- * Retrieve the PID of the process that caused the probe to fire.
- */
- emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
- emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
-
- /*
- * Look up in the BPF 'usdt_prids' map. Space for the look-up key
- * will be used on the BPF stack:
- *
- * offset value
- *
- * -sizeof(usdt_prids_map_key_t) pid (in %r0)
- *
- * -sizeof(usdt_prids_map_key_t) + sizeof(pid_t)
- * ==
- * -sizeof(dtrace_id_t) underlying-probe prid
- */
- emit(dlp, BPF_STORE(BPF_W, BPF_REG_9, (int)(-sizeof(usdt_prids_map_key_t)), BPF_REG_0));
- emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_9, (int)(-sizeof(dtrace_id_t)), uprp->desc->id));
- dt_cg_xsetx(dlp, usdt_prids, DT_LBL_NONE, BPF_REG_1, usdt_prids->di_id);
- emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_9));
- emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, (int)(-sizeof(usdt_prids_map_key_t))));
- emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_lookup_elem));
- emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, pcb->pcb_exitlbl));
-
- /*
- * If we succeeded, then we use copyout_val() above to assign:
- * *arg0 = 1;
- */
- copyout_val(pcb, DT_LBL_NONE, 1, 0);
-
- dt_cg_tramp_return(pcb);
-
- return 0;
-}
-
static char *uprobe_name(dev_t dev, ino_t ino, uint64_t addr, int flags)
{
char *name;
- if (asprintf(&name, "dt_pid%s/%c_%llx_%llx_%lx",
- flags & PP_IS_ENABLED ? "_is_enabled" : "",
+ if (asprintf(&name, "dt_pid/%c_%llx_%llx_%lx",
flags & PP_IS_RETURN ? 'r' : 'p', (unsigned long long)dev,
(unsigned long long)ino, (unsigned long)addr) < 0)
return NULL;
@@ -1019,7 +921,7 @@ static char *uprobe_name(dev_t dev, ino_t ino, uint64_t addr, int flags)
/*
* Create a uprobe for a given dev/ino, mapping filename, and address: the
- * uprobe may be a uretprobe or an is-enabled probe. Return the probe's name as
+ * uprobe may be a uretprobe. Return the probe's name as
* a new dynamically-allocated string, or NULL on error.
*/
static char *uprobe_create(dev_t dev, ino_t ino, const char *mapping_fn,
@@ -1181,21 +1083,6 @@ dt_provimpl_t dt_uprobe = {
.add_probe = &add_probe_uprobe,
};
-/*
- * Used for underlying is-enabled uprobes.
- */
-dt_provimpl_t dt_uprobe_is_enabled = {
- .name = prvname_is_enabled,
- .prog_type = BPF_PROG_TYPE_KPROBE,
- .populate = &populate,
- .load_prog = &dt_bpf_prog_load,
- .trampoline = &trampoline_is_enabled,
- .attach = &attach,
- .probe_info = &probe_info,
- .detach = &detach,
- .probe_destroy = &probe_destroy_underlying,
-};
-
/*
* Used for pid probes.
*/
diff --git a/test/unittest/usdt/tst.enable_pid.r b/test/unittest/usdt/tst.enable_pid.r
new file mode 100644
index 000000000..675fcdd6f
--- /dev/null
+++ b/test/unittest/usdt/tst.enable_pid.r
@@ -0,0 +1,45 @@
+ FUNCTION:NAME
+ :tick-1s
+
+ FUNCTION:NAME
+ :tick-1s
+
+ FUNCTION:NAME
+ :tick-1s
+
+ FUNCTION:NAME
+ :tick-1s
+
+done
+========== out 1
+is not enabled
+=== epoch ===
+is not enabled
+is enabled
+is not enabled
+=== epoch ===
+is not enabled
+=== epoch ===
+is not enabled
+is enabled
+is not enabled
+=== epoch ===
+========== out 2
+is not enabled
+=== epoch ===
+is not enabled
+=== epoch ===
+is not enabled
+is enabled
+is not enabled
+=== epoch ===
+is not enabled
+is enabled
+is not enabled
+=== epoch ===
+success
+-- @@stderr --
+dtrace: description 'test_provNNN:::go ' matched 1 probe
+dtrace: description 'test_provNNN:::go ' matched 2 probes
+dtrace: description 'test_provNNN:::go ' matched 2 probes
+dtrace: description 'test_prov*:::go ' matched 3 probes
diff --git a/test/unittest/usdt/tst.enable_pid.r.p b/test/unittest/usdt/tst.enable_pid.r.p
new file mode 100755
index 000000000..baf9d2a90
--- /dev/null
+++ b/test/unittest/usdt/tst.enable_pid.r.p
@@ -0,0 +1,7 @@
+#!/usr/bin/awk -f
+{
+ # ignore the specific process ID
+ gsub(/test_prov[0-9]+/, "test_provNNN");
+
+ print;
+}
diff --git a/test/unittest/usdt/tst.enable_pid.sh b/test/unittest/usdt/tst.enable_pid.sh
new file mode 100755
index 000000000..91923014d
--- /dev/null
+++ b/test/unittest/usdt/tst.enable_pid.sh
@@ -0,0 +1,183 @@
+#!/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.
+#
+# @@timeout: 80
+
+PATH=/usr/bin:/usr/sbin:$PATH
+
+#
+# In this test, we check that is-enabled probes depend correctly on pid.
+#
+
+dtrace=$1
+CC=/usr/bin/gcc
+CFLAGS=
+
+DIRNAME="$tmpdir/usdt-enable_pid.$$.$RANDOM"
+mkdir -p $DIRNAME
+cd $DIRNAME
+
+#
+# Set up the source files.
+#
+
+cat > prov.d <<EOF
+provider test_prov {
+ probe go();
+};
+EOF
+
+cat > main.c <<EOF
+#include <signal.h>
+#include <stdio.h>
+#include "prov.h"
+
+/* We check if the is-enabled probe is or is not enabled (or unknown). */
+#define ENABLED_IS 1
+#define ENABLED_NOT 2
+#define ENABLED_UNK 3
+
+/* Start with the previous probe "unknown". */
+int prv = ENABLED_UNK;
+long long num = 0;
+
+/* Report how many times the previous case was encountered. */
+static void report() {
+
+ /* Skip if there is nothing to report. */
+ if (num == 0)
+ return;
+
+ switch (prv) {
+ case ENABLED_IS:
+ printf("is enabled\n");
+ break;
+ case ENABLED_NOT:
+ printf("is not enabled\n");
+ break;
+ }
+ fflush(stdout);
+
+ /* Reset. */
+ prv = ENABLED_UNK;
+ num = 0;
+}
+
+/* When USR1 is received, mark an "epoch" in the output. */
+static void mark_epoch(int sig) {
+ report();
+ printf("=== epoch ===\n");
+ fflush(stdout);
+}
+
+int
+main(int argc, char **argv)
+{
+ struct sigaction act;
+
+ /* Set USR1 to mark epochs. */
+ act.sa_flags = 0;
+ act.sa_handler = mark_epoch;
+ if (sigaction(SIGUSR1, &act, NULL)) {
+ printf("set handler failed\n");
+ return 1;
+ }
+
+ /* Just keep looping, counting consecutive cases. */
+ while (1) {
+ int now;
+
+ /* Check the is-enabled probe. */
+ if (TEST_PROV_GO_ENABLED()) {
+ now = ENABLED_IS;
+ } else {
+ now = ENABLED_NOT;
+ }
+
+ /* Compare to the previous case. */
+ if (now == prv) {
+ num++;
+ } else {
+ report(); /* resets num */
+ prv = now;
+ }
+ }
+
+ return 0;
+}
+EOF
+
+#
+# Build the test program.
+#
+
+$dtrace -h -s prov.d
+if [ $? -ne 0 ]; then
+ echo "failed to generate header file" >&2
+ exit 1
+fi
+cc $test_cppflags -c main.c
+if [ $? -ne 0 ]; then
+ echo "failed to compile test" >&2
+ exit 1
+fi
+$dtrace -G -64 -s prov.d main.o
+if [ $? -ne 0 ]; then
+ echo "failed to create DOF" >&2
+ exit 1
+fi
+cc $test_cppflags -o main main.o prov.o
+if [ $? -ne 0 ]; then
+ echo "failed to link final executable" >&2
+ exit 1
+fi
+
+#
+# Start two copies.
+#
+
+./main >& out.1 &
+pid1=$!
+./main >& out.2 &
+pid2=$!
+
+#
+# Run DTrace with different pid probes, each case is its own "epoch":
+# pid1? pid2?
+# - 1 no no
+# - $pid1 YES no
+# - $pid2 no YES
+# - * YES YES
+#
+
+for pid in 1 $pid1 $pid2 '*'; do
+ sleep 1
+ $dtrace $dt_flags -Zn 'test_prov'$pid':::go { trace("hi"); }
+ tick-1s { exit(0) }'
+ if [ $? -ne 0 ]; then
+ echo ERROR: dtrace
+ kill -TERM $pid1
+ kill -TERM $pid2
+ exit 1
+ fi
+ sleep 1
+
+ # Use USR1 to mark epochs in the output.
+ kill -USR1 $pid1
+ kill -USR1 $pid2
+done
+
+echo done
+echo "========== out 1"; cat out.1
+echo "========== out 2"; cat out.2
+
+echo success
+
+kill -TERM $pid1
+kill -TERM $pid2
+
+exit 0
--
2.43.5
More information about the DTrace-devel
mailing list