[DTrace-devel] [PATCH 2/2] cc, bpf: separate final BPF program creation from linking and loading
Kris Van Hees
kris.van.hees at oracle.com
Thu Aug 24 20:16:28 UTC 2023
On Thu, Aug 24, 2023 at 04:02:26PM -0400, Eugene Loh via DTrace-devel wrote:
> 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.
Ah yes, I meant to do that after I moved the destructive operations test into
that function. Thanks for catching that.
> 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()
Yes, I have not yet re-added support for dumping the disassembler output
because I wanted to get the patch out so you could work with it and test it.
I'll definitely get those back in the code.
> Function dt_bpf_load_progs() used to have a dt_difo_free(). Now it's gone.
> Why?
Because it gets freed at a later time along with the other DIFO.
> 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;
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list