[DTrace-devel] [PATCH v3] dtrace provider: add a predicate against the current tgid

[Kris Van Hees]

On Thu, Feb 29, 2024 at 05:42:30PM +0000, 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.)
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

> ---
>  libdtrace/dt_prov_dtrace.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 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);
>  
>  	/*
> -- 
> 2.43.0.272.gce700b77fd
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel