[DTrace-devel] [PATCH 1/6] bpf: add 'agggen' BPF map
Kris Van Hees
kris.van.hees at oracle.com
Fri Feb 24 17:15:38 UTC 2023
On Thu, Feb 23, 2023 at 07:41:28PM -0500, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
>
> It's a little weird that the name dt_genmap_fd does not have the substring
> "agg" in it, though. The fd is definitely related to aggs, and the map is
> even called agggen.
>
> If this mechanism will also be used for trunc(), it'd be nice (but not
> necessary) to mention this in the commit message.
We do not know yet.
> To me, the commit message is unnecessarily vague when it could be more
> specific. E.g., "It will be used..." Okay, but how? To me, a more
> specific roadmap could be given:
>
> *) The consumer will write the per-aggid counter to the map: 1 at first,
> and incrementing it at every clear().
>
> *) The producer will use the counter to mark the per-aggid, per-CPU,
> per-key aggs that it writes and maintains.
The commit message is not meant to be a roadmap.
>
> On 2/22/23 10:17, Kris Van Hees via DTrace-devel wrote:
> > The 'agggen' BPF map associates a generation counter with an aggregation
> > ID. It will be used to determine whether the aggregation data in the
> > 'aggs' map for aggregations associated with a given aggregation ID is
> > valid or stale.
> >
> > The initial generation counter for each aggregation is 1 (because 0 is
> > used to indicate a no-data condition).
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_bpf.c | 38 +++++++++++++++++++++++------
> > libdtrace/dt_dlibs.c | 1 +
> > libdtrace/dt_impl.h | 1 +
> > test/stress/buffering/err.resize2.r | 2 +-
> > 4 files changed, 33 insertions(+), 9 deletions(-)
> >
> > diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> > index f0f46c5c..6587ea95 100644
> > --- a/libdtrace/dt_bpf.c
> > +++ b/libdtrace/dt_bpf.c
> > @@ -477,18 +477,23 @@ gmap_create_state(dtrace_hdl_t *dtp)
> > }
> > /*
> > - * Create the 'aggs' BPF map.
> > + * Create the 'aggs' BPF map (and its companion 'agggen' BPF map).
> > *
> > - * Aggregation data buffer map, associated with each CPU. The map is
> > - * implemented as a global array-of-maps indexed by CPU id. The associated
> > - * value is a map with a singleton element (key 0).
> > + * - aggs: Aggregation data buffer map, associated with each CPU. The map
> > + * is implemented as a global array-of-maps indexed by CPU id.
> > + * The associated value is a map with a singleton element (key 0).
> > + * - agggen: Aggregation generation counters. The map associates a
> > + * generation counter with each aggregation ID. The counter is
> > + * used to determine whether the aggregation data is valid. The
> > + * initial value is 1.
> > */
> > static int
> > gmap_create_aggs(dtrace_hdl_t *dtp)
> > {
> > - size_t ncpus = dtp->dt_conf.max_cpuid + 1;
> > - size_t nelems = 0;
> > - int i;
> > + size_t ncpus = dtp->dt_conf.max_cpuid + 1;
> > + size_t nelems = 0;
> > + uint32_t aggc = dt_idhash_peekid(dtp->dt_aggs);
> > + uint32_t i;
> > /* Only create the map if it is used. */
> > if (dtp->dt_maxaggdsize == 0)
> > @@ -508,6 +513,8 @@ gmap_create_aggs(dtrace_hdl_t *dtp)
> > BPF_MAP_TYPE_HASH,
> > dtp->dt_maxtuplesize,
> > dtp->dt_maxaggdsize, nelems);
> > + if (dtp->dt_aggmap_fd == -1)
> > + return -1;
> > for (i = 0; i < dtp->dt_conf.num_online_cpus; i++) {
> > int cpu = dtp->dt_conf.cpus[i].cpu_id;
> > @@ -524,8 +531,23 @@ gmap_create_aggs(dtrace_hdl_t *dtp)
> > dt_bpf_map_update(dtp->dt_aggmap_fd, &cpu, &fd);
> > }
> > + /* Create the agg generation value array. */
> > + dtp->dt_genmap_fd = create_gmap(dtp, "agggen", BPF_MAP_TYPE_ARRAY,
> > + sizeof(uint32_t), sizeof(uint64_t),
> > + aggc);
> > + if (dtp->dt_genmap_fd == -1)
> > + return -1;
> > +
> > + for (i = 0; i < aggc; i++) {
> > + uint64_t val = 1;
> > - return dtp->dt_aggmap_fd;
> > + if (dt_bpf_map_update(dtp->dt_genmap_fd, &i, &val) == -1)
> > + return dt_bpf_error(dtp,
> > + "cannot update BPF map 'agggen': %s\n",
> > + strerror(errno));
> > + }
> > +
> > + return 0;
> > }
> > /*
> > diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
> > index c302b19c..eea0ba41 100644
> > --- a/libdtrace/dt_dlibs.c
> > +++ b/libdtrace/dt_dlibs.c
> > @@ -57,6 +57,7 @@ static const dt_ident_t dt_bpf_symbols[] = {
> > /* BPF maps */
> > DT_BPF_SYMBOL(aggs, DT_IDENT_PTR),
> > + DT_BPF_SYMBOL(agggen, DT_IDENT_PTR),
> > DT_BPF_SYMBOL(buffers, DT_IDENT_PTR),
> > DT_BPF_SYMBOL(cpuinfo, DT_IDENT_PTR),
> > DT_BPF_SYMBOL(dvars, DT_IDENT_PTR),
> > diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> > index 3e14d69d..32b0c923 100644
> > --- a/libdtrace/dt_impl.h
> > +++ b/libdtrace/dt_impl.h
> > @@ -380,6 +380,7 @@ struct dtrace_hdl {
> > int dt_proc_fd; /* file descriptor for proc eventfd */
> > int dt_stmap_fd; /* file descriptor for the 'state' BPF map */
> > int dt_aggmap_fd; /* file descriptor for the 'aggs' BPF map */
> > + int dt_genmap_fd; /* file descriptor for the 'agggen' BPF map */
> > dtrace_handle_err_f *dt_errhdlr; /* error handler, if any */
> > void *dt_errarg; /* error handler argument */
> > dtrace_handle_drop_f *dt_drophdlr; /* drop handler, if any */
> > diff --git a/test/stress/buffering/err.resize2.r b/test/stress/buffering/err.resize2.r
> > index a4fbe390..2b5728ca 100644
> > --- a/test/stress/buffering/err.resize2.r
> > +++ b/test/stress/buffering/err.resize2.r
> > @@ -1,3 +1,3 @@
> > -- @@stderr --
> > dtrace: script 'test/stress/buffering/err.resize2.d' matched 1 probe
> > -dtrace: could not enable tracing: failed to create BPF map 'aggs_0': Too big
> > +dtrace: could not enable tracing: failed to create BPF map 'aggs': Too big
>
> _______________________________________________
> 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