[DTrace-devel] [PATCH v4] dtrace provider: add a predicate against the current tgid
Eugene Loh
eugene.loh at oracle.com
Fri Mar 1 16:55:13 UTC 2024
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
(and you already had R-b from Kris for previous version)
On 3/1/24 07:07, Nick Alcock via DTrace-devel wrote:
> 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 at oracle.com>
> ---
> libdtrace/dt_prov_dtrace.c | 19 ++++++++-
> test/unittest/dtrace-util/tst.DisOption.sh | 45 +++++++++++++++++++---
> 2 files changed, 57 insertions(+), 7 deletions(-)
>
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index 307ad558874f3..3e6ae88a37848 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -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).
> @@ -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).
> *
> @@ -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);
>
> /*
> diff --git a/test/unittest/dtrace-util/tst.DisOption.sh b/test/unittest/dtrace-util/tst.DisOption.sh
> index 1ad3f35f4fb8e..1f09a0869f664 100755
> --- a/test/unittest/dtrace-util/tst.DisOption.sh
> +++ b/test/unittest/dtrace-util/tst.DisOption.sh
> @@ -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"
> @@ -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");
> @@ -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
> @@ -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
More information about the DTrace-devel
mailing list