[DTrace-devel] [PATCH] Action clear() should clear only one aggregation

eugene.loh at oracle.com eugene.loh at oracle.com
Wed Aug 14 02:59:25 UTC 2024


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

FIXME:  Is libdtrace/libdtrace.ver correctly updated?

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.

Note that while we have "aggregation-walk" functions like

    dtrace_aggregate_walk()
    dtrace_aggregate_walk_joined()
    dtrace_aggregate_walk_*sorted() (many of these)

the first one (dtrace_aggregate_walk) is used only twice:

    - in dtrace_aggregate_clear() to clear all aggregations

    - in dt_clear(), where we should clear only a specific aggregation ID

Therefore, rename dtrace_aggregate_walk() to be
dtrace_aggregate_walk_oneid() that takes an aggid argument.  If
aggid==DTRACE_AGGVARIDNONE, walk all aggregations.  Otherwise, walk
only those aggregations that match the specified id.

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

Orabug: 36900180
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 libdtrace/dt_aggregate.c           |  8 ++++++--
 libdtrace/dt_consume.c             |  2 +-
 libdtrace/dtrace.h                 |  4 ++--
 libdtrace/libdtrace.ver            |  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 +++++++++
 8 files changed, 100 insertions(+), 7 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 51f0d622..e06025de 100644
--- a/libdtrace/dt_aggregate.c
+++ b/libdtrace/dt_aggregate.c
@@ -1278,7 +1278,8 @@ dt_aggregate_qsort(dtrace_hdl_t *dtp, void *base, size_t nel, size_t width,
 }
 
 int
-dtrace_aggregate_walk(dtrace_hdl_t *dtp, dtrace_aggregate_f *func, void *arg)
+dtrace_aggregate_walk_oneid(dtrace_hdl_t *dtp, dtrace_aggregate_f *func,
+			    void *arg, dtrace_aggid_t aid)
 {
 	dt_ahashent_t *h, *next;
 	dt_ahash_t *hash = &dtp->dt_aggregate.dtat_hash;
@@ -1291,6 +1292,8 @@ dtrace_aggregate_walk(dtrace_hdl_t *dtp, dtrace_aggregate_f *func, void *arg)
 		 */
 		next = h->dtahe_nextall;
 
+		if (aid != DTRACE_AGGVARIDNONE && aid != h->dtahe_data.dtada_desc->dtagd_varid)
+			continue;
 		if (dt_aggwalk_rval(dtp, h, func(&h->dtahe_data, arg)) == -1)
 			return -1;
 	}
@@ -1820,10 +1823,11 @@ 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);
+	dtrace_aggregate_walk_oneid(dtp, dt_aggregate_clear_one, dtp, DTRACE_AGGVARIDNONE);
 }
 
 void
diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
index 8d8c0f54..238b93ca 100644
--- a/libdtrace/dt_consume.c
+++ b/libdtrace/dt_consume.c
@@ -1551,7 +1551,7 @@ 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);
+	dtrace_aggregate_walk_oneid(dtp, dt_aggregate_clear_one, dtp, aid);
 
 	return 0;
 }
diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
index 1f54c10c..4df3ebf4 100644
--- a/libdtrace/dtrace.h
+++ b/libdtrace/dtrace.h
@@ -396,8 +396,8 @@ extern int dtrace_aggregate_snap(dtrace_hdl_t *dtp);
 extern int dtrace_aggregate_print(dtrace_hdl_t *dtp, FILE *fp,
     dtrace_aggregate_walk_f *func); /* func must take dtrace_print_aggdata_t */
 
-extern int dtrace_aggregate_walk(dtrace_hdl_t *dtp, dtrace_aggregate_f *func,
-    void *arg);
+extern int dtrace_aggregate_walk_oneid(dtrace_hdl_t *dtp, dtrace_aggregate_f *func,
+    void *arg, dtrace_aggid_t);
 
 extern int dtrace_aggregate_walk_joined(dtrace_hdl_t *dtp,
     dtrace_aggid_t *aggvars, int naggvars,
diff --git a/libdtrace/libdtrace.ver b/libdtrace/libdtrace.ver
index edbd5a1d..1510ad95 100644
--- a/libdtrace/libdtrace.ver
+++ b/libdtrace/libdtrace.ver
@@ -1,6 +1,6 @@
 #
 # Oracle Linux DTrace.
-# Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2009, 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.
 
@@ -10,7 +10,7 @@ LIBDTRACE_1.0 {
 	dtrace_aggregate_clear;
 	dtrace_aggregate_print;
 	dtrace_aggregate_snap;
-	dtrace_aggregate_walk;
+	dtrace_aggregate_walk_oneid;
 	dtrace_aggregate_walk_joined;
 	dtrace_aggregate_walk_keyrevsorted;
 	dtrace_aggregate_walk_keysorted;
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




More information about the DTrace-devel mailing list