[DTrace-devel] [PATCH 5/5] Add -xcpu support for general providers

Kris Van Hees kris.van.hees at oracle.com
Wed Dec 20 17:09:22 UTC 2023


On Tue, Sep 05, 2023 at 12:11:42AM -0400, eugene.loh at oracle.com wrote:
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>

I do not understand this patch, or rather why you rename

	dt_cg_tramp_prologue_act

to be

	dt_cg_tramp_prologue_sub

yet the implementation does not change?

In all, I think it might be better to add a 2nd argument to
dt_cg_tramp_prologue() to indicate whether CPU filtering should be done (and
those who do it by means of how probes are attched can disable that).  Maybe
an argument named 'no_cpu' to be 1 to skip CPU filtering if -xcpu is set.

Or alterntively, add a new dt_cg_tramp_prologue_cpu(dt_pcb_t *pcb) that simply
does dt_cg_tramp_prologue_act(pcb, DT_ACTIVITY_ACTIVE); and then change the
dt_cg_tramp_prologue() function to generate the CPU filtering if -xcpu is set,
and then call dt_cg_tramp_prologue_cpu().  That was, as much as of this as can
be done remains hidden from the provider code.

> ---
>  CODING-STYLE                             |   2 +-
>  libdtrace/dt_cg.c                        |  25 +++-
>  libdtrace/dt_cg.h                        |   2 +-
>  libdtrace/dt_prov_cpc.c                  |   2 +-
>  libdtrace/dt_prov_dtrace.c               |   4 +-
>  libdtrace/dt_prov_profile.c              |   2 +-
>  test/unittest/options/tst.cpu-syscall.sh | 151 +++++++++++++++++++++++
>  7 files changed, 178 insertions(+), 10 deletions(-)
>  create mode 100755 test/unittest/options/tst.cpu-syscall.sh
> 
> diff --git a/CODING-STYLE b/CODING-STYLE
> index c8013f05..f6382c65 100644
> --- a/CODING-STYLE
> +++ b/CODING-STYLE
> @@ -26,7 +26,7 @@ E.g.,
>  	uint_t
>  	dt_cg_tramp_prologue(dt_pcb_t *pcb)
>  	{
> -		return dt_cg_tramp_prologue_act(pcb, DT_ACTIVITY_ACTIVE);
> +		return dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>  	}
>  
>  In an "if...else if...else" statement, omit braces for the final branch if
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 0ed662f9..a79b8352 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -49,7 +49,7 @@ static void dt_cg_node(dt_node_t *, dt_irlist_t *, dt_regset_t *);
>   *	%r9 contains a pointer to dctx
>   */
>  void
> -dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
> +dt_cg_tramp_prologue_sub(dt_pcb_t *pcb, dt_activity_t act)
>  {
>  	dtrace_hdl_t	*dtp = pcb->pcb_hdl;
>  	dt_irlist_t	*dlp = &pcb->pcb_ir;
> @@ -274,7 +274,24 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
>  void
>  dt_cg_tramp_prologue(dt_pcb_t *pcb)
>  {
> -	dt_cg_tramp_prologue_act(pcb, DT_ACTIVITY_ACTIVE);
> +	dtrace_hdl_t	*dtp = pcb->pcb_hdl;
> +
> +	/* Check if we are on the specified CPU (if any). */
> +	if (dtp->dt_options[DTRACEOPT_CPU] != DTRACEOPT_UNSET) {
> +		dt_irlist_t	*dlp = &pcb->pcb_ir;
> +
> +		emit(dlp,  BPF_MOV_REG(BPF_REG_8, BPF_REG_1));
> +
> +		emit(dlp,  BPF_CALL_HELPER(BPF_FUNC_get_smp_processor_id));
> +		emit(dlp,  BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0,
> +					  dtp->dt_options[DTRACEOPT_CPU],
> +					  pcb->pcb_exitlbl));
> +
> +		emit(dlp,  BPF_MOV_REG(BPF_REG_1, BPF_REG_8));
> +	}
> +
> +	/* Call the rest of the prologue code generation. */
> +	dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>  }
>  
>  /*
> @@ -421,8 +438,8 @@ dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int called)
>   * So put the PC in both arg0 and arg1, test the PC, and then zero out
>   * either arg0 or arg1, as apropriate.
>   *
> - * The caller must ensure that %r7 and %r8 contain the values set by
> - * the dt_cg_tramp_prologue*() functions.
> + * The caller must ensure that %r7 and %r8 contain the values set by the
> + * dt_cg_tramp_prologue*() functions.
>   */
>  void
>  dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb)
> diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
> index 0ced6dd2..1a40b2de 100644
> --- a/libdtrace/dt_cg.h
> +++ b/libdtrace/dt_cg.h
> @@ -20,7 +20,7 @@ extern "C" {
>  extern void dt_cg(dt_pcb_t *, dt_node_t *);
>  extern void dt_cg_xsetx(dt_irlist_t *, dt_ident_t *, uint_t, int, uint64_t);
>  extern dt_irnode_t *dt_cg_node_alloc(uint_t, struct bpf_insn);
> -extern void dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act);
> +extern void dt_cg_tramp_prologue_sub(dt_pcb_t *pcb, dt_activity_t act);
>  extern void dt_cg_tramp_prologue(dt_pcb_t *pcb);
>  extern void dt_cg_tramp_clear_regs(dt_pcb_t *pcb);
>  extern void dt_cg_tramp_copy_regs(dt_pcb_t *pcb);
> diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
> index 9e0f5542..706d729a 100644
> --- a/libdtrace/dt_prov_cpc.c
> +++ b/libdtrace/dt_prov_cpc.c
> @@ -395,7 +395,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  	int		i;
>  	dt_irlist_t	*dlp = &pcb->pcb_ir;
>  
> -	dt_cg_tramp_prologue(pcb);
> +	dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>  
>  	/*
>  	 * After the dt_cg_tramp_prologue() call, we have:
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index a76534f8..277edae2 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -119,10 +119,10 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  		key = DT_STATE_ENDEDON;
>  	}
>  
> -	dt_cg_tramp_prologue_act(pcb, act);
> +	dt_cg_tramp_prologue_sub(pcb, act);
>  
>  	/*
> -	 * After the dt_cg_tramp_prologue_act() call, we have:
> +	 * After the dt_cg_tramp_prologue_sub() call, we have:
>  	 *				//     (%r7 = dctx->mst)
>  	 *				//     (%r8 = dctx->ctx)
>  	 */
> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
> index f3f3bf23..08216b08 100644
> --- a/libdtrace/dt_prov_profile.c
> +++ b/libdtrace/dt_prov_profile.c
> @@ -220,7 +220,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  	int		i;
>  	dt_irlist_t	*dlp = &pcb->pcb_ir;
>  
> -	dt_cg_tramp_prologue(pcb);
> +	dt_cg_tramp_prologue_sub(pcb, DT_ACTIVITY_ACTIVE);
>  
>  	/*
>  	 * After the dt_cg_tramp_prologue() call, we have:
> diff --git a/test/unittest/options/tst.cpu-syscall.sh b/test/unittest/options/tst.cpu-syscall.sh
> new file mode 100755
> index 00000000..8713e9d4
> --- /dev/null
> +++ b/test/unittest/options/tst.cpu-syscall.sh
> @@ -0,0 +1,151 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2023, 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.
> +#
> +
> +dtrace=$1
> +
> +DIRNAME="$tmpdir/cpu-syscall.$$.$RANDOM"
> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +#
> +# Make trigger code and compile it.
> +#
> +
> +cat << EOF > main.c
> +#define _GNU_SOURCE
> +#include <sched.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include <errno.h>
> +
> +int ncpus;
> +cpu_set_t *mask;
> +size_t size;
> +
> +/*
> + * Grow the CPU mask as needed.
> + *
> + * Other ways of determining the number of CPUs available on the system:
> + * - inspecting the contents of /proc/cpuinfo
> + * - using sysconf(3) to obtain _SC_NPROCESSORS_CONF and _SC_NPROCESSORS_ONLN
> + * - inspecting the list of CPU directories under /sys/devices/system/cpu/
> + */
> +int grow_mask() {
> +	ncpus = 1024;
> +	while ((mask = CPU_ALLOC(ncpus)) != NULL &&
> +	       sched_getaffinity(0, CPU_ALLOC_SIZE(ncpus), mask) != 0 &&
> +	       errno == EINVAL) {
> +		CPU_FREE(mask);
> +		errno = 0;
> +		ncpus *= 2;
> +	}
> +	if (mask == NULL || (errno != 0 && errno != EINVAL))
> +		return 1;
> +	size = CPU_ALLOC_SIZE(ncpus);
> +	return 0;
> +}
> +
> +int bind_to_cpu(size_t cpu) {
> +	CPU_ZERO_S(size, mask);
> +	CPU_SET_S(cpu, size, mask);
> +	if (sched_setaffinity(0, size, mask) != 0)
> +		return 1;
> +	return 0;
> +}
> +
> +int main(int c, char **v) {
> +	int i, fd = open("/dev/null", O_WRONLY);
> +
> +	if (fd == -1)
> +		return 1;
> +
> +	if (grow_mask() == -1)
> +		return 1;
> +
> +	/* Loop over CPUs (in argv[]). */
> +        for (i = 1; i < c; i++) {
> +		/* Bind to CPU. */
> +		if (bind_to_cpu(atol(v[i])) == -1)
> +			return 1;
> +
> +		/* Call write() from CPU. */
> +		write(fd, &i, sizeof(i));
> +	}
> +
> +	close(fd);
> +	CPU_FREE(mask);
> +
> +	return 0;
> +}
> +EOF
> +
> +gcc main.c
> +if [ $? -ne 0 ]; then
> +	echo ERROR compilation failed
> +	exit 1
> +fi
> +
> +#
> +# Get CPU list and form expected-results file.
> +#
> +
> +cpulist=`awk '/^processor[ 	]: [0-9]*$/ { print $3 }' /proc/cpuinfo`
> +echo $cpulist
> +
> +echo > expect.txt
> +for cpu in $cpulist; do
> +	echo $cpu >> expect.txt
> +done
> +
> +#
> +# Run DTrace (without -xcpu).  Check that all CPUs appear.
> +#
> +
> +$dtrace $dt_flags \
> +    -qn 'syscall::write:entry { printf("%d\n", cpu); }' \
> +    -c "./a.out $cpulist" | sort | uniq > actual.txt
> +if [ $? -ne 0 ]; then
> +	echo ERROR dtrace failed
> +	exit 1
> +fi
> +
> +if ! diff -q expect.txt actual.txt > /dev/null; then
> +	echo ERROR did not see expected CPUs in baseline case
> +	echo expect $cpulist
> +	echo actual:
> +	cat actual.txt
> +	exit 1
> +fi
> +
> +#
> +# Run DTrace (with -xcpu).  Check that only the specified CPU appears.
> +#
> +
> +nerr=0
> +for cpu0 in $cpulist; do
> +	$dtrace $dt_flags -xcpu=$cpu0 \
> +	    -qn 'syscall::write:entry { printf("%d\n", cpu); }' \
> +	    -c "./a.out $cpulist" | sort | uniq > actual.txt
> +	echo > expect.txt
> +	echo $cpu0 >> expect.txt
> +
> +	if ! diff -q expect.txt actual.txt > /dev/null; then
> +		echo ERROR did not see expected CPUs in cpu $cpu0 case, saw:
> +		cat actual.txt
> +		nerr=$(($nerr + 1))
> +	fi
> +done
> +
> +exit $nerr
> -- 
> 2.18.4
> 
> 



More information about the DTrace-devel mailing list