[PATCH 03/22] Action clear() should clear only one aggregation

eugene.loh
2024-08-29T05:22:27Z

From: Eugene Loh <eugene.loh@oracle.com>

In dt_clear(), we:

*) extract an aggregation ID (dtrace_aggid_t)

*) increment the gen counter for this aggregation ID

*) clear the consumer copy of the aggregation, using:

/* Also clear our own copy of the data, in case it gets printed. */
dtrace_aggregate_walk(dtp, dt_aggregate_clear_one, dtp);

However, this clears all the aggregations. While the next snapshot of
the aggregations will refresh our copies of the aggregations -- after all,
dtrace_aggregate_snap() itself starts with a call to dtrace_aggregate_clear()
-- if we print aggregations before the next snapshot, we would display an
erroneously cleared aggregation.

Therefore, change dt_clear() to clear only those aggregations that
correspond to the specified aggregation ID.

In order that only a specific aggregation ID can be cleared, we
modify the last argument to dt_aggregate_clear_one() to be not
simply the dtp, but a pointer to a struct that has both the dtp
and the aggregation ID. The previous "clear all" behavior can be
specified with an aggregation ID of DTRACE_AGGVARIDNONE.

Though trunc() appears not to be similarly afflicted, add a test for
it too.

Orabug: 36900180
Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
libdtrace/dt_aggregate.c | 16 ++++++++++++---
libdtrace/dt_aggregate.h | 5 +++++
libdtrace/dt_consume.c | 4 +++-
test/unittest/aggs/tst.clear-one.d | 33 ++++++++++++++++++++++++++++++
test/unittest/aggs/tst.clear-one.r | 13 ++++++++++++
test/unittest/aggs/tst.trunc-one.d | 33 ++++++++++++++++++++++++++++++
test/unittest/aggs/tst.trunc-one.r | 10 +++++++++
7 files changed, 110 insertions(+), 4 deletions(-)
create mode 100644 test/unittest/aggs/tst.clear-one.d
create mode 100644 test/unittest/aggs/tst.clear-one.r
create mode 100644 test/unittest/aggs/tst.trunc-one.d
create mode 100644 test/unittest/aggs/tst.trunc-one.r

diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
index 329b8ffa..8825739c 100644
--- a/libdtrace/dt_aggregate.c
+++ b/libdtrace/dt_aggregate.c
@@ -504,7 +504,9 @@ dt_aggregate_clear_one_percpu(const dtrace_aggdata_t *agd,
int
dt_aggregate_clear_one(const dtrace_aggdata_t *agd, void *arg)
{
- dtrace_hdl_t *dtp = arg;
+ dt_clear_arg_t *dca = arg;
+ dtrace_hdl_t *dtp = dca->dtp;
+ dtrace_aggid_t aid = dca->aid;
dtrace_aggdesc_t *agg = agd->dtada_desc;
dtrace_recdesc_t *rec = &agg->dtagd_drecs[DT_AGGDATA_RECORD];
int64_t *vals = (int64_t *)
@@ -512,6 +514,9 @@ dt_aggregate_clear_one(const dtrace_aggdata_t *agd, void *arg)
uint64_t agen;
int max_cpus = dtp->dt_conf.max_cpuid + 1;

+ if (aid != DTRACE_AGGVARIDNONE && aid != agg->dtagd_varid)
+ return DTRACE_AGGWALK_NEXT;
+
/*
* We can pass the entire key because we know that the first uint32_t
* in the key is the aggregation ID we need.
@@ -609,7 +614,9 @@ dt_aggregate_snap_one(dtrace_hdl_t *dtp, int aggid, int cpu, const char *key,
*/
hgen = *(int64_t *)agd->dtada_data;
if (dgen > hgen) {
- dt_aggregate_clear_one(agd, dtp);
+ dt_clear_arg_t arg = { dtp, DTRACE_AGGVARIDNONE };
+
+ dt_aggregate_clear_one(agd, &arg);
hgen = *(int64_t *)agd->dtada_data;
}

@@ -1859,10 +1866,13 @@ dtrace_aggregate_print(dtrace_hdl_t *dtp, FILE *fp,
return 0;
}

+/* FIXME: dtrace_aggregate_clear() is called from only one site. Should we inline it? */
void
dtrace_aggregate_clear(dtrace_hdl_t *dtp)
{
- dtrace_aggregate_walk(dtp, dt_aggregate_clear_one, dtp);
+ dt_clear_arg_t arg = { dtp, DTRACE_AGGVARIDNONE };
+
+ dtrace_aggregate_walk(dtp, dt_aggregate_clear_one, &arg);
}

void
diff --git a/libdtrace/dt_aggregate.h b/libdtrace/dt_aggregate.h
index 4f8d09bf..0dc126e9 100644
--- a/libdtrace/dt_aggregate.h
+++ b/libdtrace/dt_aggregate.h
@@ -12,6 +12,11 @@
extern "C" {
#endif

+typedef struct dt_clear_arg {
+ dtrace_hdl_t *dtp;
+ dtrace_aggid_t aid;
+} dt_clear_arg_t;
+
typedef struct dt_aggregate dt_aggregate_t;

#define DT_AGGDATA_COUNTER 0
diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
index 0e30ddbd..f2d5cb74 100644
--- a/libdtrace/dt_consume.c
+++ b/libdtrace/dt_consume.c
@@ -1552,6 +1552,7 @@ dt_clear(dtrace_hdl_t *dtp, caddr_t base, dtrace_recdesc_t *rec)
dtrace_aggid_t aid;
uint64_t gen;
caddr_t addr;
+ dt_clear_arg_t arg = { dtp, 0 };

/* We have just one record: the aggregation ID. */
addr = base + rec->dtrd_offset;
@@ -1568,7 +1569,8 @@ dt_clear(dtrace_hdl_t *dtp, caddr_t base, dtrace_recdesc_t *rec)
return -1;

/* Also clear our own copy of the data, in case it gets printed. */
- dtrace_aggregate_walk(dtp, dt_aggregate_clear_one, dtp);
+ arg.aid = aid;
+ dtrace_aggregate_walk(dtp, dt_aggregate_clear_one, &arg);

return 0;
}
diff --git a/test/unittest/aggs/tst.clear-one.d b/test/unittest/aggs/tst.clear-one.d
new file mode 100644
index 00000000..f96c2db0
--- /dev/null
+++ b/test/unittest/aggs/tst.clear-one.d
@@ -0,0 +1,33 @@
+/*
+ * 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.
+ */
+
+/*
+ * ASSERTION: Clearing one aggregation clears only that one.
+ *
+ * SECTION: Aggregations/Clearing aggregations
+ */
+/* @@nosort */
+
+#pragma D option quiet
+
+BEGIN
+{
+ @a[1] = sum(100);
+ @a[2] = sum( 20);
+ @a[3] = sum( 3);
+ @b[4] = sum(400);
+ @b[5] = sum( 50);
+ @b[6] = sum( 6);
+ @c[7] = sum(700);
+ @c[8] = sum( 80);
+ @c[9] = sum( 9);
+ clear(@b);
+ printa(@a);
+ printa(@b);
+ printa(@c);
+ exit(0);
+}
diff --git a/test/unittest/aggs/tst.clear-one.r b/test/unittest/aggs/tst.clear-one.r
new file mode 100644
index 00000000..101d173f
--- /dev/null
+++ b/test/unittest/aggs/tst.clear-one.r
@@ -0,0 +1,13 @@
+
+ 3 3
+ 2 20
+ 1 100
+
+ 4 0
+ 5 0
+ 6 0
+
+ 9 9
+ 8 80
+ 7 700
+
diff --git a/test/unittest/aggs/tst.trunc-one.d b/test/unittest/aggs/tst.trunc-one.d
new file mode 100644
index 00000000..0556bb34
--- /dev/null
+++ b/test/unittest/aggs/tst.trunc-one.d
@@ -0,0 +1,33 @@
+/*
+ * 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.
+ */
+
+/*
+ * ASSERTION: Truncating one aggregation truncates only that one.
+ *
+ * SECTION: Aggregations/Clearing aggregations
+ */
+/* @@nosort */
+
+#pragma D option quiet
+
+BEGIN
+{
+ @a[1] = sum(100);
+ @a[2] = sum( 20);
+ @a[3] = sum( 3);
+ @b[4] = sum(400);
+ @b[5] = sum( 50);
+ @b[6] = sum( 6);
+ @c[7] = sum(700);
+ @c[8] = sum( 80);
+ @c[9] = sum( 9);
+ trunc(@b);
+ printa(@a);
+ printa(@b);
+ printa(@c);
+ exit(0);
+}
diff --git a/test/unittest/aggs/tst.trunc-one.r b/test/unittest/aggs/tst.trunc-one.r
new file mode 100644
index 00000000..a611026d
--- /dev/null
+++ b/test/unittest/aggs/tst.trunc-one.r
@@ -0,0 +1,10 @@
+
+ 3 3
+ 2 20
+ 1 100
+
+
+ 9 9
+ 8 80
+ 7 700
+
--
2.43.5