[DTrace-devel] [PATCH] agg: move aggregation declarations from dt_impl.h to dt_agrgegate.h
Eugene Loh
eugene.loh at oracle.com
Thu Aug 15 22:40:18 UTC 2024
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
I assume it compiles. So that's the main test.
I assume you mean to get rid of the #if 0 blocks (two instances) in
dt_aggregate.h.
Do residual
#define DT_AGGDATA_COUNTER 0
#define DT_AGGDATA_RECORD 1
remain in dt_impl.h?
Mention the dt_aggregate allocation thing.
On 8/15/24 17:07, Kris Van Hees wrote:
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_aggregate.c | 64 +++++++++++++++++++++++-----
> libdtrace/dt_aggregate.h | 91 ++++++++++++++++++++++++++++++++++++++++
> libdtrace/dt_cg.c | 1 +
> libdtrace/dt_consume.c | 1 +
> libdtrace/dt_impl.h | 53 +----------------------
> libdtrace/dt_open.c | 3 ++
> libdtrace/dt_options.c | 6 +--
> libdtrace/dt_printf.c | 3 +-
> libdtrace/dt_work.c | 13 +++---
> 9 files changed, 163 insertions(+), 72 deletions(-)
> create mode 100644 libdtrace/dt_aggregate.h
>
> diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
> index 6472360b..329b8ffa 100644
> --- a/libdtrace/dt_aggregate.c
> +++ b/libdtrace/dt_aggregate.c
> @@ -16,8 +16,32 @@
>
> #include <libproc.h>
> #include <port.h>
> +#include <dt_aggregate.h>
> #include <dt_bpf.h>
>
> +typedef struct dt_ahashent {
> + struct dt_ahashent *dtahe_prev; /* prev on hash chain */
> + struct dt_ahashent *dtahe_next; /* next on hash chain */
> + struct dt_ahashent *dtahe_prevall; /* prev on list of all */
> + struct dt_ahashent *dtahe_nextall; /* next on list of all */
> + uint64_t dtahe_hval; /* hash value */
> + dtrace_aggdata_t dtahe_data; /* data */
> +} dt_ahashent_t;
> +
> +typedef struct dt_ahash {
> + dt_ahashent_t **dtah_hash; /* hash table */
> + dt_ahashent_t *dtah_all; /* list of all elements */
> + size_t dtah_size; /* size of hash table */
> +} dt_ahash_t;
> +
> +struct dt_aggregate {
> + char *dtat_key; /* aggregation key */
> + char *dtat_buf; /* aggregation data buffer */
> + char *dtat_nextkey; /* next aggregation key */
> + int dtat_flags; /* aggregate flags */
> + dt_ahash_t dtat_hash; /* aggregate hash table */
> +};
> +
> #define DTRACE_AHASHSIZE 32779 /* big 'ol prime */
>
> /*
> @@ -34,6 +58,21 @@ static int dt_keypos;
> #define DT_LESSTHAN (dt_revsort == 0 ? -1 : 1)
> #define DT_GREATERTHAN (dt_revsort == 0 ? 1 : -1)
>
> +int
> +dt_aggregate_init(dtrace_hdl_t *dtp) {
> + dtp->dt_aggregate = dt_zalloc(dtp, sizeof(dt_aggregate_t));
> + if (dtp->dt_aggregate == NULL)
> + return dt_set_errno(dtp, EDT_NOMEM);
> +
> + return 0;
> +}
> +
> +void
> +dt_aggregate_set_option(dtrace_hdl_t *dtp, uintptr_t opt)
> +{
> + dtp->dt_aggregate->dtat_flags |= opt;
> +}
> +
> static int
> dt_aggregate_countcmp(int64_t *lhs, int64_t *rhs)
> {
> @@ -503,7 +542,7 @@ static int
> dt_aggregate_snap_one(dtrace_hdl_t *dtp, int aggid, int cpu, const char *key,
> const char *data)
> {
> - dt_ahash_t *agh = &dtp->dt_aggregate.dtat_hash;
> + dt_ahash_t *agh = &dtp->dt_aggregate->dtat_hash;
> dt_ahashent_t *h;
> dtrace_aggdesc_t *agg;
> dtrace_aggdata_t *agd;
> @@ -621,7 +660,7 @@ hashnext:
>
> memcpy(ptr, data, size);
> agd->dtada_data = ptr;
> - if (dtp->dt_aggregate.dtat_flags & DTRACE_A_PERCPU) {
> + if (dtp->dt_aggregate->dtat_flags & DTRACE_A_PERCPU) {
> int i, max_cpus = dtp->dt_conf.max_cpuid + 1;
> dtrace_recdesc_t *rec = &agg->dtagd_drecs[DT_AGGDATA_RECORD];
>
> @@ -658,7 +697,7 @@ hashnext:
> static int
> dt_aggregate_snap_cpu(dtrace_hdl_t *dtp, processorid_t cpu, int fd)
> {
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> + dt_aggregate_t *agp = dtp->dt_aggregate;
> size_t ksize = dtp->dt_maxtuplesize;
> char *key = agp->dtat_key;
> char *data = agp->dtat_buf;
> @@ -692,7 +731,7 @@ int
> dtrace_aggregate_snap(dtrace_hdl_t *dtp)
> {
> dtrace_optval_t interval = dtp->dt_options[DTRACEOPT_AGGRATE];
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> + dt_aggregate_t *agp = dtp->dt_aggregate;
> int i, rval;
>
> /* Has tracing started yet? */
> @@ -1104,7 +1143,7 @@ dt_aggregate_bundlecmp(const void *lhs, const void *rhs)
> int
> dt_aggregate_go(dtrace_hdl_t *dtp)
> {
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> + dt_aggregate_t *agp = dtp->dt_aggregate;
> dt_ahash_t *agh = &agp->dtat_hash;
>
> /* If there are no aggregations there is nothing to do. */
> @@ -1149,7 +1188,7 @@ dt_aggwalk_remove(dtrace_hdl_t *dtp, dt_ahashent_t *h)
> {
> dtrace_aggdata_t *agd = &h->dtahe_data;
> int i, ncpus = dtp->dt_conf.num_online_cpus;
> - char *key = dtp->dt_aggregate.dtat_key;
> + char *key = dtp->dt_aggregate->dtat_key;
>
> memset(key, 0, dtp->dt_maxtuplesize);
> memcpy(key, agd->dtada_key, agd->dtada_desc->dtagd_ksize);
> @@ -1171,7 +1210,7 @@ dt_aggwalk_remove(dtrace_hdl_t *dtp, dt_ahashent_t *h)
> static int
> dt_aggwalk_rval(dtrace_hdl_t *dtp, dt_ahashent_t *h, int rval)
> {
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> + dt_aggregate_t *agp = dtp->dt_aggregate;
>
> switch (rval) {
> case DTRACE_AGGWALK_NEXT:
> @@ -1281,7 +1320,7 @@ int
> dtrace_aggregate_walk(dtrace_hdl_t *dtp, dtrace_aggregate_f *func, void *arg)
> {
> dt_ahashent_t *h, *next;
> - dt_ahash_t *hash = &dtp->dt_aggregate.dtat_hash;
> + dt_ahash_t *hash = &dtp->dt_aggregate->dtat_hash;
>
> for (h = hash->dtah_all; h != NULL; h = next) {
> /*
> @@ -1302,7 +1341,7 @@ static int
> dt_aggregate_walk_sorted(dtrace_hdl_t *dtp, dtrace_aggregate_f *func,
> void *arg, int (*sfunc)(const void *, const void *))
> {
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> + dt_aggregate_t *agp = dtp->dt_aggregate;
> dt_ahashent_t *h, **sorted;
> dt_ahash_t *hash = &agp->dtat_hash;
> size_t i, nentries = 0;
> @@ -1428,7 +1467,7 @@ dtrace_aggregate_walk_joined(dtrace_hdl_t *dtp, dtrace_aggid_t *aggvars,
> int naggvars, dtrace_aggregate_walk_joined_f *func,
> void *arg)
> {
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> + dt_aggregate_t *agp = dtp->dt_aggregate;
> dt_ahashent_t *h, **sorted = NULL, ***bundle, **nbundle;
> const dtrace_aggdata_t **data;
> dt_ahashent_t *zaggdata = NULL;
> @@ -1829,7 +1868,7 @@ dtrace_aggregate_clear(dtrace_hdl_t *dtp)
> void
> dt_aggregate_destroy(dtrace_hdl_t *dtp)
> {
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> + dt_aggregate_t *agp = dtp->dt_aggregate;
> dt_ahash_t *hash = &agp->dtat_hash;
> dt_ahashent_t *h, *next;
> dtrace_aggdata_t *agd;
> @@ -1864,4 +1903,7 @@ dt_aggregate_destroy(dtrace_hdl_t *dtp)
> dt_free(dtp, agp->dtat_key);
> dt_free(dtp, agp->dtat_buf);
> dt_free(dtp, agp->dtat_nextkey);
> + dt_free(dtp, agp);
> +
> + dtp->dt_aggregate = NULL;
> }
> diff --git a/libdtrace/dt_aggregate.h b/libdtrace/dt_aggregate.h
> new file mode 100644
> index 00000000..33921a72
> --- /dev/null
> +++ b/libdtrace/dt_aggregate.h
> @@ -0,0 +1,91 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +#ifndef _DT_AGGREGATE_H
> +#define _DT_AGGREGATE_H
> +
> +#if 0
> +#include <sys/param.h>
> +#include <setjmp.h>
> +#include <sys/ctf-api.h>
> +#include <dtrace.h>
> +#include <pthread.h>
> +
> +#include <sys/types.h>
> +#include <sys/dtrace_types.h>
> +#include <sys/utsname.h>
> +#include <sys/compiler.h>
> +#include <math.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <stddef.h>
> +#include <bpf_asm.h>
> +#endif
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#if 0
> +#include <dt_bpf_maps.h>
> +#include <dt_parser.h>
> +#include <dt_regset.h>
> +#include <dt_rodata.h>
> +#include <dt_strtab.h>
> +#include <dt_symtab.h>
> +#include <dt_ident.h>
> +#include <dt_htab.h>
> +#include <dt_list.h>
> +#include <dt_decl.h>
> +#include <dt_as.h>
> +#include <dt_proc.h>
> +#include <dt_pcap.h>
> +#include <dt_dof.h>
> +#include <dt_pcb.h>
> +#include <dt_printf.h>
> +#include <dt_debug.h>
> +#include <dt_version.h>
> +#endif
> +
> +typedef struct dt_aggregate dt_aggregate_t;
> +
> +#define DT_AGGDATA_COUNTER 0
> +#define DT_AGGDATA_RECORD 1
> +
> +/*
> + * Aggregation functions.
> + */
> +#define DT_AGG_BASE DTRACEACT_AGGREGATION
> +#define DT_AGG(n) (DT_AGG_BASE + (n))
> +#define DT_AGG_IDX(n) ((n) - DT_AGG_BASE)
> +#define DT_AGG_NUM (DT_AGG_IDX(DT_AGG_HIGHEST))
> +
> +typedef enum dt_aggfid {
> + DT_AGG_AVG = DT_AGG_BASE,
> + DT_AGG_COUNT,
> + DT_AGG_LLQUANTIZE,
> + DT_AGG_LQUANTIZE,
> + DT_AGG_MAX,
> + DT_AGG_MIN,
> + DT_AGG_QUANTIZE,
> + DT_AGG_STDDEV,
> + DT_AGG_SUM,
> +
> + DT_AGG_HIGHEST
> +} dt_aggfid_t;
> +
> +extern int dt_aggregate_init(dtrace_hdl_t *);
> +extern void dt_aggregate_set_option(dtrace_hdl_t *, uintptr_t);
> +extern int dt_aggregate_go(dtrace_hdl_t *);
> +extern int dt_aggregate_clear_one(const dtrace_aggdata_t *, void *);
> +extern void dt_aggregate_destroy(dtrace_hdl_t *);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _DT_AGGREGATE_H */
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 164f9e5f..cce2535c 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -16,6 +16,7 @@
> #include <linux/if_arp.h> /* ARPHRD_ETHER ARPHRD_INFINIBAND */
>
> #include <dt_impl.h>
> +#include <dt_aggregate.h>
> #include <dt_dis.h>
> #include <dt_dctx.h>
> #include <dt_cg.h>
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 07856746..6ad73324 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -14,6 +14,7 @@
> #include <ctype.h>
> #include <alloca.h>
> #include <dt_impl.h>
> +#include <dt_aggregate.h>
> #include <dt_module.h>
> #include <dt_pcap.h>
> #include <dt_peb.h>
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index 88b31ce0..fa2b2602 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -83,6 +83,7 @@ extern "C" {
> #define DT_NULL_STRING ((uint16_t)0x7f00)
> #endif
>
> +struct dt_aggregate; /* see <dt_aggregate.h> */
> struct dt_module; /* see below */
> struct dt_pfdict; /* see <dt_printf.h> */
> struct dt_arg; /* see below */
> @@ -197,21 +198,6 @@ typedef struct dt_kern_path {
> #define DT_DM_CTF_ARCHIVED 0x4 /* module found in a CTF archive */
> #define DT_DM_KERN_UNLOADED 0x8 /* module not loaded into the kernel */
>
> -typedef struct dt_ahashent {
> - struct dt_ahashent *dtahe_prev; /* prev on hash chain */
> - struct dt_ahashent *dtahe_next; /* next on hash chain */
> - struct dt_ahashent *dtahe_prevall; /* prev on list of all */
> - struct dt_ahashent *dtahe_nextall; /* next on list of all */
> - uint64_t dtahe_hval; /* hash value */
> - dtrace_aggdata_t dtahe_data; /* data */
> -} dt_ahashent_t;
> -
> -typedef struct dt_ahash {
> - dt_ahashent_t **dtah_hash; /* hash table */
> - dt_ahashent_t *dtah_all; /* list of all elements */
> - size_t dtah_size; /* size of hash table */
> -} dt_ahash_t;
> -
> #define DT_AGGDATA_COUNTER 0
> #define DT_AGGDATA_RECORD 1
>
> @@ -245,14 +231,6 @@ typedef struct dt_tstring {
> int in_use; /* In use (1) or not (0) */
> } dt_tstring_t;
>
> -typedef struct dt_aggregate {
> - char *dtat_key; /* aggregation key */
> - char *dtat_buf; /* aggregation data buffer */
> - char *dtat_nextkey; /* next aggregation key */
> - int dtat_flags; /* aggregate flags */
> - dt_ahash_t dtat_hash; /* aggregate hash table */
> -} dt_aggregate_t;
> -
> typedef struct dt_dirpath {
> dt_list_t dir_list; /* linked-list forward/back pointers */
> char *dir_path; /* directory pathname */
> @@ -387,7 +365,7 @@ struct dtrace_hdl {
> size_t dt_maxagg; /* max aggregation ID */
> dtrace_aggdesc_t **dt_adesc; /* aggregation descriptions */
> int dt_maxformat; /* max format ID */
> - dt_aggregate_t dt_aggregate; /* aggregate */
> + struct dt_aggregate *dt_aggregate; /* aggregate */
> struct dt_pebset *dt_pebset; /* perf event buffers set */
> struct dt_pfdict *dt_pfdict; /* dictionary of printf conversions */
> dt_version_t dt_vmax; /* optional ceiling on program API binding */
> @@ -593,28 +571,6 @@ struct dtrace_hdl {
>
> #define DT_ACT_MAX 31
>
> -/*
> - * Aggregation functions.
> - */
> -#define DT_AGG_BASE DTRACEACT_AGGREGATION
> -#define DT_AGG(n) (DT_AGG_BASE + (n))
> -#define DT_AGG_IDX(n) ((n) - DT_AGG_BASE)
> -#define DT_AGG_NUM (DT_AGG_IDX(DT_AGG_HIGHEST))
> -
> -typedef enum dt_aggfid {
> - DT_AGG_AVG = DT_AGG_BASE,
> - DT_AGG_COUNT,
> - DT_AGG_LLQUANTIZE,
> - DT_AGG_LQUANTIZE,
> - DT_AGG_MAX,
> - DT_AGG_MIN,
> - DT_AGG_QUANTIZE,
> - DT_AGG_STDDEV,
> - DT_AGG_SUM,
> -
> - DT_AGG_HIGHEST
> -} dt_aggfid_t;
> -
> /*
> * Sentinel to tell freopen() to restore the saved stdout. This must not
> * be ever valid for opening for write access via freopen(3C), which of
> @@ -828,11 +784,6 @@ extern int dt_reduce(dtrace_hdl_t *, dt_version_t);
> extern dtrace_difo_t *dt_as(dt_pcb_t *);
> extern dtrace_difo_t *dt_difo_copy(dtrace_hdl_t *dtp, const dtrace_difo_t *odp);
>
> -extern int dt_aggregate_go(dtrace_hdl_t *);
> -extern int dt_aggregate_init(dtrace_hdl_t *);
> -extern void dt_aggregate_destroy(dtrace_hdl_t *);
> -extern int dt_aggregate_clear_one(const dtrace_aggdata_t *, void *);
> -
> extern int dt_consume_init(dtrace_hdl_t *);
> extern void dt_consume_fini(dtrace_hdl_t *);
>
> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> index 5c922488..77ffb6d2 100644
> --- a/libdtrace/dt_open.c
> +++ b/libdtrace/dt_open.c
> @@ -28,6 +28,7 @@
> #include <libproc.h>
>
> #include <dt_impl.h>
> +#include <dt_aggregate.h>
> #include <dt_bpf.h>
> #include <dt_pcap.h>
> #include <dt_program.h>
> @@ -740,6 +741,8 @@ dt_vopen(int version, int flags, int *errp,
> dtp->dt_proc_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
> dtp->dt_nextepid = 1;
> dtp->dt_maxprobe = 0;
> + if (dt_aggregate_init(dtp) == -1)
> + return set_open_errno(dtp, errp, dtrace_errno(dtp));
> dtp->dt_vmax = DT_VERS_LATEST;
> dtp->dt_cpp_path = strdup(_dtrace_defcpp);
> dtp->dt_cpp_argv = malloc(sizeof(char *));
> diff --git a/libdtrace/dt_options.c b/libdtrace/dt_options.c
> index 997f11a2..2f7917a0 100644
> --- a/libdtrace/dt_options.c
> +++ b/libdtrace/dt_options.c
> @@ -20,6 +20,7 @@
> #include <ctype.h>
>
> #include <dt_impl.h>
> +#include <dt_aggregate.h>
> #include <dt_pcap.h>
> #include <dt_string.h>
> #include <libproc.h>
> @@ -27,12 +28,11 @@
> static int
> dt_opt_agg(dtrace_hdl_t *dtp, const char *arg, uintptr_t option)
> {
> - dt_aggregate_t *agp = &dtp->dt_aggregate;
> -
> if (arg != NULL)
> return dt_set_errno(dtp, EDT_BADOPTVAL);
>
> - agp->dtat_flags |= option;
> + dt_aggregate_set_option(dtp, option);
> +
> return 0;
> }
>
> diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> index e999498c..21ec7411 100644
> --- a/libdtrace/dt_printf.c
> +++ b/libdtrace/dt_printf.c
> @@ -14,10 +14,11 @@
> #include <limits.h>
> #include <port.h>
>
> +#include <dt_impl.h>
> +#include <dt_aggregate.h>
> #include <dt_module.h>
> #include <dt_printf.h>
> #include <dt_string.h>
> -#include <dt_impl.h>
>
> /*ARGSUSED*/
> static int
> diff --git a/libdtrace/dt_work.c b/libdtrace/dt_work.c
> index 69a86358..13483301 100644
> --- a/libdtrace/dt_work.c
> +++ b/libdtrace/dt_work.c
> @@ -5,12 +5,6 @@
> * http://oss.oracle.com/licenses/upl.
> */
>
> -#include <dt_impl.h>
> -#include <dt_peb.h>
> -#include <dt_probe.h>
> -#include <dt_bpf.h>
> -#include <dt_bpf_maps.h>
> -#include <dt_state.h>
> #include <stddef.h>
> #include <errno.h>
> #include <assert.h>
> @@ -20,6 +14,13 @@
> #include <linux/perf_event.h>
> #include <sys/epoll.h>
> #include <valgrind/valgrind.h>
> +#include <dt_impl.h>
> +#include <dt_aggregate.h>
> +#include <dt_peb.h>
> +#include <dt_probe.h>
> +#include <dt_bpf.h>
> +#include <dt_bpf_maps.h>
> +#include <dt_state.h>
>
> void
> BEGIN_probe(void)
More information about the DTrace-devel
mailing list