[DTrace-devel] [PATCH 2/4] Aggregations are not appropriately reset
eugene.loh at oracle.com
eugene.loh at oracle.com
Wed Jan 6 17:39:11 PST 2021
From: Eugene Loh <eugene.loh at oracle.com>
In DTv1, each user-space aggregation snapshot would read data from the
kernel and aggregate, not only over CPUs but also with previous
snapshots.
In DTv2, the kernel maintains aggregations in BPF maps. Each user-space
snapshot should still aggregate over CPUs, but not with previous data.
Reset aggregations appropriately with each snapshot.
Tweak testing to catch this problem. The test suite will catch this
problem even more once other functionality has been implemented.
https://github.com/oracle/dtrace-utils/issues/4
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
libdtrace/dt_aggregate.c | 50 ++++++++-----
test/unittest/aggs/tst.llquantize_basic.d | 1 -
test/unittest/aggs/tst.reset.d | 38 ++++++++++
test/unittest/aggs/tst.reset.r | 86 +++++++++++++++++++++++
test/unittest/multiaggs/tst.same.d | 1 -
5 files changed, 157 insertions(+), 19 deletions(-)
create mode 100644 test/unittest/aggs/tst.reset.d
create mode 100644 test/unittest/aggs/tst.reset.r
diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
index d59f107e..ba32d9d5 100644
--- a/libdtrace/dt_aggregate.c
+++ b/libdtrace/dt_aggregate.c
@@ -424,16 +424,17 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
uint_t i;
int64_t *src;
int realsz;
+ int firstcpu = (st->cpu == st->dtp->dt_conf.cpus[0].cpu_id);
rval = dt_aggid_lookup(st->dtp, aid->di_id, &agg);
if (rval != 0)
return rval;
- /* check latch sequence number to see if there is any data */
+ /* skip if no data (latch sequence number is zero) */
+ /* (except, if first CPU, still need to clear any previous data) */
src = (int64_t *)(st->buf + aid->di_offset);
- if (*src == 0)
+ if (*src == 0 && !firstcpu)
return 0;
- src++;
/* real size excludes latch sequence number and second data copy */
realsz = (aid->di_size - sizeof(uint64_t)) / 2;
@@ -456,21 +457,31 @@ dt_aggregate_snap_one(dt_idhash_t *dhp, dt_ident_t *aid, dt_snapstate_t *st)
: NULL;
accumulate:
- switch (((dt_ident_t *)aid->di_iarg)->di_id) {
- case DT_AGG_MAX:
- if (*src > *dst)
- *dst = *src;
+ /* skip over latch sequence number to get to the data */
+ src++;
- break;
- case DT_AGG_MIN:
- if (*src < *dst)
- *dst = *src;
+ /* either copy in initial data or combine with existing data */
+ if (dst != (int64_t *)agd->dtada_data || firstcpu) {
+ /* if per-CPU or it's the first CPU, just copy in */
+ memcpy(dst, src, realsz);
+ } else {
+ /* otherwise, combine with existing data */
+ switch (((dt_ident_t *)aid->di_iarg)->di_id) {
+ case DT_AGG_MAX:
+ if (*src > *dst)
+ *dst = *src;
- break;
- default:
- for (i = 0, cnt = realsz / sizeof(int64_t);
- i < cnt; i++, dst++, src++)
- *dst += *src;
+ break;
+ case DT_AGG_MIN:
+ if (*src < *dst)
+ *dst = *src;
+
+ break;
+ default:
+ for (i = 0, cnt = realsz / sizeof(int64_t);
+ i < cnt; i++, dst++, src++)
+ *dst += *src;
+ }
}
/* If we keep per-CPU data - process that as well. */
@@ -484,7 +495,10 @@ accumulate:
return 0;
}
- /* Not found - add it. */
+ /* Not found - add it unless there is no data. */
+ if (*src == 0)
+ return 0;
+
h = dt_zalloc(st->dtp, sizeof(dt_ahashent_t));
if (h == NULL)
return dt_set_errno(st->dtp, EDT_NOMEM);
@@ -496,6 +510,8 @@ accumulate:
return dt_set_errno(st->dtp, EDT_NOMEM);
}
+ /* skip over latch sequence number to get to the data */
+ src++;
memcpy(agd->dtada_data, src, realsz);
agd->dtada_size = realsz;
agd->dtada_desc = agg;
diff --git a/test/unittest/aggs/tst.llquantize_basic.d b/test/unittest/aggs/tst.llquantize_basic.d
index 1aa30680..74d6fe7a 100644
--- a/test/unittest/aggs/tst.llquantize_basic.d
+++ b/test/unittest/aggs/tst.llquantize_basic.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 */
/*
* ASSERTION:
diff --git a/test/unittest/aggs/tst.reset.d b/test/unittest/aggs/tst.reset.d
new file mode 100644
index 00000000..2ab3a78e
--- /dev/null
+++ b/test/unittest/aggs/tst.reset.d
@@ -0,0 +1,38 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2020, 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: Printing aggregations repeatedly should reproduce results
+ *
+ * SECTION: Aggregations/Aggregations
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ i = 10;
+ j = 20;
+ k = 30;
+ l = 40;
+ m = 50;
+ @a = sum(i); @b = stddev(i); @c = count(); @d = avg(i); @e = quantize(i);
+ @a = sum(j); @b = stddev(j); @c = count(); @d = avg(j); @e = quantize(j);
+ @a = sum(k); @b = stddev(k); @c = count(); @d = avg(k); @e = quantize(k);
+ @a = sum(l); @b = stddev(l); @c = count(); @d = avg(l); @e = quantize(l);
+ @a = sum(m); @b = stddev(m); @c = count(); @d = avg(m); @e = quantize(m);
+ exit(0)
+}
+
+END
+{
+ printa(@a); printa(@b); printa(@c); printa(@d); printa(@e);
+ printa(@a); printa(@b); printa(@c); printa(@d); printa(@e);
+ printa(@a); printa(@b); printa(@c); printa(@d); printa(@e);
+ printa(@a); printa(@b); printa(@c); printa(@d); printa(@e);
+ printa(@a); printa(@b); printa(@c); printa(@d); printa(@e);
+}
diff --git a/test/unittest/aggs/tst.reset.r b/test/unittest/aggs/tst.reset.r
new file mode 100644
index 00000000..eb3447a7
--- /dev/null
+++ b/test/unittest/aggs/tst.reset.r
@@ -0,0 +1,86 @@
+
+ 150
+
+ 14
+
+ 5
+
+ 30
+
+
+ value ------------- Distribution ------------- count
+ 4 | 0
+ 8 |@@@@@@@@ 1
+ 16 |@@@@@@@@@@@@@@@@ 2
+ 32 |@@@@@@@@@@@@@@@@ 2
+ 64 | 0
+
+
+ 150
+
+ 14
+
+ 5
+
+ 30
+
+
+ value ------------- Distribution ------------- count
+ 4 | 0
+ 8 |@@@@@@@@ 1
+ 16 |@@@@@@@@@@@@@@@@ 2
+ 32 |@@@@@@@@@@@@@@@@ 2
+ 64 | 0
+
+
+ 150
+
+ 14
+
+ 5
+
+ 30
+
+
+ value ------------- Distribution ------------- count
+ 4 | 0
+ 8 |@@@@@@@@ 1
+ 16 |@@@@@@@@@@@@@@@@ 2
+ 32 |@@@@@@@@@@@@@@@@ 2
+ 64 | 0
+
+
+ 150
+
+ 14
+
+ 5
+
+ 30
+
+
+ value ------------- Distribution ------------- count
+ 4 | 0
+ 8 |@@@@@@@@ 1
+ 16 |@@@@@@@@@@@@@@@@ 2
+ 32 |@@@@@@@@@@@@@@@@ 2
+ 64 | 0
+
+
+ 150
+
+ 14
+
+ 5
+
+ 30
+
+
+ value ------------- Distribution ------------- count
+ 4 | 0
+ 8 |@@@@@@@@ 1
+ 16 |@@@@@@@@@@@@@@@@ 2
+ 32 |@@@@@@@@@@@@@@@@ 2
+ 64 | 0
+
+
diff --git a/test/unittest/multiaggs/tst.same.d b/test/unittest/multiaggs/tst.same.d
index 69121b09..4fd8815d 100644
--- a/test/unittest/multiaggs/tst.same.d
+++ b/test/unittest/multiaggs/tst.same.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 */
#pragma D option quiet
--
2.18.4
More information about the DTrace-devel
mailing list