[DTrace-devel] [PATCH v2 07/08] Move BPF stack, dctx, and mstate definitions out of dt_impl.h
Eugene Loh
eugene.loh at oracle.com
Thu Sep 17 21:47:38 PDT 2020
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
On 09/14/2020 12:51 PM, Kris Van Hees wrote:
> The BPF stack layout defines do not belong in dt_impl.h and therefore
> are moved to dt_dctx.h. The dt_impl.h does not need to include
> dt_dctx.h either because many files do not need it.
>
> This change has a lot of cascading effects across various source code
> files due to the include file content reschuffling, so a lot of small
> tweaks are included in this patch.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_bpf.c | 1 +
> libdtrace/dt_bpf.h | 9 ++--
> libdtrace/dt_cc.c | 1 +
> libdtrace/dt_cg.c | 1 +
> libdtrace/dt_dctx.h | 82 ++++++++++++++++++++++++++++++++
> libdtrace/dt_dis.c | 1 +
> libdtrace/dt_impl.h | 83 ---------------------------------
> libdtrace/dt_prov_dtrace.c | 1 +
> libdtrace/dt_prov_fbt.c | 1 +
> libdtrace/dt_prov_profile.c | 1 +
> libdtrace/dt_prov_sdt.c | 1 +
> libdtrace/dt_prov_syscall.c | 1 +
> libdtrace/dt_pt_regs.h | 3 ++
> libdtrace/dt_state.h | 22 ++++-----
> test/utils/print-stack-layout.c | 3 +-
> 15 files changed, 113 insertions(+), 98 deletions(-)
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index a381a86e..742b9b9b 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -13,6 +13,7 @@
> #include <sys/syscall.h>
> #include <dtrace.h>
> #include <dt_impl.h>
> +#include <dt_dctx.h>
> #include <dt_probe.h>
> #include <dt_state.h>
> #include <dt_bpf.h>
> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> index e87d4653..171b2866 100644
> --- a/libdtrace/dt_bpf.h
> +++ b/libdtrace/dt_bpf.h
> @@ -8,8 +8,11 @@
> #ifndef _DT_BPF_H
> #define _DT_BPF_H
>
> +#include <sys/dtrace_types.h>
> +#include <linux/bpf.h>
> #include <linux/perf_event.h>
> -#include <dt_impl.h>
> +
> +struct dtrace_hdl;
>
> #ifdef __cplusplus
> extern "C" {
> @@ -22,10 +25,10 @@ extern int perf_event_open(struct perf_event_attr *attr, pid_t pid, int cpu,
> int group_fd, unsigned long flags);
> extern int bpf(enum bpf_cmd cmd, union bpf_attr *attr);
>
> -extern int dt_bpf_gmap_create(dtrace_hdl_t *);
> +extern int dt_bpf_gmap_create(struct dtrace_hdl *);
> extern int dt_bpf_map_lookup(int fd, const void *key, void *val);
> extern int dt_bpf_map_update(int fd, const void *key, const void *val);
> -extern int dt_bpf_load_progs(dtrace_hdl_t *, uint_t);
> +extern int dt_bpf_load_progs(struct dtrace_hdl *, uint_t);
>
> #ifdef __cplusplus
> }
> diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> index d0c47bc0..34245649 100644
> --- a/libdtrace/dt_cc.c
> +++ b/libdtrace/dt_cc.c
> @@ -89,6 +89,7 @@
> #include <dt_string.h>
> #include <dt_impl.h>
> #include <dt_cg.h>
> +#include <dt_dctx.h>
> #include <dt_bpf.h>
> #include <bpf_asm.h>
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 16d5abff..63b14f06 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -13,6 +13,7 @@
> #include <errno.h>
>
> #include <dt_impl.h>
> +#include <dt_dctx.h>
> #include <dt_cg.h>
> #include <dt_grammar.h>
> #include <dt_parser.h>
> diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
> index b315f522..80c646a4 100644
> --- a/libdtrace/dt_dctx.h
> +++ b/libdtrace/dt_dctx.h
> @@ -8,6 +8,9 @@
> #ifndef _DT_DCTX_H
> #define _DT_DCTX_H
>
> +#include <stdint.h>
> +#include <linux/bpf.h>
> +#include <bpf_asm.h>
> #include <dt_pt_regs.h>
>
> /*
> @@ -51,4 +54,83 @@ typedef struct dt_dctx {
> #define DMST_REGS offsetof(dt_mstate_t, regs)
> #define DMST_ARG(n) offsetof(dt_mstate_t, argv[n])
>
> +/*
> + * DTrace BPF programs can use all BPF registers except for the the %fp (frame
> + * pointer) register and the highest numbered register (currently %r9) that is
> + * used to store the base pointer for the trace output record.
> + */
> +#define DT_STK_NREGS (MAX_BPF_REG - 2)
> +
> +/*
> + * An area of 256 bytes is set aside on the stack as scratch area for things
> + * like string operations. It extends from the end of the stack towards the
> + * local variable storage area. DT_STK_LVAR_END is therefore the last byte
> + * location on the stack that can be used for local variable storage.
> + */
> +#define DT_STK_SCRATCH_BASE (-MAX_BPF_STACK)
> +#define DT_STK_SCRATCH_SZ (256)
> +#define DT_STK_LVAR_END (DT_STK_SCRATCH_BASE + DT_STK_SCRATCH_SZ)
> +
> +/*
> + * The stack layout for functions that implement a D clause is encoded with the
> + * following constants.
> + *
> + * Note: The BPF frame pointer points to the address of the first byte past the
> + * end of the stack. If the stack size is 512 bytes, valid offsets are
> + * -1 through -512 (inclusive), So, the first 64-bit value on the stack
> + * occupies bytes at offsets -8 through -1. The second -16 through -9,
> + * and so on... 64-bit values are properly aligned at offsets -n where
> + * n is a multiple of 8 (sizeof(uint64_t)).
> + *
> + * The following diagram shows the stack layout for a size of 512 bytes.
> + *
> + * +----------------+
> + * SCRATCH_BASE = -512 | Scratch Memory |
> + * +----------------+
> + * LVAR_END = LVAR(n) = -256 | LVAR n | (n = DT_VAR_LOCAL_MAX = 19)
> + * +----------------+
> + * | ... |
> + * +----------------+
> + * LVAR(1) = -112 | LVAR 1 |
> + * +----------------+
> + * LVAR_BASE = LVAR(0) = -104 | LVAR 0 |
> + * +----------------+
> + * SPILL(n) = -96 | %r8 | (n = DT_STK_NREGS - 1 = 8)
> + * +----------------+
> + * | ... |
> + * +----------------+
> + * SPILL(1) = -40 | %r1 |
> + * +----------------+
> + * SPILL_BASE = SPILL(0) = -32 | %r0 |
> + * +----------------+
> + * DCTX = -24 | DTrace Context | -1
> + * +----------------+
> + */
> +#define DT_STK_BASE ((int16_t)0)
> +#define DT_STK_SLOT_SZ ((int16_t)sizeof(uint64_t))
> +
> +#define DT_STK_DCTX (DT_STK_BASE - DCTX_SIZE)
> +#define DT_STK_SPILL_BASE (DT_STK_DCTX - DT_STK_SLOT_SZ)
> +#define DT_STK_SPILL(n) (DT_STK_SPILL_BASE - (n) * DT_STK_SLOT_SZ)
> +#define DT_STK_LVAR_BASE (DT_STK_SPILL(DT_STK_NREGS - 1) - \
> + DT_STK_SLOT_SZ)
> +#define DT_STK_LVAR(n) (DT_STK_LVAR_BASE - (n) * DT_STK_SLOT_SZ)
> +
> +/*
> + * Calculate a local variable ID based on a given stack offset. If the stack
> + * offset is outside the valid range, this should evaluate as -1.
> + */
> +#define DT_LVAR_OFF2ID(n) (((n) > DT_STK_LVAR_BASE || \
> + (n) < DT_STK_LVAR_END) ? -1 : \
> + (- ((n) - DT_STK_LVAR_BASE) / DT_STK_SLOT_SZ))
> +
> +/*
> + * Maximum number of local variables stored by value (scalars). This is bound
> + * by the choice to store them on the stack between the register spill space,
> + * and 256 bytes set aside as string scratch space. We also use the fact that
> + * the (current) maximum stack space for BPF programs is 512 bytes.
> + */
> +#define DT_LVAR_MAX (- (DT_STK_LVAR_END - DT_STK_LVAR_BASE) / \
> + DT_STK_SLOT_SZ)
> +
> #endif /* _DT_DCTX_H */
> diff --git a/libdtrace/dt_dis.c b/libdtrace/dt_dis.c
> index a038bdcc..342db62b 100644
> --- a/libdtrace/dt_dis.c
> +++ b/libdtrace/dt_dis.c
> @@ -9,6 +9,7 @@
> #include <stdio.h>
>
> #include <dt_impl.h>
> +#include <dt_dctx.h>
> #include <dt_ident.h>
> #include <dt_printf.h>
> #include <dt_string.h>
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index f1a2513f..dea16680 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -14,9 +14,6 @@
> #include <dtrace.h>
> #include <pthread.h>
>
> -#include <sys/ptrace.h>
> -#include <asm/ptrace.h>
> -
> #include <sys/types.h>
> #include <sys/dtrace_types.h>
> #include <sys/utsname.h>
> @@ -49,7 +46,6 @@ extern "C" {
> #include <dt_pcb.h>
> #include <dt_pt_regs.h>
> #include <dt_printf.h>
> -#include <dt_dctx.h>
> #include <dt_debug.h>
> #include <dt_version.h>
>
> @@ -462,85 +458,6 @@ struct dtrace_hdl {
> #define DT_USYMADDR_CTFP(dtp) ((dtp)->dt_ddefs->dm_ctfp)
> #define DT_USYMADDR_TYPE(dtp) ((dtp)->dt_type_usymaddr)
>
> -/*
> - * DTrace BPF programs can use all BPF registers except for the the %fp (frame
> - * pointer) register and the highest numbered register (currently %r9) that is
> - * used to store the base pointer for the trace output record.
> - */
> -#define DT_STK_NREGS (MAX_BPF_REG - 2)
> -
> -/*
> - * An area of 256 bytes is set aside on the stack as scratch area for things
> - * like string operations. It extends from the end of the stack towards the
> - * local variable storage area. DT_STK_LVAR_END is therefore the last byte
> - * location on the stack that can be used for local variable storage.
> - */
> -#define DT_STK_SCRATCH_BASE (-MAX_BPF_STACK)
> -#define DT_STK_SCRATCH_SZ (256)
> -#define DT_STK_LVAR_END (DT_STK_SCRATCH_BASE + DT_STK_SCRATCH_SZ)
> -
> -/*
> - * The stack layout for functions that implement a D clause is encoded with the
> - * following constants.
> - *
> - * Note: The BPF frame pointer points to the address of the first byte past the
> - * end of the stack. If the stack size is 512 bytes, valid offsets are
> - * -1 through -512 (inclusive), So, the first 64-bit value on the stack
> - * occupies bytes at offsets -8 through -1. The second -16 through -9,
> - * and so on... 64-bit values are properly aligned at offsets -n where
> - * n is a multiple of 8 (sizeof(uint64_t)).
> - *
> - * The following diagram shows the stack layout for a size of 512 bytes.
> - *
> - * +----------------+
> - * SCRATCH_BASE = -512 | Scratch Memory |
> - * +----------------+
> - * LVAR_END = LVAR(n) = -256 | LVAR n | (n = DT_VAR_LOCAL_MAX = 19)
> - * +----------------+
> - * | ... |
> - * +----------------+
> - * LVAR(1) = -112 | LVAR 1 |
> - * +----------------+
> - * LVAR_BASE = LVAR(0) = -104 | LVAR 0 |
> - * +----------------+
> - * SPILL(n) = -96 | %r8 | (n = DT_STK_NREGS - 1 = 8)
> - * +----------------+
> - * | ... |
> - * +----------------+
> - * SPILL(1) = -40 | %r1 |
> - * +----------------+
> - * SPILL_BASE = SPILL(0) = -32 | %r0 |
> - * +----------------+
> - * DCTX = -24 | DTrace Context | -1
> - * +----------------+
> - */
> -#define DT_STK_BASE ((int16_t)0)
> -#define DT_STK_SLOT_SZ ((int16_t)sizeof(uint64_t))
> -
> -#define DT_STK_DCTX (DT_STK_BASE - DCTX_SIZE)
> -#define DT_STK_SPILL_BASE (DT_STK_DCTX - DT_STK_SLOT_SZ)
> -#define DT_STK_SPILL(n) (DT_STK_SPILL_BASE - (n) * DT_STK_SLOT_SZ)
> -#define DT_STK_LVAR_BASE (DT_STK_SPILL(DT_STK_NREGS - 1) - \
> - DT_STK_SLOT_SZ)
> -#define DT_STK_LVAR(n) (DT_STK_LVAR_BASE - (n) * DT_STK_SLOT_SZ)
> -
> -/*
> - * Calculate a local variable ID based on a given stack offset. If the stack
> - * offset is outside the valid range, this should evaluate as -1.
> - */
> -#define DT_LVAR_OFF2ID(n) (((n) > DT_STK_LVAR_BASE || \
> - (n) < DT_STK_LVAR_END) ? -1 : \
> - (- ((n) - DT_STK_LVAR_BASE) / DT_STK_SLOT_SZ))
> -
> -/*
> - * Maximum number of local variables stored by value (scalars). This is bound
> - * by the choice to store them on the stack between the register spill space,
> - * and 256 bytes set aside as string scratch space. We also use the fact that
> - * the (current) maximum stack space for BPF programs is 512 bytes.
> - */
> -#define DT_LVAR_MAX (- (DT_STK_LVAR_END - DT_STK_LVAR_BASE) / \
> - DT_STK_SLOT_SZ)
> -
> /*
> * Actions and subroutines are both DT_NODE_FUNC nodes; to avoid confusing
> * an action for a subroutine (or vice versa), we assure that the DT_ACT_*
> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
> index cf811d24..4fde9942 100644
> --- a/libdtrace/dt_prov_dtrace.c
> +++ b/libdtrace/dt_prov_dtrace.c
> @@ -12,6 +12,7 @@
>
> #include <bpf_asm.h>
>
> +#include "dt_dctx.h"
> #include "dt_cg.h"
> #include "dt_provider.h"
> #include "dt_probe.h"
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index ac77a5fe..842ba53b 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -31,6 +31,7 @@
>
> #include <bpf_asm.h>
>
> +#include "dt_dctx.h"
> #include "dt_cg.h"
> #include "dt_bpf_builtins.h"
> #include "dt_provider.h"
> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
> index 2499d429..0e90351f 100644
> --- a/libdtrace/dt_prov_profile.c
> +++ b/libdtrace/dt_prov_profile.c
> @@ -11,6 +11,7 @@
>
> #include <bpf_asm.h>
>
> +#include "dt_dctx.h"
> #include "dt_cg.h"
> #include "dt_bpf.h"
> #include "dt_probe.h"
> diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
> index a0cf5f7b..fbfdb7fe 100644
> --- a/libdtrace/dt_prov_sdt.c
> +++ b/libdtrace/dt_prov_sdt.c
> @@ -27,6 +27,7 @@
>
> #include <bpf_asm.h>
>
> +#include "dt_dctx.h"
> #include "dt_cg.h"
> #include "dt_bpf.h"
> #include "dt_bpf_builtins.h"
> diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
> index 8d7043dc..b817f528 100644
> --- a/libdtrace/dt_prov_syscall.c
> +++ b/libdtrace/dt_prov_syscall.c
> @@ -29,6 +29,7 @@
>
> #include <bpf_asm.h>
>
> +#include "dt_dctx.h"
> #include "dt_cg.h"
> #include "dt_bpf_builtins.h"
> #include "dt_provider.h"
> diff --git a/libdtrace/dt_pt_regs.h b/libdtrace/dt_pt_regs.h
> index 52b04822..72e065c1 100644
> --- a/libdtrace/dt_pt_regs.h
> +++ b/libdtrace/dt_pt_regs.h
> @@ -8,6 +8,9 @@
> #ifndef _DT_PR_REGS_H
> #define _DT_PR_REGS_H
>
> +#ifndef __BPF__
> +# include <sys/ptrace.h>
> +#endif
> #include <asm/ptrace.h>
>
> #ifdef __cplusplus
> diff --git a/libdtrace/dt_state.h b/libdtrace/dt_state.h
> index dff0aabf..c6500dca 100644
> --- a/libdtrace/dt_state.h
> +++ b/libdtrace/dt_state.h
> @@ -8,11 +8,6 @@
> #ifndef _DT_STATE_H
> #define _DT_STATE_H
>
> -#include "dt_bpf.h"
> -#include "dt_impl.h"
> -
> -struct dtrace_hdl;
> -
> /*
> * DTrace 'state' BPF map.
> *
> @@ -39,6 +34,10 @@ typedef enum dt_activity {
> DT_ACTIVITY_STOPPED /* tracing stopped */
> } dt_activity_t;
>
> +#ifndef __BPF__
> +# include "dt_bpf.h"
> +# include "dt_impl.h"
> +
> static inline uint32_t
> dt_state_get(dtrace_hdl_t *dtp, uint32_t key)
> {
> @@ -55,12 +54,13 @@ dt_state_set(dtrace_hdl_t *dtp, uint32_t key, uint32_t val)
> dt_bpf_map_update(dtp->dt_stmap_fd, &key, &val);
> }
>
> -#define dt_state_get_activity(dtp) ((dt_activity_t) \
> - dt_state_get(dtp, DT_STATE_ACTIVITY))
> -#define dt_state_get_beganon(dtp) dt_state_get(dtp, DT_STATE_BEGANON)
> -#define dt_state_get_endedon(dtp) dt_state_get(dtp, DT_STATE_ENDEDON)
> +# define dt_state_get_activity(dtp) \
> + ((dt_activity_t)dt_state_get(dtp, DT_STATE_ACTIVITY))
> +# define dt_state_set_activity(dtp, act) \
> + dt_state_set(dtp, DT_STATE_ACTIVITY, (uint32_t)(act))
>
> -#define dt_state_set_activity(dtp, act) dt_state_set(dtp, DT_STATE_ACTIVITY, \
> - (uint32_t)(act))
> +# define dt_state_get_beganon(dtp) dt_state_get(dtp, DT_STATE_BEGANON)
> +# define dt_state_get_endedon(dtp) dt_state_get(dtp, DT_STATE_ENDEDON)
> +#endif
>
> #endif /* _DT_STATE_H */
> diff --git a/test/utils/print-stack-layout.c b/test/utils/print-stack-layout.c
> index f19d25a7..bb5da3af 100644
> --- a/test/utils/print-stack-layout.c
> +++ b/test/utils/print-stack-layout.c
> @@ -6,7 +6,8 @@
> */
>
> #include <stdio.h>
> -#include <dt_impl.h>
> +#include <stdlib.h>
> +#include <dt_dctx.h>
>
> int main(void)
> {
More information about the DTrace-devel
mailing list