Skip to content

Commit

Permalink
dtrace provider: add a predicate against the current tgid
Browse files Browse the repository at this point in the history
If we don't put a predicate on, BEGIN/END probes fire in every running
dtrace at the same time, messing up the activity state of all but the one it
was meant to fire in and often causing the others to fail to exit on exit()
(they hang until ended by some other means, like an interrupt or -c
termination).

Thankfully the -xcpu run-BEGIN/END-in-a-thread complexities can be ignored
because we can match on DTrace's tgid instead of its PID (thread ID),
which will always catch exactly our BEGIN/END firings and no-one else's.

(In theory this might cause trouble if you run multiple consumers in
different threads in the same process, but that's not going to work as it
is, and has never been considered a sane thing to do.)

One test is perturbed by the new changing-every-invocation value this
adds to the BEGIN prologue, and is suitably adjusted.

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
  • Loading branch information
nickalcock authored and kvanhees committed Mar 6, 2024
1 parent ccd6059 commit ae8e471
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 7 deletions.
19 changes: 17 additions & 2 deletions libdtrace/dt_prov_dtrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
}

/*
* The BEGIN probe should only run when the activity state is INACTIVE.
* The BEGIN probe should only run when the activity state is INACTIVE,
* for this process's PID (TGID).
* At the end of the trampoline (after executing any clauses), the
* state must be advanced to the next state (INACTIVE -> ACTIVE, or if
* there was an exit() action in the clause, DRAINING -> STOPPED).
Expand All @@ -102,7 +103,8 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
* on in state[DT_STATE_BEGANON] to ensure that we know which trace
* data buffer to process first.
*
* The END probe should only run when the activity state is DRAINING.
* The END probe should only run when the activity state is DRAINING,
* for this process's PID (TGID).
* At the end of the trampoline (after executing any clauses), the
* state must be advanced to the next state (DRAINING -> STOPPED).
*
Expand All @@ -118,6 +120,19 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
key = DT_STATE_ENDEDON;
}

/*
* Retrieve the PID (TGID) of the process that caused the probe to fire,
* and check it against the PID we're meant to be using. Do it before
* the trampoline to minimize the cost of pointless firings in other
* tracers, even though this means preserving the context in %r1 around
* the call.
*/
emit(dlp, BPF_MOV_REG(BPF_REG_6, BPF_REG_1));
emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
emit(dlp, BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0, getpid(), pcb->pcb_exitlbl));
emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_6));

dt_cg_tramp_prologue_act(pcb, act);

/*
Expand Down
45 changes: 40 additions & 5 deletions test/unittest/dtrace-util/tst.DisOption.sh
Original file line number Diff line number Diff line change
@@ -1,13 +1,37 @@
#!/bin/bash
#
# Oracle Linux DTrace.
# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2022, 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.
#

# ASSERTION: That the dtrace -xdisasm option can control the listings.

# We create simple D scripts (as well as their expected output).
#
# We run with different settings of -xdisasm:
#
# # default
# -xdisasm=1 # listing mode 0
# -xdisasm=2 # listing mode 1
# -xdisasm=4 # listing mode 2
# -xdisasm=8 # listing mode 3
# -xdisasm=15 # all
#
# We sanity check stdout and stderr.
#
# We check that the default is equivalent to -xdisasm=1.
#
# We split the -xdisasm=15 output into different pieces, and check
# that the pieces correspond to the -xdisasm=1, 2, 4, and 8 output.
#
# One problem is that, since the checks are between different invocations
# of dtrace, some of the output may change -- specifically:
# * the BOOTTM value
# * the tgid value that the predicate checks
# So, we filter those run-dependent items out.

dtrace=$1

DIRNAME="$tmpdir/DisOption.$$.$RANDOM"
Expand Down Expand Up @@ -36,13 +60,14 @@ EOF
# run dtrace for various xdisasm settings

function run_dtrace() {
$dtrace $dt_flags $2 -Sq -s D1.d -s D2.d > $1.out 2> $1.tmp
$dtrace $dt_flags $2 -Sq -s D1.d -s D2.d > $1.out 2> $1.err
if [ $? -ne 0 ]; then
echo DTrace failed $2
cat $1.out $1.err
exit 1
fi
# Avoid differenced due to different BOOTTM values.

# Avoid differences due to different BOOTTM values.
awk '/: 18 [0-9] 0 / && /lddw/ {
sub(/0x[0-9a-f]+/, 0x0);
sub(/[0-9a-f]{8}/, "00000000");
Expand All @@ -58,7 +83,17 @@ function run_dtrace() {
print;
next;
}
{ print; }' $1.tmp > $1.err
{ print; }' $1.err > $1.tmp
mv $1.tmp $1.err

# Avoid differences due to different tgid values in predicates.
# If we see bpf_get_current_pid_tgid, omit the 3rd line if it's
# "jne %r0, ..." since the check value will change from run to run.
awk '/call bpf_get_current_pid_tgid/ { ncount = 0 }
{ ncount++ }
ncount == 3 && /^[ :0-9a-f]* jne *%r0, / { next }
{ print; }' $1.err > $1.tmp
mv $1.tmp $1.err
}

run_dtrace def # default
Expand All @@ -68,7 +103,7 @@ run_dtrace 2 -xdisasm=4 # listing mode 2
run_dtrace 3 -xdisasm=8 # listing mode 3
run_dtrace all -xdisasm=15 # all

# check individual D stdout and stderr files
# sanity check individual D stdout and stderr files

for x in def 0 1 2 3 all; do
if [ `cat $x.err | wc -l` -lt 10 ]; then
Expand Down

0 comments on commit ae8e471

Please sign in to comment.