[DTrace-devel] [PATCH 2/2] cc, bpf: separate final BPF program creation from linking and loading
Eugene Loh
eugene.loh at oracle.com
Thu Aug 24 20:02:26 UTC 2023
In dt_bpf_load_progs(), if you define and use "dp = prp->difo", the code
becomes a little more compact and also mimics the previous code better.
Here's a test that shows a difference due to this patch:
for x in 1 2 4 8; do
echo $x `sudo build/run-dtrace -xdisasm=$x -Sn 'BEGIN {
strstr("abcde", "xyz"); exit(0) }' |& wc -l`
done
I get:
x before after
1 111 111
2 279 16
4 1194 352
8 846 852
Maybe you have to add:
DT_DISASM_PROG (dtp, cflags, dp, stderr, NULL, prp->desc); in
dt_bpf_make_progs()
DT_DISASM_PROG_LINKED(dtp, cflags, dp, stderr, NULL, prp->desc); in
dt_bpf_load_progs()
Function dt_bpf_load_progs() used to have a dt_difo_free(). Now it's
gone. Why?
Before I give a R-b, I want to rebase the io provider on this patch.
I've done that, but there are some test problems. Ugh. In all
likelihood, it's a problem with the io provider and not your patch, but
I'd like to understand what's going on.
On 8/24/23 11:13, Kris Van Hees via DTrace-devel wrote:
> Some providers may need to do more complex code generaetion for the
> probe BPF program trampoline. The generation of trampoline code was
> being done after BPF map creation, making it impossible for trampoline
> code to use constructs that require data to be added to BPF maps.
>
> The final BPF program handling has been changed to first create the
> final programs, then create the BPF maps, and finally link and load the
> BPF programs (performing the final relocations) and attaching them to
> their probes.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_bpf.c | 48 ++++++++++++++++++++++++++++++--------------
> libdtrace/dt_bpf.h | 1 +
> libdtrace/dt_cc.c | 4 ++--
> libdtrace/dt_impl.h | 4 ++++
> libdtrace/dt_probe.c | 3 +++
> libdtrace/dt_probe.h | 1 +
> libdtrace/dt_work.c | 4 ++++
> 7 files changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index 9688c444..5ae5f55f 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -1085,12 +1085,11 @@ dt_bpf_reloc_error_prog(dtrace_hdl_t *dtp, dtrace_difo_t *dp)
> }
>
> int
> -dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> +dt_bpf_make_progs(dtrace_hdl_t *dtp, uint_t cflags)
> {
> dt_probe_t *prp;
> dtrace_difo_t *dp;
> dt_ident_t *idp = dt_dlib_get_func(dtp, "dt_error");
> - dtrace_optval_t dest_ok = DTRACEOPT_UNSET;
>
> assert(idp != NULL);
>
> @@ -1108,19 +1107,11 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> idp->di_flags |= DT_IDFLG_CGREG; /* mark it as inline-ready */
> dt_bpf_reloc_error_prog(dtp, dp);
>
> - /*
> - * Determine whether we can allow destructive actions.
> - */
> - dtrace_getopt(dtp, "destructive", &dest_ok);
> -
> /*
> * Now construct all the other programs.
> */
> for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
> prp = dt_list_next(prp)) {
> - int fd;
> - int rc = -1;
> -
> /* Already done. */
> if (prp == dtp->dt_error)
> continue;
> @@ -1132,19 +1123,46 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> if (prp->prov->impl->prog_type == BPF_PROG_TYPE_UNSPEC)
> continue;
>
> - dp = dt_program_construct(dtp, prp, cflags, NULL);
> + dp = dt_construct(dtp, prp, cflags, NULL);
> if (dp == NULL)
> return -1;
> - if (dp->dtdo_flags & DIFOFLG_DESTRUCTIVE &&
> +
> + prp->difo = dp;
> + }
> +
> + return 0;
> +}
> +
> +int
> +dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> +{
> + dt_probe_t *prp;
> + dtrace_optval_t dest_ok = DTRACEOPT_UNSET;
> +
> + /*
> + * Determine whether we can allow destructive actions.
> + */
> + dtrace_getopt(dtp, "destructive", &dest_ok);
> +
> + for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
> + prp = dt_list_next(prp)) {
> + int fd;
> + int rc = -1;
> +
> + if (prp->difo == NULL)
> + continue;
> +
> + if (dt_link(dtp, prp, prp->difo, NULL) == -1)
> + return -1;
> +
> + if (prp->difo->dtdo_flags & DIFOFLG_DESTRUCTIVE &&
> dest_ok == DTRACEOPT_UNSET)
> return dt_set_errno(dtp, EDT_DESTRUCTIVE);
>
> - fd = dt_bpf_load_prog(dtp, prp, dp, cflags);
> + fd = dt_bpf_load_prog(dtp, prp, prp->difo, cflags);
> if (fd == -1)
> return -1;
>
> - dt_difo_free(dtp, dp);
> -
> if (prp->prov->impl->attach)
> rc = prp->prov->impl->attach(dtp, prp, fd);
>
> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> index 578b7cf9..0fee533b 100644
> --- a/libdtrace/dt_bpf.h
> +++ b/libdtrace/dt_bpf.h
> @@ -63,6 +63,7 @@ extern int dt_bpf_prog_load(enum bpf_prog_type prog_type,
> const dtrace_difo_t *dp, uint32_t log_level,
> char *log_buf, size_t log_buf_sz);
> extern int dt_bpf_raw_tracepoint_open(const void *tp, int fd);
> +extern int dt_bpf_make_progs(struct dtrace_hdl *, uint_t);
> extern int dt_bpf_load_progs(struct dtrace_hdl *, uint_t);
> extern void dt_bpf_init_helpers(struct dtrace_hdl *dtp);
>
> diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> index 86807caa..a42109f1 100644
> --- a/libdtrace/dt_cc.c
> +++ b/libdtrace/dt_cc.c
> @@ -775,7 +775,7 @@ out:
> return err ? NULL : rv;
> }
>
> -static dtrace_difo_t *
> +dtrace_difo_t *
> dt_construct(dtrace_hdl_t *dtp, dt_probe_t *prp, uint_t cflags, dt_ident_t *idp)
> {
> dt_pcb_t pcb;
> @@ -1264,7 +1264,7 @@ dt_link_resolve(dtrace_hdl_t *dtp, dtrace_difo_t *dp)
> }
> }
>
> -static int
> +int
> dt_link(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> dt_ident_t *idp)
> {
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index de46d3db..674905b9 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -803,6 +803,10 @@ extern int dt_load_libs(dtrace_hdl_t *dtp);
> extern void *dt_compile(dtrace_hdl_t *dtp, int context,
> dtrace_probespec_t pspec, void *arg, uint_t cflags,
> int argc, char *const argv[], FILE *fp, const char *s);
> +extern dtrace_difo_t *dt_construct(dtrace_hdl_t *dtp, struct dt_probe *prp,
> + uint_t cflags, dt_ident_t *idp);
> +extern int dt_link(dtrace_hdl_t *dtp, const struct dt_probe *prp,
> + dtrace_difo_t *dp, dt_ident_t *idp);
> extern dtrace_difo_t *dt_program_construct(dtrace_hdl_t *dtp,
> struct dt_probe *prp, uint_t cflags,
> dt_ident_t *idp);
> diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
> index 8f32adce..93266ff7 100644
> --- a/libdtrace/dt_probe.c
> +++ b/libdtrace/dt_probe.c
> @@ -477,6 +477,9 @@ dt_probe_destroy(dt_probe_t *prp)
> else
> dtp = yypcb->pcb_hdl;
>
> + if (prp->difo)
> + dt_difo_free(dtp, prp->difo);
> +
> if (prp->desc) {
> dtp->dt_probes[prp->desc->id] = NULL;
>
> diff --git a/libdtrace/dt_probe.h b/libdtrace/dt_probe.h
> index 4b9396a7..c9839124 100644
> --- a/libdtrace/dt_probe.h
> +++ b/libdtrace/dt_probe.h
> @@ -54,6 +54,7 @@ typedef struct dt_probe {
> dtrace_typeinfo_t *argv; /* output argument types */
> int argc; /* output argument count */
> dt_probe_instance_t *pr_inst; /* list of functions and offsets */
> + dtrace_difo_t *difo; /* BPF probe program */
> } dt_probe_t;
>
> extern dt_probe_t *dt_probe_lookup2(dt_provider_t *, const char *);
> diff --git a/libdtrace/dt_work.c b/libdtrace/dt_work.c
> index 4492f563..1bb6104c 100644
> --- a/libdtrace/dt_work.c
> +++ b/libdtrace/dt_work.c
> @@ -150,6 +150,10 @@ dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
> setrlimit(RLIMIT_MEMLOCK, &rl);
> }
>
> + /* Create the BPF programs. */
> + if (dt_bpf_make_progs(dtp, cflags) == -1)
> + return -1;
> +
> /* Create the global BPF maps. */
> if (dt_bpf_gmap_create(dtp) == -1)
> return -1;
More information about the DTrace-devel
mailing list