[DTrace-devel] [PATCH] Implement the trunc() action
Eugene Loh
eugene.loh at oracle.com
Fri Aug 4 14:30:43 UTC 2023
Just a progress report on this one... The patch looks good to me, but
the tests don't seem to work for me. The patch takes the xfail off of
negtrunc and trunc, but trunc0 is the test that is starting to pass for
me. Also, I have a separate test that sets up some agg with multiple
keys, waits a few seconds, truncates, waits a few more seconds, and
exits. The truncation seems incomplete: I can see (in different ways,
but including "bpftool map") that some keys that should be removed
nonetheless remain, and then dtrace finds and reports them. More
specifically, I have a UEKr6 VM on which I see all those problems and a
UEKr7 VM on which I see only the testsuite problems (but no issue with
my special test). I'll poke more, but this is frustrating for a patch
that isn't that long and looks fine to me.
On 8/3/23 01:09, Kris Van Hees via DTrace-devel wrote:
> Some tests will not yield the desired results yet due to issues with
> switchrate/aggrate/statusrate implementation details that those
> tests depend on.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_aggregate.c | 22 ++++++++++++++++++++++
> libdtrace/dt_cg.c | 27 ++++-----------------------
> libdtrace/dt_consume.c | 19 +++++++++++++------
> libdtrace/dt_open.c | 2 +-
> test/unittest/aggs/tst.negtrunc.d | 1 -
> test/unittest/aggs/tst.trunc.d | 1 -
> 6 files changed, 40 insertions(+), 32 deletions(-)
>
> diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
> index 7c09908d..24faf1c1 100644
> --- a/libdtrace/dt_aggregate.c
> +++ b/libdtrace/dt_aggregate.c
> @@ -1086,6 +1086,26 @@ no_mem:
> return dt_set_errno(dtp, EDT_NOMEM);
> }
>
> +static int
> +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;
> +
> + for (i = 0; i < ncpus; i++) {
> + int cpu = dtp->dt_conf.cpus[i].cpu_id;
> + int fd = dt_bpf_map_lookup_fd(dtp->dt_aggmap_fd, &cpu);
> +
> + if (fd < 0)
> + return DTRACE_WORKSTATUS_ERROR;
> +
> + dt_bpf_map_delete(fd, agd->dtada_key);
> + close(fd);
> + }
> +
> + return DTRACE_WORKSTATUS_OKAY;
> +}
> +
> static int
> dt_aggwalk_rval(dtrace_hdl_t *dtp, dt_ahashent_t *h, int rval)
> {
> @@ -1148,6 +1168,8 @@ dt_aggwalk_rval(dtrace_hdl_t *dtp, dt_ahashent_t *h, int rval)
> dt_free(dtp, aggdata->dtada_percpu);
> }
>
> + dt_aggwalk_remove(dtp, h);
> +
> dt_free(dtp, aggdata->dtada_key);
> dt_free(dtp, aggdata->dtada_data);
> dt_free(dtp, h);
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 5391a143..82db9bd7 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2261,11 +2261,6 @@ dt_cg_act_trunc(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> dt_node_t *anp, *trunc;
> dt_ident_t *aid;
> char n[DT_TYPE_NAMELEN];
> - int argc = 0;
> -
> - for (anp = dnp->dn_args; anp != NULL; anp = anp->dn_list)
> - argc++;
> - assert(argc == 1 || argc == 2);
>
> anp = dnp->dn_args;
> assert(anp != NULL);
> @@ -2276,32 +2271,18 @@ dt_cg_act_trunc(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> dnp->dn_ident->di_name,
> dt_node_type_name(anp, n, sizeof(n)));
>
> - trunc = anp->dn_list;
> - if (argc == 2)
> - assert(trunc != NULL && dt_node_is_scalar(trunc));
> -
> aid = anp->dn_ident;
> if (aid->di_gen == pcb->pcb_hdl->dt_gen &&
> !(aid->di_flags & DT_IDFLG_MOD))
> dnerror(dnp, D_TRUNC_AGGBAD,
> "undefined aggregation: @%s\n", aid->di_name);
>
> - /*
> - * FIXME: Needs implementation
> - * TODO: Emit code to truncate the given aggregation.
> - * DEPENDS ON: How aggregations are implemented using eBPF (hashmap?).
> - * AGGID = aid->di_id
> - */
> + trunc = anp->dn_list;
> + if (trunc == NULL)
> + trunc = dt_node_int(0);
> +
> dt_cg_store_val(pcb, anp, DTRACEACT_LIBACT, NULL, DT_ACT_TRUNC);
> -#ifdef FIXME
> - /*
> - * FIXME: There is an optional trunction value.
> - * if (argc == 1), the optional value is missing, but "0" may be
> - * specified.
> - */
> dt_cg_store_val(pcb, trunc, DTRACEACT_LIBACT, NULL, DT_ACT_TRUNC);
> -#endif
> - dnerror(dnp, D_UNKNOWN, "trunc() is not implemented (yet)\n");
> }
>
> static void
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 43a9cea3..a67c0f7b 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -1537,7 +1537,6 @@ dt_clear(dtrace_hdl_t *dtp, caddr_t base, dtrace_recdesc_t *rec)
> return 0;
> }
>
> -#ifdef FIXME
> typedef struct dt_trunc {
> dtrace_aggid_t dttd_id;
> uint64_t dttd_remaining;
> @@ -1594,19 +1593,19 @@ dt_trunc(dtrace_hdl_t *dtp, caddr_t base, dtrace_recdesc_t *rec)
> addr = base + rec->dtrd_offset;
>
> switch (rec->dtrd_size) {
> - case sizeof(uint64_t):
> + case sizeof(int64_t):
> /* LINTED - alignment */
> remaining = *((int64_t *)addr);
> break;
> - case sizeof(uint32_t):
> + case sizeof(int32_t):
> /* LINTED - alignment */
> remaining = *((int32_t *)addr);
> break;
> - case sizeof(uint16_t):
> + case sizeof(int16_t):
> /* LINTED - alignment */
> remaining = *((int16_t *)addr);
> break;
> - case sizeof(uint8_t):
> + case sizeof(int8_t):
> remaining = *((int8_t *)addr);
> break;
> default:
> @@ -1626,7 +1625,6 @@ dt_trunc(dtrace_hdl_t *dtp, caddr_t base, dtrace_recdesc_t *rec)
>
> return 0;
> }
> -#endif
>
> static int
> dt_print_datum(dtrace_hdl_t *dtp, FILE *fp, dtrace_recdesc_t *rec,
> @@ -2344,6 +2342,15 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
> if (dt_clear(dtp, data, rec) != 0)
> return DTRACE_WORKSTATUS_ERROR;
>
> + continue;
> + case DT_ACT_TRUNC:
> + if (i == epd->dtdd_nrecs - 1)
> + return dt_set_errno(dtp, EDT_BADTRUNC);
> +
> + if (dt_trunc(dtp, data, rec) != 0)
> + return DTRACE_WORKSTATUS_ERROR;
> +
> + i++;
> continue;
> case DT_ACT_FTRUNCATE:
> if (fp == NULL)
> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> index b6a0b623..93e8535a 100644
> --- a/libdtrace/dt_open.c
> +++ b/libdtrace/dt_open.c
> @@ -343,7 +343,7 @@ static const dt_ident_t _dtrace_globals[] = {
> DT_ATTR_STABCMN, DT_VERS_1_0,
> &dt_idops_func, "void(@, size_t, [size_t])" },
> { "trunc", DT_IDENT_ACTFUNC, 0, DT_ACT_TRUNC, DT_ATTR_STABCMN,
> - DT_VERS_1_0, &dt_idops_func, "void(@, [uint64_t])" },
> + DT_VERS_1_0, &dt_idops_func, "void(@, [int64_t])" },
> { "uaddr", DT_IDENT_ACTFUNC, 0, DT_ACT_UADDR, DT_ATTR_STABCMN,
> DT_VERS_1_2, &dt_idops_func, "_usymaddr(uintptr_t)" },
> { "ucaller", DT_IDENT_SCALAR, 0, DIF_VAR_UCALLER, DT_ATTR_STABCMN,
> diff --git a/test/unittest/aggs/tst.negtrunc.d b/test/unittest/aggs/tst.negtrunc.d
> index 2c35151b..97db1967 100644
> --- a/test/unittest/aggs/tst.negtrunc.d
> +++ b/test/unittest/aggs/tst.negtrunc.d
> @@ -4,7 +4,6 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 - requires trunc() */
>
> #pragma D option quiet
>
> diff --git a/test/unittest/aggs/tst.trunc.d b/test/unittest/aggs/tst.trunc.d
> index 0c0496a5..dbaeb09a 100644
> --- a/test/unittest/aggs/tst.trunc.d
> +++ b/test/unittest/aggs/tst.trunc.d
> @@ -4,7 +4,6 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 - requires trunc() */
>
> #pragma D option quiet
>
More information about the DTrace-devel
mailing list