[DTrace-devel] [PATCH v4] Fix records for aggregation descriptions
Kris Van Hees
kris.van.hees at oracle.com
Thu Aug 25 21:22:58 UTC 2022
On Thu, Aug 25, 2022 at 04:15:01PM -0400, Kris Van Hees via DTrace-devel wrote:
> The implementation of aggregations erronously registered the value of
> an aggregation as a series of 64-bit values, each with their own record
> description. However, aggregatios in their most generic form should
> comprise the following records:
>
> (1) variable ID
> (2) keys (values of the index tuple, if any)
> (3) latch sequence number
> (4) aggregation data
>
> (1) and (2) are records that identify the aggregation, whereas (3) and
> (4) are data that is generated by the producer.
>
> Change the aggregation record descriptions to match the layout described
> above.
>
> Note that the variable ID is now stored as a 32-bit value whereas DTrace
> used to store it as a 64-bit value. This change is valid because the
> variable ID is an identifier ID and therefore cannot exceed the range of
> a 32-bit integer,
>
> Support for aggregtion keys will be added in later patches.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> cmd/dtrace.c | 2 +-
> include/dtrace/metadesc.h | 8 +++--
> libdtrace/dt_aggregate.c | 74 +++++++++++++++++++--------------------
> libdtrace/dt_cg.c | 9 ++++-
> libdtrace/dt_consume.c | 41 +++++++++-------------
> libdtrace/dt_impl.h | 3 ++
> libdtrace/dt_map.c | 65 ++++++++++++++++++++--------------
> libdtrace/dt_printf.c | 25 ++++++-------
> 8 files changed, 119 insertions(+), 108 deletions(-)
>
> diff --git a/cmd/dtrace.c b/cmd/dtrace.c
> index 22ac28c3..350cab94 100644
> --- a/cmd/dtrace.c
> +++ b/cmd/dtrace.c
> @@ -682,7 +682,7 @@ bufhandler(const dtrace_bufdata_t *bufdata, void *arg)
> BUFDUMPSTR(desc, dtagd_name);
> BUFDUMP(desc, dtagd_varid);
> BUFDUMP(desc, dtagd_id);
> - BUFDUMP(desc, dtagd_nrecs);
> + BUFDUMP(desc, dtagd_nkrecs + 1);
> BUFDUMPHDR("");
> }
>
> diff --git a/include/dtrace/metadesc.h b/include/dtrace/metadesc.h
> index 20a5b8a0..f75b0a8f 100644
> --- a/include/dtrace/metadesc.h
> +++ b/include/dtrace/metadesc.h
> @@ -57,9 +57,11 @@ typedef struct dtrace_aggdesc {
> dtrace_aggid_t dtagd_id; /* aggregation ID */
> uint64_t dtagd_sig; /* aggregation signature */
> uint64_t dtagd_normal; /* aggregation normalization */
> - uint32_t dtagd_size; /* size in bytes */
> - int dtagd_nrecs; /* number of records */
> - dtrace_recdesc_t *dtagd_recs; /* record descriptions */
> + uint32_t dtagd_ksize; /* keys size in bytes */
> + uint32_t dtagd_dsize; /* data size in bytes */
> + int dtagd_nkrecs; /* number of key records */
> + dtrace_recdesc_t *dtagd_krecs; /* key record descriptions */
> + dtrace_recdesc_t *dtagd_drecs; /* data record descriptions */
> } dtrace_aggdesc_t;
>
> typedef struct dtrace_fmtdesc {
> diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
> index fa65ecd3..f810efd3 100644
> --- a/libdtrace/dt_aggregate.c
> +++ b/libdtrace/dt_aggregate.c
> @@ -390,7 +390,7 @@ dt_aggregate_aggid(dt_ahashent_t *ent)
> {
> dtrace_aggdesc_t *agg = ent->dtahe_data.dtada_desc;
> caddr_t data = ent->dtahe_data.dtada_data;
> - dtrace_recdesc_t *rec = agg->dtagd_recs;
> + dtrace_recdesc_t *rec = agg->dtagd_krecs;
>
> /*
> * First, we'll check the variable ID in the aggdesc. If it's valid,
> @@ -406,8 +406,8 @@ dt_aggregate_aggid(dt_ahashent_t *ent)
> return agg->dtagd_varid;
> }
>
> -typedef void (*agg_cpu_f)(dt_ident_t *aid,
> - int64_t *dst, int64_t *src, uint_t realsz);
> +typedef void (*agg_cpu_f)(dt_ident_t *aid, int64_t *dst, int64_t *src,
> + uint_t datasz);
>
> typedef struct dt_snapstate {
> dtrace_hdl_t *dtp;
> @@ -418,34 +418,34 @@ typedef struct dt_snapstate {
> } dt_snapstate_t;
>
> static void
> -dt_agg_one_copy(dt_ident_t *aid, int64_t *dst, int64_t *src, uint_t realsz)
> +dt_agg_one_copy(dt_ident_t *aid, int64_t *dst, int64_t *src, uint_t datasz)
> {
> - src++; /* skip latch sequence number */
> - memcpy(dst, src, realsz);
> + memcpy(dst, src, datasz);
> }
>
> static void
> -dt_agg_one_agg(dt_ident_t *aid, int64_t *dst, int64_t *src, uint_t realsz)
> +dt_agg_one_agg(dt_ident_t *aid, int64_t *dst, int64_t *src, uint_t datasz)
> {
> uint_t i, cnt;
>
> if (*src == 0)
> return;
>
> - src++; /* skip latch sequence number */
> switch (((dt_ident_t *)aid->di_iarg)->di_id) {
> case DT_AGG_MAX:
> + *dst++ = *src++; /* copy latch sequence number */
> if (*src > *dst)
> *dst = *src;
>
> break;
> case DT_AGG_MIN:
> + *dst++ = *src++; /* copy latch sequence number */
> if (*src < *dst)
> *dst = *src;
>
> break;
> default:
> - for (i = 0, cnt = realsz / sizeof(int64_t);
> + for (i = 0, cnt = datasz / sizeof(int64_t);
> i < cnt; i++, dst++, src++)
> *dst += *src;
> }
> @@ -461,7 +461,7 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
> uint64_t hval = aid->di_id;
> size_t ndx = hval % agh->dtah_size;
> int rval;
> - uint_t i, realsz;
> + uint_t i, datasz;
> int64_t *src;
>
> rval = dt_aggid_lookup(st->dtp, aid->di_id, &agg);
> @@ -471,23 +471,22 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
> /* point to the latch sequence number */
> src = (int64_t *)(st->buf + aid->di_offset);
>
> - /* real size excludes latch sequence number and second data copy, if any */
> - realsz = (aid->di_size - sizeof(uint64_t)) / DT_AGG_NUM_COPIES;
> + datasz = agg->dtagd_dsize;
>
> /* See if we already have an entry for this aggregation. */
> for (h = agh->dtah_hash[ndx]; h != NULL; h = h->dtahe_next) {
> - if (h->dtahe_hval != hval || h->dtahe_size != realsz)
> + if (h->dtahe_hval != hval || h->dtahe_size != datasz)
> continue;
>
> /* Entry found - process the data. */
> agd = &h->dtahe_data;
>
> - st->fun(aid, (int64_t *)agd->dtada_data, src, realsz);
> + st->fun(aid, (int64_t *)agd->dtada_data, src, datasz);
>
> /* If we keep per-CPU data - process that as well. */
> if (agd->dtada_percpu != NULL)
> st->fun(aid, (int64_t *)agd->dtada_percpu[st->cpu],
> - src, realsz);
> + src, datasz);
>
> return 0;
> }
> @@ -502,20 +501,19 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
> return dt_set_errno(st->dtp, EDT_NOMEM);
>
> agd = &h->dtahe_data;
> - agd->dtada_data = dt_alloc(st->dtp, realsz);
> + agd->dtada_data = dt_alloc(st->dtp, datasz);
> if (agd->dtada_data == NULL) {
> dt_free(st->dtp, h);
> return dt_set_errno(st->dtp, EDT_NOMEM);
> }
>
> - src++;
> - memcpy(agd->dtada_data, src, realsz);
> - agd->dtada_size = realsz;
> + memcpy(agd->dtada_data, src, datasz);
> + agd->dtada_size = datasz;
> agd->dtada_desc = agg;
> agd->dtada_hdl = st->dtp;
>
> h->dtahe_hval = hval;
> - h->dtahe_size = realsz;
> + h->dtahe_size = datasz;
>
> if (st->agp->dtat_flags & DTRACE_A_PERCPU) {
> char **percpu = dt_calloc(st->dtp,
> @@ -530,7 +528,7 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
> }
>
> for (i = 0; i <= st->dtp->dt_conf.max_cpuid; i++) {
> - percpu[i] = dt_zalloc(st->dtp, realsz);
> + percpu[i] = dt_zalloc(st->dtp, datasz);
> if (percpu[i] == NULL) {
> while (--i >= 0)
> dt_free(st->dtp, percpu[i]);
> @@ -541,7 +539,7 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
> }
> }
>
> - memcpy(percpu[st->cpu], src, realsz);
> + memcpy(percpu[st->cpu], src, datasz);
> agd->dtada_percpu = percpu;
> }
>
> @@ -617,10 +615,10 @@ dt_aggregate_hashcmp(const void *lhs, const void *rhs)
> dtrace_aggdesc_t *lagg = lh->dtahe_data.dtada_desc;
> dtrace_aggdesc_t *ragg = rh->dtahe_data.dtada_desc;
>
> - if (lagg->dtagd_nrecs < ragg->dtagd_nrecs)
> + if (lagg->dtagd_nkrecs < ragg->dtagd_nkrecs)
> return DT_LESSTHAN;
>
> - if (lagg->dtagd_nrecs > ragg->dtagd_nrecs)
> + if (lagg->dtagd_nkrecs > ragg->dtagd_nkrecs)
> return DT_GREATERTHAN;
>
> return 0;
> @@ -659,8 +657,8 @@ dt_aggregate_keycmp(const void *lhs, const void *rhs)
> if ((rval = dt_aggregate_hashcmp(lhs, rhs)) != 0)
> return rval;
>
> - nrecs = lagg->dtagd_nrecs - 1;
> - assert(nrecs == ragg->dtagd_nrecs - 1);
> + nrecs = lagg->dtagd_nkrecs;
> + assert(nrecs == ragg->dtagd_nkrecs);
>
> keypos = dt_keypos + 1 >= nrecs ? 0 : dt_keypos;
>
> @@ -671,8 +669,8 @@ dt_aggregate_keycmp(const void *lhs, const void *rhs)
> if (ndx >= nrecs)
> ndx = ndx - nrecs + 1;
>
> - lrec = &lagg->dtagd_recs[ndx];
> - rrec = &ragg->dtagd_recs[ndx];
> + lrec = &lagg->dtagd_krecs[ndx];
> + rrec = &ragg->dtagd_krecs[ndx];
>
> ldata = lh->dtahe_data.dtada_data + lrec->dtrd_offset;
> rdata = rh->dtahe_data.dtada_data + rrec->dtrd_offset;
> @@ -772,18 +770,18 @@ dt_aggregate_valcmp(const void *lhs, const void *rhs)
> if ((rval = dt_aggregate_hashcmp(lhs, rhs)) != 0)
> return rval;
>
> - if (lagg->dtagd_nrecs > ragg->dtagd_nrecs)
> + if (lagg->dtagd_nkrecs > ragg->dtagd_nkrecs)
> return DT_GREATERTHAN;
>
> - if (lagg->dtagd_nrecs < ragg->dtagd_nrecs)
> + if (lagg->dtagd_nkrecs < ragg->dtagd_nkrecs)
> return DT_LESSTHAN;
>
> - if (lagg->dtagd_nrecs <= 0)
> + if (lagg->dtagd_nkrecs <= 0)
> return 0;
>
> - for (i = 0; i < lagg->dtagd_nrecs; i++) {
> - lrec = &lagg->dtagd_recs[i];
> - rrec = &ragg->dtagd_recs[i];
> + for (i = 0; i < lagg->dtagd_nkrecs; i++) {
> + lrec = &lagg->dtagd_krecs[i];
> + rrec = &ragg->dtagd_krecs[i];
>
> if (lrec->dtrd_offset < rrec->dtrd_offset)
> return DT_LESSTHAN;
> @@ -1089,7 +1087,7 @@ dt_aggwalk_rval(dtrace_hdl_t *dtp, dt_ahashent_t *h, int rval)
> uint32_t size, offs = 0;
>
> aggdesc = h->dtahe_data.dtada_desc;
> - rec = &aggdesc->dtagd_recs[aggdesc->dtagd_nrecs - 1];
> + rec = &aggdesc->dtagd_drecs[DT_AGGDATA_RECORD];
> size = rec->dtrd_size;
> data = &h->dtahe_data;
>
> @@ -1514,11 +1512,11 @@ dtrace_aggregate_walk_joined(dtrace_hdl_t *dtp, dtrace_aggid_t *aggvars,
> * cons up the zaggdata entry for it.
> */
> aggdata = &zaggdata[i].dtahe_data;
> - aggdata->dtada_size = agg->dtagd_size;
> + aggdata->dtada_size = agg->dtagd_dsize;
> aggdata->dtada_desc = agg;
> aggdata->dtada_hdl = dtp;
> zaggdata[i].dtahe_hval = 0;
> - zaggdata[i].dtahe_size = agg->dtagd_size;
> + zaggdata[i].dtahe_size = agg->dtagd_dsize;
> break;
> }
>
> @@ -1773,7 +1771,7 @@ dtrace_aggregate_clear(dtrace_hdl_t *dtp)
>
> for (h = hash->dtah_all; h != NULL; h = h->dtahe_nextall) {
> aggdesc = h->dtahe_data.dtada_desc;
> - rec = &aggdesc->dtagd_recs[aggdesc->dtagd_nrecs - 1];
> + rec = &aggdesc->dtagd_drecs[DT_AGGDATA_RECORD];
> data = &h->dtahe_data;
>
> memset(&data->dtada_data[rec->dtrd_offset], 0, rec->dtrd_size);
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index ebd41906..7bff5895 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -6712,6 +6712,7 @@ static dt_cg_aggfunc_f *_dt_cg_agg[DT_AGG_NUM] = {
> static void
> dt_cg_agg(dt_pcb_t *pcb, dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> {
> + dtrace_hdl_t *dtp = pcb->pcb_hdl;
> dt_ident_t *aid, *fid;
> dt_cg_aggfunc_f *aggfp;
>
> @@ -6744,7 +6745,13 @@ dt_cg_agg(dt_pcb_t *pcb, dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> assert(aggfp != NULL);
>
> (*aggfp)(pcb, aid, dnp, dlp, drp);
> - dt_aggid_add(pcb->pcb_hdl, aid);
> +
> + /*
> + * Add this aggid if we see it for the first time. We do this after
> + * the BPF code generation because aggfp() sets aid->di_size.
> + */
> + if (dt_aggid_lookup(dtp, aid->di_id, NULL) == -1)
> + dt_aggid_add(dtp, aid);
> }
>
> void
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 973084d6..7e6622d1 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -1001,8 +1001,8 @@ dt_print_bytes(dtrace_hdl_t *dtp, FILE *fp, caddr_t addr,
> return dt_printf(dtp, fp, " %-*s", width, s);
> }
>
> - /* print the bytes raw */
> - return dt_print_rawbytes(dtp, fp, addr, nbytes);
> + /* print the bytes raw */
> + return dt_print_rawbytes(dtp, fp, addr, nbytes);
> }
>
> #ifdef FIXME
> @@ -1519,7 +1519,7 @@ dt_clear_agg(const dtrace_aggdata_t *aggdata, void *arg)
> dtrace_aggdesc_t *agg = aggdata->dtada_desc;
> dtrace_aggid_t id = *((dtrace_aggid_t *)arg);
>
> - if (agg->dtagd_nrecs == 0)
> + if (agg->dtagd_nkrecs == 0)
> return DTRACE_AGGWALK_NEXT;
>
> if (agg->dtagd_varid != id)
> @@ -1540,7 +1540,7 @@ dt_trunc_agg(const dtrace_aggdata_t *aggdata, void *arg)
> dtrace_aggdesc_t *agg = aggdata->dtada_desc;
> dtrace_aggid_t id = trunc->dttd_id;
>
> - if (agg->dtagd_nrecs == 0)
> + if (agg->dtagd_nkrecs == 0)
> return DTRACE_AGGWALK_NEXT;
>
> if (agg->dtagd_varid != id)
> @@ -1620,10 +1620,14 @@ dt_trunc(dtrace_hdl_t *dtp, caddr_t base, dtrace_recdesc_t *rec)
>
> static int
> dt_print_datum(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
> - caddr_t addr, size_t size, uint64_t normal, uint64_t sig)
> + caddr_t addr, uint64_t normal, uint64_t sig)
> {
> int err;
> dtrace_actkind_t act = rec->dtrd_action;
> + size_t size = rec->dtrd_size;
> +
> + /* Apply the record offset to the base address. */
> + addr += rec->dtrd_offset;
>
> switch (act) {
> case DTRACEACT_STACK:
> @@ -1697,31 +1701,22 @@ dt_print_datum(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
> static int
> dt_print_aggs(const dtrace_aggdata_t **aggsdata, int naggvars, void *arg)
> {
> - int i, aggact = 0;
> + int i;
> dtrace_print_aggdata_t *pd = arg;
> const dtrace_aggdata_t *aggdata = aggsdata[0];
> dtrace_aggdesc_t *agg = aggdata->dtada_desc;
> FILE *fp = pd->dtpa_fp;
> dtrace_hdl_t *dtp = pd->dtpa_dtp;
> dtrace_recdesc_t *rec;
> - caddr_t addr;
> - size_t size;
>
> /*
> * Iterate over each record description in the key, printing the traced
> - * data.
> + * data. The first record is skipped because it holds the variable ID.
> */
> - for (i = 0; i < agg->dtagd_nrecs; i++) {
> - rec = &agg->dtagd_recs[i];
> - addr = aggdata->dtada_data + rec->dtrd_offset;
> - size = rec->dtrd_size;
> -
> - if (DTRACEACT_ISAGG(rec->dtrd_action)) {
> - aggact = i;
> - break;
> - }
> + for (i = 1; i < agg->dtagd_nkrecs; i++) {
> + rec = &agg->dtagd_krecs[i];
>
> - if (dt_print_datum(dtp, fp, rec, addr, size, 1, 0) < 0)
> + if (dt_print_datum(dtp, fp, rec, aggdata->dtada_data, 1, 0) < 0)
> return DTRACE_AGGWALK_ERROR;
>
> if (dt_buffered_flush(dtp, NULL, rec, aggdata,
> @@ -1734,14 +1729,12 @@ dt_print_aggs(const dtrace_aggdata_t **aggsdata, int naggvars, void *arg)
>
> aggdata = aggsdata[i];
> agg = aggdata->dtada_desc;
> - rec = &agg->dtagd_recs[aggact];
> - addr = aggdata->dtada_data + rec->dtrd_offset;
> - size = agg->dtagd_size;
> + rec = &agg->dtagd_drecs[DT_AGGDATA_RECORD];
>
> assert(DTRACEACT_ISAGG(rec->dtrd_action));
> normal = aggdata->dtada_desc->dtagd_normal;
>
> - if (dt_print_datum(dtp, fp, rec, addr, size, normal,
> + if (dt_print_datum(dtp, fp, rec, aggdata->dtada_data, normal,
> agg->dtagd_sig) < 0)
> return DTRACE_AGGWALK_ERROR;
>
> @@ -1781,7 +1774,7 @@ dt_print_agg(const dtrace_aggdata_t *aggdata, void *arg)
> * variable that we should print -- skip any other aggregations
> * that we encounter.
> */
> - if (agg->dtagd_nrecs == 0)
> + if (agg->dtagd_nkrecs == 0)
> return DTRACE_AGGWALK_NEXT;
>
> if (aggvarid != agg->dtagd_varid)
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index 0e3cf218..704cfed0 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -188,6 +188,9 @@ typedef struct dt_ahash {
> size_t dtah_size; /* size of hash table */
> } dt_ahash_t;
>
> +#define DT_AGGDATA_COUNTER 0
> +#define DT_AGGDATA_RECORD 1
> +
> /*
> * Why do we need (only) 4 slots? The maximum amount of string arguments to
> * any function is 2, and if the result is a string as well, that means we may
> diff --git a/libdtrace/dt_map.c b/libdtrace/dt_map.c
> index 0098e374..cc09fff0 100644
> --- a/libdtrace/dt_map.c
> +++ b/libdtrace/dt_map.c
> @@ -241,11 +241,9 @@ dt_aggid_add(dtrace_hdl_t *dtp, const dt_ident_t *aid)
> {
> dtrace_id_t max;
> dtrace_aggdesc_t *agg;
> - dtrace_recdesc_t *recs;
> + dtrace_recdesc_t *recs, *drecs;
> dtrace_aggid_t id = aid->di_id;
> - dt_ident_t *fid = aid->di_iarg;
> - int i;
> - uint_t off = 0;
> + int nrecs = ((dt_idsig_t *)aid->di_data)->dis_argc;
>
> while (id >= (max = dtp->dt_maxagg) || dtp->dt_adesc == NULL) {
> dtrace_id_t nmax = max ? (max << 1) : 1;
> @@ -274,39 +272,51 @@ dt_aggid_add(dtrace_hdl_t *dtp, const dt_ident_t *aid)
> if (agg == NULL)
> return dt_set_errno(dtp, EDT_NOMEM);
>
> - /*
> - * Note the relationship between the aggregation storage
> - * size (di_size) and the aggregation data size (dtagd_size):
> - * di_size = dtagd_size * (num copies) + (size of latch seq #)
> - */
> agg->dtagd_id = id;
> agg->dtagd_name = aid->di_name;
> agg->dtagd_sig = ((dt_idsig_t *)aid->di_data)->dis_auxinfo;
> agg->dtagd_normal = 1;
> agg->dtagd_varid = aid->di_id;
> - agg->dtagd_size = (aid->di_size - sizeof(uint64_t)) / DT_AGG_NUM_COPIES;
> - agg->dtagd_nrecs = agg->dtagd_size / sizeof(uint64_t);
> + agg->dtagd_nkrecs = 1 + nrecs; /* var ID and keys */
>
> - recs = dt_calloc(dtp, agg->dtagd_nrecs, sizeof(dtrace_recdesc_t));
> + /* We allocate all records at once (varid, keys, data counter, data). */
> + recs = dt_calloc(dtp, 1 + nrecs + 2, sizeof(dtrace_recdesc_t));
> if (recs == NULL) {
> dt_free(dtp, agg);
> return dt_set_errno(dtp, EDT_NOMEM);
> }
>
> - agg->dtagd_recs = recs;
> -
> - for (i = 0; i < agg->dtagd_nrecs; i++) {
> - dtrace_recdesc_t *rec = &recs[i];
> -
> - rec->dtrd_action = fid->di_id;
> - rec->dtrd_size = sizeof(uint64_t);
> - rec->dtrd_offset = off;
> - rec->dtrd_alignment = sizeof(uint64_t);
> - rec->dtrd_format = NULL;
> - rec->dtrd_arg = 1;
> -
> - off += sizeof(uint64_t);
> - }
> + /* Set up the variable ID record (first record in the keys). */
> + recs[0].dtrd_action = DTRACEACT_DIFEXPR;
> + recs[0].dtrd_size = sizeof(uint32_t);
> + recs[0].dtrd_offset = 0;
> + recs[0].dtrd_alignment = sizeof(uint32_t);
> + recs[0].dtrd_format = NULL;
> + recs[0].dtrd_arg = 1;
> +
> + agg->dtagd_krecs = recs;
> + agg->dtagd_ksize = recs[0].dtrd_size; /* Updated when adding keys */
> +
> + /* Set up the data records. */
> + drecs = &recs[nrecs];
This needs to be:
drecs = &recs[1 + nrecs];
> +
> + drecs[DT_AGGDATA_COUNTER].dtrd_action = ((dt_ident_t *) aid->di_iarg)->di_id;
> + drecs[DT_AGGDATA_COUNTER].dtrd_size = sizeof(uint64_t);
> + drecs[DT_AGGDATA_COUNTER].dtrd_offset = 0;
> + drecs[DT_AGGDATA_COUNTER].dtrd_alignment = sizeof(uint64_t);
> + drecs[DT_AGGDATA_COUNTER].dtrd_format = NULL;
> + drecs[DT_AGGDATA_COUNTER].dtrd_arg = 1;
> +
> + drecs[DT_AGGDATA_RECORD].dtrd_action = ((dt_ident_t *) aid->di_iarg)->di_id;
> + drecs[DT_AGGDATA_RECORD].dtrd_size = aid->di_size - sizeof(uint64_t);
> + drecs[DT_AGGDATA_RECORD].dtrd_offset = sizeof(uint64_t);
> + drecs[DT_AGGDATA_RECORD].dtrd_alignment = sizeof(uint64_t);
> + drecs[DT_AGGDATA_RECORD].dtrd_format = NULL;
> + drecs[DT_AGGDATA_RECORD].dtrd_arg = 1;
> +
> + agg->dtagd_drecs = drecs;
> + agg->dtagd_dsize = drecs[DT_AGGDATA_COUNTER].dtrd_size +
> + drecs[DT_AGGDATA_RECORD].dtrd_size;
>
> dtp->dt_adesc[id] = agg;
>
> @@ -338,7 +348,8 @@ dt_aggid_destroy(dtrace_hdl_t *dtp)
>
> for (i = 0; i < dtp->dt_maxagg; i++) {
> if (dtp->dt_adesc[i] != NULL) {
> - dt_free(dtp, dtp->dt_adesc[i]->dtagd_recs);
> + /* Freeing dtagd_recs also frees dtagd_drecs. */
> + dt_free(dtp, dtp->dt_adesc[i]->dtagd_krecs);
> dt_free(dtp, dtp->dt_adesc[i]);
> }
> }
> diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> index 4fb20b23..4f0e6b8a 100644
> --- a/libdtrace/dt_printf.c
> +++ b/libdtrace/dt_printf.c
> @@ -1264,10 +1264,12 @@ dt_printf_format(dtrace_hdl_t *dtp, FILE *fp, const dt_pfargv_t *pfv,
> dtrace_aggdesc_t *agg;
> caddr_t lim = (caddr_t)buf + len, limit;
> char format[64] = "%";
> - int i, aggrec = 0, curagg = -1;
> + int i, curagg = -1;
> uint64_t normal, sig;
>
> /*
> + * FIXME: Comment needs rewrite.
> + *
> * If we are formatting an aggregation, set 'aggrec' to the index of
> * the final record description (the aggregation result) so we can use
> * this record index with any conversion where DT_PFCONV_AGG is set.
> @@ -1280,13 +1282,8 @@ dt_printf_format(dtrace_hdl_t *dtp, FILE *fp, const dt_pfargv_t *pfv,
> assert(aggsdata != NULL);
> assert(naggvars > 0);
>
> - if (nrecs == 0)
> - return dt_set_errno(dtp, EDT_DMISMATCH);
> -
> curagg = naggvars > 1 ? 1 : 0;
> aggdata = aggsdata[0];
> - aggrec = aggdata->dtada_desc->dtagd_nrecs - 1;
> - nrecs--;
> }
>
> for (i = 0; i < pfv->pfv_argc; i++, pfd = pfd->pfd_next) {
> @@ -1380,11 +1377,10 @@ dt_printf_format(dtrace_hdl_t *dtp, FILE *fp, const dt_pfargv_t *pfv,
> if (curagg < naggvars - 1)
> curagg++;
>
> - rec = &agg->dtagd_recs[aggrec];
> - addr = aggdata->dtada_data;
> + rec = &agg->dtagd_drecs[DT_AGGDATA_RECORD];
> + addr = aggdata->dtada_data + rec->dtrd_offset;
> limit = addr + aggdata->dtada_size;
> normal = agg->dtagd_normal;
> - size = agg->dtagd_size;
> sig = agg->dtagd_sig;
> flags = DTRACE_BUFDATA_AGGVAL;
> } else {
> @@ -1409,10 +1405,11 @@ dt_printf_format(dtrace_hdl_t *dtp, FILE *fp, const dt_pfargv_t *pfv,
> addr = (caddr_t)buf + rec->dtrd_offset;
> limit = lim;
> normal = 1;
> - size = rec->dtrd_size;
> sig = 0;
> }
>
> + size = rec->dtrd_size;
> +
> if (addr + size > limit) {
> dt_dprintf("bad size: addr=%p size=0x%x lim=%p\n",
> (void *)addr, rec->dtrd_size, (void *)lim);
> @@ -1809,8 +1806,8 @@ static int
> dt_fprinta(const dtrace_aggdata_t *adp, void *arg)
> {
> const dtrace_aggdesc_t *agg = adp->dtada_desc;
> - const dtrace_recdesc_t *rec = agg->dtagd_recs;
> - uint_t nrecs = agg->dtagd_nrecs;
> + const dtrace_recdesc_t *rec = agg->dtagd_krecs;
> + uint_t nrecs = agg->dtagd_nkrecs;
> dt_pfwalk_t *pfw = arg;
> dtrace_hdl_t *dtp = pfw->pfw_argv->pfv_dtp;
>
> @@ -1837,8 +1834,8 @@ dt_fprintas(const dtrace_aggdata_t **aggsdata, int naggvars, void *arg)
> {
> const dtrace_aggdata_t *aggdata = aggsdata[0];
> const dtrace_aggdesc_t *agg = aggdata->dtada_desc;
> - const dtrace_recdesc_t *rec = agg->dtagd_recs;
> - uint_t nrecs = agg->dtagd_nrecs;
> + const dtrace_recdesc_t *rec = &agg->dtagd_krecs[1];
> + uint_t nrecs = agg->dtagd_nkrecs - 1;
> dt_pfwalk_t *pfw = arg;
> dtrace_hdl_t *dtp = pfw->pfw_argv->pfv_dtp;
> int i;
> --
> 2.34.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