[DTrace-devel] [PATCH 1/3] Implement a 'state' BPF map to communicate tracing session state
Kris Van Hees
kris.van.hees at oracle.com
Fri Sep 4 12:12:39 PDT 2020
On Fri, Sep 04, 2020 at 11:41:54AM -0700, Eugene Loh wrote:
> Looks good with two minor comments:
>
> *) As we apparently agree, the dt_bpf_map_[lookup|update] code movement
> does not belong in this patch.
Indeed.
> *) The naming of the values DT_STATE_BEGANON and DT_STATE_ENDEDON
> strike me as funny. A matter of opinion, to be sure. I guess "anon"
> and "ANON" make me think of anonymous tracing; yes, I realize that's
> not a thing in this version of DTrace, but so be it. Also, the strings
> BEGIN and END are common, while the past tenses BEGAN and ENDED are
> not. Also, "on" is less descriptive than, e.g., "CPU". So, my $0.02
> would be that names like DT_STATE_BEGIN_CPU and DT_STATE_END_CPU would
> be clearer, but I do not intend to wage a war of opinions. So... for
> your consideration.
Beganon and endedon are the names used in the legacy implementation. We
used for the same purpose as they were used there so retaining that makes
sense.
> On 08/27/2020 11:54 AM, Kris Van Hees wrote:
> > The is a need to communicate information about the tracing session
> > between the BPF programs we generate and the userspace consumer. The
> > current need covers:
> >
> > - DT_STATE_ACTIVITY Records the activity state of the session
> > - DT_STATE_BEGANON Records the CPU the BEGIN probe ran on
> > - DT_STATE_ENDEDON Records the CPU the END probe ran on
> >
> > Patches will be forthcoming that provide the implementation of these
> > features.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_bpf.c | 73 +++++++++++++++++++++++++-------------------
> > libdtrace/dt_dlibs.c | 1 +
> > libdtrace/dt_impl.h | 1 +
> > libdtrace/dt_state.h | 27 ++++++++++++++++
> > 4 files changed, 71 insertions(+), 31 deletions(-)
> > create mode 100644 libdtrace/dt_state.h
> >
> > diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> > index 0531a37e..a381a86e 100644
> > --- a/libdtrace/dt_bpf.c
> > +++ b/libdtrace/dt_bpf.c
> > @@ -14,6 +14,7 @@
> > #include <dtrace.h>
> > #include <dt_impl.h>
> > #include <dt_probe.h>
> > +#include <dt_state.h>
> > #include <dt_bpf.h>
> > #include <port.h>
> >
> > @@ -51,6 +52,37 @@ dt_bpf_error(dtrace_hdl_t *dtp, const char *fmt, ...)
> > return dt_set_errno(dtp, EDT_BPF);
> > }
> >
> > +/*
> > + * Load the value for the given key in the map referenced by the given fd.
> > + */
> > +int dt_bpf_map_lookup(int fd, const void *key, void *val)
> > +{
> > + union bpf_attr attr;
> > +
> > + memset(&attr, 0, sizeof(attr));
> > + attr.map_fd = fd;
> > + attr.key = (uint64_t)(unsigned long)key;
> > + attr.value = (uint64_t)(unsigned long)val;
> > +
> > + return bpf(BPF_MAP_LOOKUP_ELEM, &attr);
> > +}
> > +
> > +/*
> > + * Store the (key, value) pair in the map referenced by the given fd.
> > + */
> > +int dt_bpf_map_update(int fd, const void *key, const void *val)
> > +{
> > + union bpf_attr attr;
> > +
> > + memset(&attr, 0, sizeof(attr));
> > + attr.map_fd = fd;
> > + attr.key = (uint64_t)(unsigned long)key;
> > + attr.value = (uint64_t)(unsigned long)val;
> > + attr.flags = 0;
> > +
> > + return bpf(BPF_MAP_UPDATE_ELEM, &attr);
> > +}
> > +
> > static int
> > create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
> > int ksz, int vsz, int size)
> > @@ -86,6 +118,9 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
> > * Create the global BPF maps that are shared between all BPF programs in a
> > * single tracing session:
> > *
> > + * - state: DTrace session state, used to communicate state between BPF
> > + * programs and userspace. The content of the map is defined in
> > + * dt_state.h.
> > * - buffers: Perf event output buffer map, associating a perf event output
> > * buffer with each CPU. The map is indexed by CPU id.
> > * - cpuinfo: CPU information map, associating a cpuinfo_t structure with
> > @@ -149,6 +184,13 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
> > tvarc = dt_idhash_peekid(dtp->dt_tls) - DIF_VAR_OTHER_UBASE;
> >
> > /* Create global maps as long as there are no errors. */
> > + dtp->dt_stmap_fd = create_gmap(dtp, "state", BPF_MAP_TYPE_ARRAY,
> > + sizeof(DT_STATE_KEY_TYPE),
> > + sizeof(DT_STATE_VAL_TYPE),
> > + DT_STATE_NUM_ELEMS);
> > + if (dtp->dt_stmap_fd == -1)
> > + return -1; /* dt_errno is set for us */
> > +
> > if (create_gmap(dtp, "buffers", BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> > sizeof(uint32_t), sizeof(uint32_t),
> > dtp->dt_conf.num_online_cpus) == -1)
> > @@ -185,37 +227,6 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
> > return 0;
> > }
> >
> > -/*
> > - * Load the value for the given key in the map referenced by the given fd.
> > - */
> > -int dt_bpf_map_lookup(int fd, const void *key, void *val)
> > -{
> > - union bpf_attr attr;
> > -
> > - memset(&attr, 0, sizeof(attr));
> > - attr.map_fd = fd;
> > - attr.key = (uint64_t)(unsigned long)key;
> > - attr.value = (uint64_t)(unsigned long)val;
> > -
> > - return bpf(BPF_MAP_LOOKUP_ELEM, &attr);
> > -}
> > -
> > -/*
> > - * Store the (key, value) pair in the map referenced by the given fd.
> > - */
> > -int dt_bpf_map_update(int fd, const void *key, const void *val)
> > -{
> > - union bpf_attr attr;
> > -
> > - memset(&attr, 0, sizeof(attr));
> > - attr.map_fd = fd;
> > - attr.key = (uint64_t)(unsigned long)key;
> > - attr.value = (uint64_t)(unsigned long)val;
> > - attr.flags = 0;
> > -
> > - return bpf(BPF_MAP_UPDATE_ELEM, &attr);
> > -}
> > -
> > /*
> > * Perform relocation processing on a program.
> > */
> > diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
> > index 83eb4a10..15b8525a 100644
> > --- a/libdtrace/dt_dlibs.c
> > +++ b/libdtrace/dt_dlibs.c
> > @@ -65,6 +65,7 @@ static const dt_ident_t dt_bpf_symbols[] = {
> > DT_BPF_SYMBOL(cpuinfo, DT_IDENT_PTR),
> > DT_BPF_SYMBOL(gvars, DT_IDENT_PTR),
> > DT_BPF_SYMBOL(mem, DT_IDENT_PTR),
> > + DT_BPF_SYMBOL(state, DT_IDENT_PTR),
> > DT_BPF_SYMBOL(strtab, DT_IDENT_PTR),
> > DT_BPF_SYMBOL(tvars, DT_IDENT_PTR),
> > /* BPF internal identifiers */
> > diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> > index 0f79b45f..60095a80 100644
> > --- a/libdtrace/dt_impl.h
> > +++ b/libdtrace/dt_impl.h
> > @@ -339,6 +339,7 @@ struct dtrace_hdl {
> > int dt_ddefs_fd; /* file descriptor for D CTF debugging cache */
> > int dt_stdout_fd; /* file descriptor for saved stdout */
> > int dt_poll_fd; /* file descriptor for event polling */
> > + int dt_stmap_fd; /* file descriptor for the 'state' BPF map */
> > dtrace_handle_err_f *dt_errhdlr; /* error handler, if any */
> > void *dt_errarg; /* error handler argument */
> > dtrace_prog_t *dt_errprog; /* error handler program, if any */
> > diff --git a/libdtrace/dt_state.h b/libdtrace/dt_state.h
> > new file mode 100644
> > index 00000000..9acb66b2
> > --- /dev/null
> > +++ b/libdtrace/dt_state.h
> > @@ -0,0 +1,27 @@
> > +/*
> > + * Licensed under the Universal Permissive License v 1.0 as shown at
> > + * http://oss.oracle.com/licenses/upl.
> > + *
> > + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
> > + */
> > +
> > +#ifndef _DT_STATE_H
> > +#define _DT_STATE_H
> > +
> > +/*
> > + * DTrace 'state' BPF map.
> > + *
> > + * The keys are uint32_t values corresponding to dt_state_elem_t values.
> > + * The values are uint32_t values.
> > + */
> > +#define DT_STATE_KEY_TYPE uint32_t
> > +#define DT_STATE_VAL_TYPE uint32_t
> > +
> > +typedef enum dt_state_elem {
> > + DT_STATE_ACTIVITY = 0, /* activity state of the seesion */
> > + DT_STATE_BEGANON, /* cpu BEGIN probe executed on */
> > + DT_STATE_ENDEDON, /* cpu END probe executed on */
> > + DT_STATE_NUM_ELEMS
> > +} dt_state_elem_t;
> > +
> > +#endif /* _DT_STATE_H */
>
>
> _______________________________________________
> 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