[PATCH 1/8] test: Allow aggpercpu test to omit some CPUs

[PATCH 1/8] test: Allow aggpercpu test to omit some CPUs

[eugene.loh]

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

The aggpercpu test uses profile-* to fire aggregations on every CPU.
But whether for good (offline CPUs) or bad (unreliable probe) reasons,
a CPU might not aggregate any values.

Allow this test to skip over such CPUs.  That is, while a BPF agg map
might have the correct default value for the aggregation function, it
may not see any anticipated data.  The quality of profile-* probes
should be checked by a different test.

Also, when computing avg() and stddev() in the awk check program,
truncate results to ints more often to mimic the algorithms in DTrace
for more robust checks.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/unittest/aggs/tst.aggpercpu.sh | 62 +++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 17 deletions(-)

diff --git a/test/unittest/aggs/tst.aggpercpu.sh b/test/unittest/aggs/tst.aggpercpu.sh
index be74890a..6092bd17 100755
--- a/test/unittest/aggs/tst.aggpercpu.sh
+++ b/test/unittest/aggs/tst.aggpercpu.sh
@@ -46,7 +46,8 @@ fi
 awk '
     # The expected value for the aggregation is aggval.
     # The expected value on a CPU is (m * cpu + b).
-    function check(label, aggval, m, b) {
+    # The default value on a CPU that did not fire is defval.
+    function check(label, aggval, m, b, defval) {
         # Check the aggregation over all CPUs.
         getline;
         print "check:", $0;
@@ -54,12 +55,38 @@ awk '
 
         # Check the per-CPU values.
         for (i = 1; i <= ncpu; i++) {
-            getline;
-            print "check:", $0;
-            if (match($0, "^    \\[CPU ") != 1 ||
-                strtonum($2) != cpu[i] ||
-                strtonum($3) != m * cpu[i] + b)
-                printf("ERROR: %s, agg per cpu %d, line: %s\n", label, cpu[i], $0);
+            do {
+                getline;
+                print "check:", $0;
+
+                # pass is +1 for pass, -1 for error, 0 for skip CPU
+                pass = 0;
+
+                if (match($0, "^    \\[CPU ") != 1) {
+                    print "ERROR: no CPU to read"
+                    pass = -1;
+                } else if (strtonum($2) > cpu[i]) {
+                    print "ERROR: skipped over expected CPU"
+                    pass = -1;
+                } else if (strtonum($2) == cpu[i]) {
+                    if (strtonum($3) != m * cpu[i] + b) {
+                        print "ERROR: wrong value"
+                        pass = -1;
+                    } else {
+                        # right value
+                        pass = +1;
+                    }
+                } else if ($3 != defval) {
+                    print "ERROR: wrong default value"
+                    pass = -1;
+                } else {
+                    # skip over CPU that apparently did not fire
+                    pass = 0;
+                }
+
+                if (pass == -1)
+                    printf("ERROR: %s, agg per cpu %d, line: %s\n", label, cpu[i], $0);
+            } while (pass == 0);
         }
     }
 
@@ -99,13 +126,14 @@ awk '
     {
         # First we finish computing our estimates for avg and stddev.
         # (The other results require no further action.)
+        # (We keep truncating to ints to mimic DTrace algorithms.)
 
-        xavg /= xcnt;
+        xavg /= xcnt;             xavg = int(xavg);
 
-        xstm /= xcnt;
-        xstd /= xcnt;
+        xstm /= xcnt;             xstm = int(xstm);
+        xstd /= xcnt;             xstd = int(xstd);
         xstd -= xstm * xstm;
-        xstd = int(sqrt(xstd));
+        xstd = sqrt(xstd);        xstd = int(xstd);
 
         # Sort the cpus.
 
@@ -113,12 +141,12 @@ awk '
 
         # Now read the results and compare.
 
-        check("cnt", xcnt,  0,   1);
-        check("avg", xavg, 10,   3);
-        check("std", xstd,  0,   0);
-        check("min", xmin, 30, -10);
-        check("max", xmax, 40, -15);
-        check("sum", xsum, 50,   0);
+        check("cnt", xcnt,  0,   1, "0");
+        check("avg", xavg, 10,   3, "0");
+        check("std", xstd,  0,   0, "0");
+        check("min", xmin, 30, -10,  "9223372036854775807");
+        check("max", xmax, 40, -15, "-9223372036854775808");
+        check("sum", xsum, 50,   0, "0");
 
         printf("done\n");
     }
-- 
2.18.4

[PATCH 2/8] Reduce stack depth if kernel returns NULL frames

[eugene.loh]

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

The return value from the BPF helper function bpf_get_stack()
basically returns the size of the stack returned.  We use this
value to report stack depth.

Some of the top frames can be NULL, however, leading to some
inconsistencies between reported stacks and stack depths.

Add some code to reduce the stack depth if one or two top
frames are NULL.

There is an existing test to check for this problem.  It will
appear in a later patch since it has multiple problems.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 bpf/get_bvar.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
index ea5dc6b1..a0c04f3a 100644
--- a/bpf/get_bvar.c
+++ b/bpf/get_bvar.c
@@ -67,7 +67,9 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
 		uint32_t bufsiz = (uint32_t) (uint64_t) (&STKSIZ);
 		uint64_t flags;
 		char *buf = dctx->mem + (uint64_t)(&STACK_OFF);
-		uint64_t stacksize;
+		int64_t stacksize;
+		int64_t topslot;
+		uint64_t *pcs = (uint64_t *)buf;
 
 		if (id == DIF_VAR_USTACKDEPTH)
 			flags = BPF_F_USER_STACK;
@@ -87,8 +89,19 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
 		 * what we can retrieve.  But it's also possible that the
 		 * buffer was exactly large enough.  So, leave it to the user
 		 * to interpret the result.
+		 *
+		 * The helper function also sometimes returns some empty frames
+		 * at the top.  Bump the depth down some so that the stack depth
+		 * we report is consistent with the number of frames returned.
+		 * Arguably, this should be fixed in the kernel, but we can
+		 * work around the problem for now.
 		 */
-		return stacksize / sizeof(uint64_t);
+		topslot = stacksize / sizeof(uint64_t) - 1;
+		if (topslot >= 0 && topslot < (bufsiz / sizeof(uint64_t)) && pcs[topslot] == 0)
+			topslot--;
+		if (topslot >= 0 && topslot < (bufsiz / sizeof(uint64_t)) && pcs[topslot] == 0)
+			topslot--;
+		return topslot + 1;
 	}
 	case DIF_VAR_CALLER:
 	case DIF_VAR_UCALLER: {
-- 
2.18.4

Re: [DTrace-devel] [PATCH 2/8] Reduce stack depth if kernel returns NULL frames

[Kris Van Hees]

If bpf_get_stack() can give us stacks with NULL pointers at the top, wouldn't
we need code to remove those (if they need to be removed) from the actual
stack() data also?  If they are left there, then I would argue that we should
also include them in the stackdepth count.

On Tue, Jun 04, 2024 at 02:00:02PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> The return value from the BPF helper function bpf_get_stack()
> basically returns the size of the stack returned.  We use this
> value to report stack depth.
> 
> Some of the top frames can be NULL, however, leading to some
> inconsistencies between reported stacks and stack depths.
> 
> Add some code to reduce the stack depth if one or two top
> frames are NULL.
> 
> There is an existing test to check for this problem.  It will
> appear in a later patch since it has multiple problems.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  bpf/get_bvar.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
> index ea5dc6b1..a0c04f3a 100644
> --- a/bpf/get_bvar.c
> +++ b/bpf/get_bvar.c
> @@ -67,7 +67,9 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
>  		uint32_t bufsiz = (uint32_t) (uint64_t) (&STKSIZ);
>  		uint64_t flags;
>  		char *buf = dctx->mem + (uint64_t)(&STACK_OFF);
> -		uint64_t stacksize;
> +		int64_t stacksize;
> +		int64_t topslot;
> +		uint64_t *pcs = (uint64_t *)buf;
>  
>  		if (id == DIF_VAR_USTACKDEPTH)
>  			flags = BPF_F_USER_STACK;
> @@ -87,8 +89,19 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
>  		 * what we can retrieve.  But it's also possible that the
>  		 * buffer was exactly large enough.  So, leave it to the user
>  		 * to interpret the result.
> +		 *
> +		 * The helper function also sometimes returns some empty frames
> +		 * at the top.  Bump the depth down some so that the stack depth
> +		 * we report is consistent with the number of frames returned.
> +		 * Arguably, this should be fixed in the kernel, but we can
> +		 * work around the problem for now.
>  		 */
> -		return stacksize / sizeof(uint64_t);
> +		topslot = stacksize / sizeof(uint64_t) - 1;
> +		if (topslot >= 0 && topslot < (bufsiz / sizeof(uint64_t)) && pcs[topslot] == 0)
> +			topslot--;
> +		if (topslot >= 0 && topslot < (bufsiz / sizeof(uint64_t)) && pcs[topslot] == 0)
> +			topslot--;
> +		return topslot + 1;
>  	}
>  	case DIF_VAR_CALLER:
>  	case DIF_VAR_UCALLER: {
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

Re: [DTrace-devel] [PATCH 2/8] Reduce stack depth if kernel returns NULL frames

[Eugene Loh]

It's been a while, but if I remember correctly it's actually just the 
other way around.  That is, to make stack and depth consistent, we're 
forced into this "depth reduction" patch.  Put another way, the stack 
simply does not include NULL pointers -- there is no way to remove them.

E.g., let's say we have a buffer of 8 pointers and we ask for the stack 
and get back:

     0xdead 0xbeef 0xfeed 0xface NULL NULL NULL NULL

Looks like 4 pointers.  But we don't count them.  We just use the return 
value from the helper function.  If it tells us "4", then everything is 
consistent.  But for some reason (I haven't looked at the code to figure 
out why), it can be 5 or even 6.  So this patch bumps that value down -- 
to attain the consistency I think you're asking about.

There is a test that exposes the problem (inconsistency between stack 
and depth) and this patch's fix.  Again, the stack already does not have 
NULL pointers, and so it's simply a matter of patching up the depth.  
Unfortunately, the test is riddled with all sorts of problems and I 
haven't yet gotten around to cleaning it up to the point where I can put 
it back.

On 8/19/24 19:30, Kris Van Hees wrote:
> If bpf_get_stack() can give us stacks with NULL pointers at the top, wouldn't
> we need code to remove those (if they need to be removed) from the actual
> stack() data also?  If they are left there, then I would argue that we should
> also include them in the stackdepth count.
>
> On Tue, Jun 04, 2024 at 02:00:02PM -0400, eugene.loh--- via DTrace-devel wrote:
>> From: Eugene Loh <eugene.loh@oracle.com>
>>
>> The return value from the BPF helper function bpf_get_stack()
>> basically returns the size of the stack returned.  We use this
>> value to report stack depth.
>>
>> Some of the top frames can be NULL, however, leading to some
>> inconsistencies between reported stacks and stack depths.
>>
>> Add some code to reduce the stack depth if one or two top
>> frames are NULL.
>>
>> There is an existing test to check for this problem.  It will
>> appear in a later patch since it has multiple problems.
>>
>> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
>> ---
>>   bpf/get_bvar.c | 17 +++++++++++++++--
>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
>> index ea5dc6b1..a0c04f3a 100644
>> --- a/bpf/get_bvar.c
>> +++ b/bpf/get_bvar.c
>> @@ -67,7 +67,9 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
>>   		uint32_t bufsiz = (uint32_t) (uint64_t) (&STKSIZ);
>>   		uint64_t flags;
>>   		char *buf = dctx->mem + (uint64_t)(&STACK_OFF);
>> -		uint64_t stacksize;
>> +		int64_t stacksize;
>> +		int64_t topslot;
>> +		uint64_t *pcs = (uint64_t *)buf;
>>   
>>   		if (id == DIF_VAR_USTACKDEPTH)
>>   			flags = BPF_F_USER_STACK;
>> @@ -87,8 +89,19 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
>>   		 * what we can retrieve.  But it's also possible that the
>>   		 * buffer was exactly large enough.  So, leave it to the user
>>   		 * to interpret the result.
>> +		 *
>> +		 * The helper function also sometimes returns some empty frames
>> +		 * at the top.  Bump the depth down some so that the stack depth
>> +		 * we report is consistent with the number of frames returned.
>> +		 * Arguably, this should be fixed in the kernel, but we can
>> +		 * work around the problem for now.
>>   		 */
>> -		return stacksize / sizeof(uint64_t);
>> +		topslot = stacksize / sizeof(uint64_t) - 1;
>> +		if (topslot >= 0 && topslot < (bufsiz / sizeof(uint64_t)) && pcs[topslot] == 0)
>> +			topslot--;
>> +		if (topslot >= 0 && topslot < (bufsiz / sizeof(uint64_t)) && pcs[topslot] == 0)
>> +			topslot--;
>> +		return topslot + 1;
>>   	}
>>   	case DIF_VAR_CALLER:
>>   	case DIF_VAR_UCALLER: {
>> -- 
>> 2.18.4
>>
>>
>> _______________________________________________
>> DTrace-devel mailing list
>> DTrace-devel@oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/dtrace-devel

Re: [DTrace-devel] [PATCH 2/8] Reduce stack depth if kernel returns NULL frames

[Kris Van Hees]

On Wed, Aug 28, 2024 at 04:11:29PM -0400, Eugene Loh wrote:
> It's been a while, but if I remember correctly it's actually just the other
> way around.  That is, to make stack and depth consistent, we're forced into
> this "depth reduction" patch.  Put another way, the stack simply does not
> include NULL pointers -- there is no way to remove them.
> 
> E.g., let's say we have a buffer of 8 pointers and we ask for the stack and
> get back:
> 
>     0xdead 0xbeef 0xfeed 0xface NULL NULL NULL NULL
> 
> Looks like 4 pointers.  But we don't count them.  We just use the return
> value from the helper function.  If it tells us "4", then everything is
> consistent.  But for some reason (I haven't looked at the code to figure out
> why), it can be 5 or even 6.  So this patch bumps that value down -- to
> attain the consistency I think you're asking about.

So you are saying that the number of values that is actually filled in is not
consistent with the return value of the bpf_get_stack() helper?  That would
sound like a kernel bug.

Re: [DTrace-devel] [PATCH 2/8] Reduce stack depth if kernel returns NULL frames

[Eugene Loh]

On 8/28/24 16:17, Kris Van Hees wrote:

> On Wed, Aug 28, 2024 at 04:11:29PM -0400, Eugene Loh wrote:
>> It's been a while, but if I remember correctly it's actually just the other
>> way around.  That is, to make stack and depth consistent, we're forced into
>> this "depth reduction" patch.  Put another way, the stack simply does not
>> include NULL pointers -- there is no way to remove them.
>>
>> E.g., let's say we have a buffer of 8 pointers and we ask for the stack and
>> get back:
>>
>>      0xdead 0xbeef 0xfeed 0xface NULL NULL NULL NULL
>>
>> Looks like 4 pointers.  But we don't count them.  We just use the return
>> value from the helper function.  If it tells us "4", then everything is
>> consistent.  But for some reason (I haven't looked at the code to figure out
>> why), it can be 5 or even 6.  So this patch bumps that value down -- to
>> attain the consistency I think you're asking about.
> So you are saying that the number of values that is actually filled in is not
> consistent with the return value of the bpf_get_stack() helper?  That would
> sound like a kernel bug.

I think there might be a kernel bug.  For a variety of practical reasons, I went with a DTrace workaround (and haven't looked at the kernel code).

Re: [DTrace-devel] [PATCH 2/8] Reduce stack depth if kernel returns NULL frames

[Kris Van Hees]

On Wed, Aug 28, 2024 at 04:23:09PM -0400, Eugene Loh via DTrace-devel wrote:
> On 8/28/24 16:17, Kris Van Hees wrote:
> 
> > On Wed, Aug 28, 2024 at 04:11:29PM -0400, Eugene Loh wrote:
> > > It's been a while, but if I remember correctly it's actually just the other
> > > way around.  That is, to make stack and depth consistent, we're forced into
> > > this "depth reduction" patch.  Put another way, the stack simply does not
> > > include NULL pointers -- there is no way to remove them.
> > > 
> > > E.g., let's say we have a buffer of 8 pointers and we ask for the stack and
> > > get back:
> > > 
> > >      0xdead 0xbeef 0xfeed 0xface NULL NULL NULL NULL
> > > 
> > > Looks like 4 pointers.  But we don't count them.  We just use the return
> > > value from the helper function.  If it tells us "4", then everything is
> > > consistent.  But for some reason (I haven't looked at the code to figure out
> > > why), it can be 5 or even 6.  So this patch bumps that value down -- to
> > > attain the consistency I think you're asking about.
> > So you are saying that the number of values that is actually filled in is not
> > consistent with the return value of the bpf_get_stack() helper?  That would
> > sound like a kernel bug.
> 
> I think there might be a kernel bug.  For a variety of practical reasons, I went with a DTrace workaround (and haven't looked at the kernel code).

I'll look around to see if I can figure out what is going on and whether it is
a kernel bug (which of course may already have been fixed in some kernel
version).  In general, we should mention in patches like this that it seems to
be a kernel bug (or first determine if it is) because otherwise we're going
to implement workarounds without ever making sure the kernel bug gets fixed.
Or if it is not a kernel bug, we need to look elsewhere why this goes wrong.

Re: [DTrace-devel] [PATCH 2/8] Reduce stack depth if kernel returns NULL frames

[Eugene Loh]

I cannot remember which test this patch was trying to fix and recent 
test results do not suggest any obvious candidates.  Under the 
circumstances, I'm fine rescinding this patch.  Arguably it was just a 
workaround for a kernel bug anyhow, and perhaps the underlying kernel 
bug has been fixed.

On 8/28/24 16:37, Kris Van Hees wrote:
> On Wed, Aug 28, 2024 at 04:23:09PM -0400, Eugene Loh via DTrace-devel wrote:
>> On 8/28/24 16:17, Kris Van Hees wrote:
>>
>>> On Wed, Aug 28, 2024 at 04:11:29PM -0400, Eugene Loh wrote:
>>>> It's been a while, but if I remember correctly it's actually just the other
>>>> way around.  That is, to make stack and depth consistent, we're forced into
>>>> this "depth reduction" patch.  Put another way, the stack simply does not
>>>> include NULL pointers -- there is no way to remove them.
>>>>
>>>> E.g., let's say we have a buffer of 8 pointers and we ask for the stack and
>>>> get back:
>>>>
>>>>       0xdead 0xbeef 0xfeed 0xface NULL NULL NULL NULL
>>>>
>>>> Looks like 4 pointers.  But we don't count them.  We just use the return
>>>> value from the helper function.  If it tells us "4", then everything is
>>>> consistent.  But for some reason (I haven't looked at the code to figure out
>>>> why), it can be 5 or even 6.  So this patch bumps that value down -- to
>>>> attain the consistency I think you're asking about.
>>> So you are saying that the number of values that is actually filled in is not
>>> consistent with the return value of the bpf_get_stack() helper?  That would
>>> sound like a kernel bug.
>> I think there might be a kernel bug.  For a variety of practical reasons, I went with a DTrace workaround (and haven't looked at the kernel code).
> I'll look around to see if I can figure out what is going on and whether it is
> a kernel bug (which of course may already have been fixed in some kernel
> version).  In general, we should mention in patches like this that it seems to
> be a kernel bug (or first determine if it is) because otherwise we're going
> to implement workarounds without ever making sure the kernel bug gets fixed.
> Or if it is not a kernel bug, we need to look elsewhere why this goes wrong.

[PATCH 3/8] test: Fix nonexistent @@sort option

[eugene.loh]

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

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/unittest/options/tst.aggrate-fast.d | 4 ++--
 test/unittest/options/tst.aggrate-fast.r | 2 +-
 test/unittest/options/tst.aggrate-slow.d | 4 ++--
 test/unittest/options/tst.aggrate-slow.r | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/unittest/options/tst.aggrate-fast.d b/test/unittest/options/tst.aggrate-fast.d
index 17fe0225..68c46a1d 100644
--- a/test/unittest/options/tst.aggrate-fast.d
+++ b/test/unittest/options/tst.aggrate-fast.d
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 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.
  */
@@ -11,7 +11,7 @@
  */
 
 /* @@trigger: periodic_output */
-/* @@sort */
+/* @@nosort */
 
 #pragma D option quiet
 #pragma D option switchrate=100ms
diff --git a/test/unittest/options/tst.aggrate-fast.r b/test/unittest/options/tst.aggrate-fast.r
index a32cf7df..c47c6a20 100644
--- a/test/unittest/options/tst.aggrate-fast.r
+++ b/test/unittest/options/tst.aggrate-fast.r
@@ -1,3 +1,4 @@
+
                 1
 
                 2
@@ -14,4 +15,3 @@
 
                 8
 
-
diff --git a/test/unittest/options/tst.aggrate-slow.d b/test/unittest/options/tst.aggrate-slow.d
index c4bac5e4..e2a0f2cb 100644
--- a/test/unittest/options/tst.aggrate-slow.d
+++ b/test/unittest/options/tst.aggrate-slow.d
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 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 @@
  * actions, multiple printa() should all reflect the same stale count.
  */
 /* @@trigger: periodic_output */
-/* @@sort */
+/* @@nosort */
 
 #pragma D option quiet
 #pragma D option switchrate=100ms
diff --git a/test/unittest/options/tst.aggrate-slow.r b/test/unittest/options/tst.aggrate-slow.r
index 6cbd6c2f..ba2e979f 100644
--- a/test/unittest/options/tst.aggrate-slow.r
+++ b/test/unittest/options/tst.aggrate-slow.r
@@ -1,3 +1,4 @@
+
                 1
 
                 1
@@ -14,4 +15,3 @@
 
                 5
 
-
-- 
2.18.4

Re: [DTrace-devel] [PATCH 3/8] test: Fix nonexistent @@sort option

[Kris Van Hees]

On Tue, Jun 04, 2024 at 02:00:03PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  test/unittest/options/tst.aggrate-fast.d | 4 ++--
>  test/unittest/options/tst.aggrate-fast.r | 2 +-
>  test/unittest/options/tst.aggrate-slow.d | 4 ++--
>  test/unittest/options/tst.aggrate-slow.r | 2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/test/unittest/options/tst.aggrate-fast.d b/test/unittest/options/tst.aggrate-fast.d
> index 17fe0225..68c46a1d 100644
> --- a/test/unittest/options/tst.aggrate-fast.d
> +++ b/test/unittest/options/tst.aggrate-fast.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2023, 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.
>   */
> @@ -11,7 +11,7 @@
>   */
>  
>  /* @@trigger: periodic_output */
> -/* @@sort */
> +/* @@nosort */
>  
>  #pragma D option quiet
>  #pragma D option switchrate=100ms
> diff --git a/test/unittest/options/tst.aggrate-fast.r b/test/unittest/options/tst.aggrate-fast.r
> index a32cf7df..c47c6a20 100644
> --- a/test/unittest/options/tst.aggrate-fast.r
> +++ b/test/unittest/options/tst.aggrate-fast.r
> @@ -1,3 +1,4 @@
> +
>                  1
>  
>                  2
> @@ -14,4 +15,3 @@
>  
>                  8
>  
> -
> diff --git a/test/unittest/options/tst.aggrate-slow.d b/test/unittest/options/tst.aggrate-slow.d
> index c4bac5e4..e2a0f2cb 100644
> --- a/test/unittest/options/tst.aggrate-slow.d
> +++ b/test/unittest/options/tst.aggrate-slow.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2023, 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 @@
>   * actions, multiple printa() should all reflect the same stale count.
>   */
>  /* @@trigger: periodic_output */
> -/* @@sort */
> +/* @@nosort */
>  
>  #pragma D option quiet
>  #pragma D option switchrate=100ms
> diff --git a/test/unittest/options/tst.aggrate-slow.r b/test/unittest/options/tst.aggrate-slow.r
> index 6cbd6c2f..ba2e979f 100644
> --- a/test/unittest/options/tst.aggrate-slow.r
> +++ b/test/unittest/options/tst.aggrate-slow.r
> @@ -1,3 +1,4 @@
> +
>                  1
>  
>                  1
> @@ -14,4 +15,3 @@
>  
>                  5
>  
> -
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

[PATCH 4/8] test: Remove unneeded -w option

[eugene.loh]

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

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/unittest/usdt/tst.dlclose2.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/unittest/usdt/tst.dlclose2.sh b/test/unittest/usdt/tst.dlclose2.sh
index 94be773d..6044cf4b 100755
--- a/test/unittest/usdt/tst.dlclose2.sh
+++ b/test/unittest/usdt/tst.dlclose2.sh
@@ -128,7 +128,7 @@ if [ $? -ne 0 ]; then
 fi
 
 script() {
-	$dtrace -w -c ./main -Zqs /dev/stdin <<EOF
+	$dtrace -c ./main -Zqs /dev/stdin <<EOF
 	test_prov*:::
 	{
 		printf("%s:%s:%s\n", probemod, probefunc, probename);
-- 
2.18.4

Re: [DTrace-devel] [PATCH 4/8] test: Remove unneeded -w option

[Kris Van Hees]

On Tue, Jun 04, 2024 at 02:00:04PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  test/unittest/usdt/tst.dlclose2.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/unittest/usdt/tst.dlclose2.sh b/test/unittest/usdt/tst.dlclose2.sh
> index 94be773d..6044cf4b 100755
> --- a/test/unittest/usdt/tst.dlclose2.sh
> +++ b/test/unittest/usdt/tst.dlclose2.sh
> @@ -128,7 +128,7 @@ if [ $? -ne 0 ]; then
>  fi
>  
>  script() {
> -	$dtrace -w -c ./main -Zqs /dev/stdin <<EOF
> +	$dtrace -c ./main -Zqs /dev/stdin <<EOF
>  	test_prov*:::
>  	{
>  		printf("%s:%s:%s\n", probemod, probefunc, probename);
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

[PATCH 5/8] Fix stddev() carryover computation

[eugene.loh]

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

The stddev() aggregation function squares 64-bit data values.  A
value is split into 32-bit high and low values.  Then, (high + low)
is squared to produce high*high, 2*high*low, and low*low.  Each is
managed in its own 64-bit register, with the final result residing
in two 64-bit registers.

When the 2*high*low portion is combined with the low*low portion,
care is exercised in case the combination has a carryover portion to
the higher bits.  This check was broken in the case where low==0.
That is, data values whose lowest 32 bits were 0 resulted in
outrageously bad stddev() results.

Fix the check and add a test for such cases.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_cg.c                |  2 +-
 test/unittest/aggs/tst.stddev2.d | 45 ++++++++++++++++++++++++++++++++
 test/unittest/aggs/tst.stddev2.r | 13 +++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 test/unittest/aggs/tst.stddev2.d
 create mode 100644 test/unittest/aggs/tst.stddev2.r

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index a1c24e37..fc2cebf0 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -8290,7 +8290,7 @@ dt_cg_agg_stddev(dt_pcb_t *pcb, dt_ident_t *aid, dt_node_t *dnp,
 	/* Add low value part from mid to lowreg */
 	emit(dlp,  BPF_ALU64_REG(BPF_ADD, lowreg, lmdreg));
 	/* Handle the overflow/carry case */
-	emit(dlp,  BPF_BRANCH_REG(BPF_JLT, lmdreg, lowreg, Lncy));
+	emit(dlp,  BPF_BRANCH_REG(BPF_JLE, lmdreg, lowreg, Lncy));
 	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, hi_reg, 1)) /* account for carry */;
 
 	/* Sum high value; no overflow expected nor accounted for */
diff --git a/test/unittest/aggs/tst.stddev2.d b/test/unittest/aggs/tst.stddev2.d
new file mode 100644
index 00000000..994bc3e2
--- /dev/null
+++ b/test/unittest/aggs/tst.stddev2.d
@@ -0,0 +1,45 @@
+/*
+ * 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: Positive stddev() test
+ *
+ * SECTION: Aggregations/Aggregations
+ *
+ * NOTES: This is a simple verifiable positive test of the stddev() function.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	@a = stddev(          0); @a = stddev(          0);
+	@b = stddev(       0x10); @b = stddev(       0x20);
+	@c = stddev(      0x100); @c = stddev(      0x200);
+	@d = stddev(     0x1000); @d = stddev(     0x2000);
+	@e = stddev(    0x10000); @e = stddev(    0x20000);
+	@f = stddev(   0x100000); @f = stddev(   0x200000);
+	@g = stddev(  0x1000000); @g = stddev(  0x2000000);
+	@h = stddev( 0x10000000); @h = stddev( 0x20000000);
+	@i = stddev( 0x20000000); @i = stddev( 0x40000000);
+	@j = stddev( 0x40000000); @j = stddev( 0x80000000);
+	@k = stddev( 0x80000000); @k = stddev(0x100000000);
+	@l = stddev(0x100000000); @l = stddev(0x200000000);
+	printa("%9@x\n", @a);
+	printa("%9@x\n", @b);
+	printa("%9@x\n", @c);
+	printa("%9@x\n", @d);
+	printa("%9@x\n", @e);
+	printa("%9@x\n", @f);
+	printa("%9@x\n", @g);
+	printa("%9@x\n", @h);
+	printa("%9@x\n", @i);
+	printa("%9@x\n", @j);
+	printa("%9@x\n", @k);
+	printa("%9@x\n", @l);
+	exit(0);
+}
diff --git a/test/unittest/aggs/tst.stddev2.r b/test/unittest/aggs/tst.stddev2.r
new file mode 100644
index 00000000..16e17736
--- /dev/null
+++ b/test/unittest/aggs/tst.stddev2.r
@@ -0,0 +1,13 @@
+
+        0
+        8
+       80
+      800
+     8000
+    80000
+   800000
+  8000000
+ 10000000
+ 20000000
+ 40000000
+ 80000000
-- 
2.18.4

Re: [DTrace-devel] [PATCH 5/8] Fix stddev() carryover computation

[Kris Van Hees]

On Tue, Jun 04, 2024 at 02:00:05PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> The stddev() aggregation function squares 64-bit data values.  A
> value is split into 32-bit high and low values.  Then, (high + low)
> is squared to produce high*high, 2*high*low, and low*low.  Each is
> managed in its own 64-bit register, with the final result residing
> in two 64-bit registers.
> 
> When the 2*high*low portion is combined with the low*low portion,
> care is exercised in case the combination has a carryover portion to
> the higher bits.  This check was broken in the case where low==0.
> That is, data values whose lowest 32 bits were 0 resulted in
> outrageously bad stddev() results.
> 
> Fix the check and add a test for such cases.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  libdtrace/dt_cg.c                |  2 +-
>  test/unittest/aggs/tst.stddev2.d | 45 ++++++++++++++++++++++++++++++++
>  test/unittest/aggs/tst.stddev2.r | 13 +++++++++
>  3 files changed, 59 insertions(+), 1 deletion(-)
>  create mode 100644 test/unittest/aggs/tst.stddev2.d
>  create mode 100644 test/unittest/aggs/tst.stddev2.r
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index a1c24e37..fc2cebf0 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -8290,7 +8290,7 @@ dt_cg_agg_stddev(dt_pcb_t *pcb, dt_ident_t *aid, dt_node_t *dnp,
>  	/* Add low value part from mid to lowreg */
>  	emit(dlp,  BPF_ALU64_REG(BPF_ADD, lowreg, lmdreg));
>  	/* Handle the overflow/carry case */
> -	emit(dlp,  BPF_BRANCH_REG(BPF_JLT, lmdreg, lowreg, Lncy));
> +	emit(dlp,  BPF_BRANCH_REG(BPF_JLE, lmdreg, lowreg, Lncy));
>  	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, hi_reg, 1)) /* account for carry */;
>  
>  	/* Sum high value; no overflow expected nor accounted for */
> diff --git a/test/unittest/aggs/tst.stddev2.d b/test/unittest/aggs/tst.stddev2.d
> new file mode 100644
> index 00000000..994bc3e2
> --- /dev/null
> +++ b/test/unittest/aggs/tst.stddev2.d
> @@ -0,0 +1,45 @@
> +/*
> + * 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: Positive stddev() test
> + *
> + * SECTION: Aggregations/Aggregations
> + *
> + * NOTES: This is a simple verifiable positive test of the stddev() function.
> + */
> +
> +#pragma D option quiet
> +
> +BEGIN
> +{
> +	@a = stddev(          0); @a = stddev(          0);
> +	@b = stddev(       0x10); @b = stddev(       0x20);
> +	@c = stddev(      0x100); @c = stddev(      0x200);
> +	@d = stddev(     0x1000); @d = stddev(     0x2000);
> +	@e = stddev(    0x10000); @e = stddev(    0x20000);
> +	@f = stddev(   0x100000); @f = stddev(   0x200000);
> +	@g = stddev(  0x1000000); @g = stddev(  0x2000000);
> +	@h = stddev( 0x10000000); @h = stddev( 0x20000000);
> +	@i = stddev( 0x20000000); @i = stddev( 0x40000000);
> +	@j = stddev( 0x40000000); @j = stddev( 0x80000000);
> +	@k = stddev( 0x80000000); @k = stddev(0x100000000);
> +	@l = stddev(0x100000000); @l = stddev(0x200000000);
> +	printa("%9@x\n", @a);
> +	printa("%9@x\n", @b);
> +	printa("%9@x\n", @c);
> +	printa("%9@x\n", @d);
> +	printa("%9@x\n", @e);
> +	printa("%9@x\n", @f);
> +	printa("%9@x\n", @g);
> +	printa("%9@x\n", @h);
> +	printa("%9@x\n", @i);
> +	printa("%9@x\n", @j);
> +	printa("%9@x\n", @k);
> +	printa("%9@x\n", @l);
> +	exit(0);
> +}
> diff --git a/test/unittest/aggs/tst.stddev2.r b/test/unittest/aggs/tst.stddev2.r
> new file mode 100644
> index 00000000..16e17736
> --- /dev/null
> +++ b/test/unittest/aggs/tst.stddev2.r
> @@ -0,0 +1,13 @@
> +
> +        0
> +        8
> +       80
> +      800
> +     8000
> +    80000
> +   800000
> +  8000000
> + 10000000
> + 20000000
> + 40000000
> + 80000000
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

[PATCH 6/8] test: Check dtrace return status

[eugene.loh]

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

This test was spuriously passing.  DTrace failed to run -- many error
messages in the log file indicated as much -- and yet the test passed.

Check the dtrace return status.  Due to recent USDT changes, the test
now passes (properly).

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/unittest/usdt/tst.forker.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/test/unittest/usdt/tst.forker.sh b/test/unittest/usdt/tst.forker.sh
index 018a6572..92513eb5 100755
--- a/test/unittest/usdt/tst.forker.sh
+++ b/test/unittest/usdt/tst.forker.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 #
 # Oracle Linux DTrace.
-# Copyright (c) 2007, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2007, 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.
 #
@@ -32,6 +32,10 @@ while kill -0 $id >/dev/null 2>&1; do
 			exit(0);
 		}
 	EOF
+	if [ $? -ne 0 ]; then
+		echo ERROR: DTrace failed to run
+		exit 1
+	fi
 done
 
 exit 0
-- 
2.18.4

[PATCH 7/8] Clean up double semicolons

[eugene.loh]

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

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 dtprobed/dtprobed.c          | 2 +-
 libdtrace/dt_cg.c            | 2 +-
 libdtrace/dt_probe.c         | 4 ++--
 libdtrace/dt_prov_fbt.c      | 2 +-
 libdtrace/dt_prov_lockstat.c | 4 ++--
 libdtrace/dt_prov_profile.c  | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index bbdde35e..fdcdee14 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -516,7 +516,7 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
 		gen = dof_stash_remove(pid, (uintptr_t) arg);
 		fuse_reply_ioctl(req, gen, NULL, 0);
 		return;
-	default: errmsg = "invalid ioctl";;
+	default: errmsg = "invalid ioctl";
 		fuse_log(FUSE_LOG_WARNING, "%i: dtprobed: %s %x\n",
 			 pid, errmsg, cmd);
 		goto fuse_err;
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index fc2cebf0..e166d2d8 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -6562,7 +6562,7 @@ dt_cg_subr_inet_ntop(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 {
 	dt_node_t	*af = dnp->dn_args;
 	dt_node_t	*addr = af->dn_list;
-	dt_node_t	*tnp, *cnp, *lnp, *rnp, *anp, *xnp;;
+	dt_node_t	*tnp, *cnp, *lnp, *rnp, *anp, *xnp;
 	dt_idsig_t	*isp;
 	dt_decl_t	*ddp;
 
diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
index 4f8730b9..e1099d75 100644
--- a/libdtrace/dt_probe.c
+++ b/libdtrace/dt_probe.c
@@ -1345,7 +1345,7 @@ dt_probe_add_clause(dtrace_hdl_t *dtp, dt_probe_t *prp, dt_ident_t *idp)
 {
 	dt_probe_clause_t	*pcp;
 
-	pcp = dt_zalloc(dtp, sizeof(dt_probe_clause_t));;
+	pcp = dt_zalloc(dtp, sizeof(dt_probe_clause_t));
 	if (pcp == NULL)
 		return dt_set_errno(dtp, EDT_NOMEM);
 
@@ -1395,7 +1395,7 @@ dt_probe_add_dependent(dtrace_hdl_t *dtp, dt_probe_t *prp, dt_probe_t *dprp)
 			return 0;
 	}
 
-	pdp = dt_zalloc(dtp, sizeof(dt_probe_dependent_t));;
+	pdp = dt_zalloc(dtp, sizeof(dt_probe_dependent_t));
 	if (pdp == NULL)
 		return dt_set_errno(dtp, EDT_NOMEM);
 
diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
index fa888ed8..f9d81d9a 100644
--- a/libdtrace/dt_prov_fbt.c
+++ b/libdtrace/dt_prov_fbt.c
@@ -367,7 +367,7 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
 			       EVENTSFS, FBT_GROUP_DATA, prp->desc->fun) + 1;
 		fn = dt_alloc(dtp, len);
 		if (fn == NULL)
-			return -ENOENT;;
+			return -ENOENT;
 
 		snprintf(fn, len, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS,
 			 FBT_GROUP_DATA, prp->desc->fun);
diff --git a/libdtrace/dt_prov_lockstat.c b/libdtrace/dt_prov_lockstat.c
index e98287ca..c73edf9b 100644
--- a/libdtrace/dt_prov_lockstat.c
+++ b/libdtrace/dt_prov_lockstat.c
@@ -288,7 +288,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 			return 1;
 		} else {
 			int	kind = 1;	/* reader (default) */
-			uint_t	lbl_reset = dt_irlist_label(dlp);;
+			uint_t	lbl_reset = dt_irlist_label(dlp);
 
 			if (strstr(uprp->desc->fun, "_write_") != NULL)
 				kind = 0;	/* writer */
@@ -337,7 +337,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 			copy_lockaddr_into_cpuinfo(dlp, 0);
 			return 1;
 		} else {
-			uint_t	lbl_reset = dt_irlist_label(dlp);;
+			uint_t	lbl_reset = dt_irlist_label(dlp);
 
 			if (strstr(uprp->desc->fun, "_trylock") != NULL) {
 				/* The return value (arg1) must be 1. */
diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
index 86fc03cb..bc224348 100644
--- a/libdtrace/dt_prov_profile.c
+++ b/libdtrace/dt_prov_profile.c
@@ -261,7 +261,7 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
 {
 	profile_probe_t		*pp = prp->prv_data;
 	struct perf_event_attr	attr;
-	int			i, nattach = 0;;
+	int			i, nattach = 0;
 	int			cnt = FDS_CNT(pp->kind);
 
 	memset(&attr, 0, sizeof(attr));
-- 
2.18.4

Re: [DTrace-devel] [PATCH 7/8] Clean up double semicolons

[Kris Van Hees]

On Tue, Jun 04, 2024 at 02:00:07PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  dtprobed/dtprobed.c          | 2 +-
>  libdtrace/dt_cg.c            | 2 +-
>  libdtrace/dt_probe.c         | 4 ++--
>  libdtrace/dt_prov_fbt.c      | 2 +-
>  libdtrace/dt_prov_lockstat.c | 4 ++--
>  libdtrace/dt_prov_profile.c  | 2 +-
>  6 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
> index bbdde35e..fdcdee14 100644
> --- a/dtprobed/dtprobed.c
> +++ b/dtprobed/dtprobed.c
> @@ -516,7 +516,7 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
>  		gen = dof_stash_remove(pid, (uintptr_t) arg);
>  		fuse_reply_ioctl(req, gen, NULL, 0);
>  		return;
> -	default: errmsg = "invalid ioctl";;
> +	default: errmsg = "invalid ioctl";
>  		fuse_log(FUSE_LOG_WARNING, "%i: dtprobed: %s %x\n",
>  			 pid, errmsg, cmd);
>  		goto fuse_err;
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index fc2cebf0..e166d2d8 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -6562,7 +6562,7 @@ dt_cg_subr_inet_ntop(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
>  	dt_node_t	*af = dnp->dn_args;
>  	dt_node_t	*addr = af->dn_list;
> -	dt_node_t	*tnp, *cnp, *lnp, *rnp, *anp, *xnp;;
> +	dt_node_t	*tnp, *cnp, *lnp, *rnp, *anp, *xnp;
>  	dt_idsig_t	*isp;
>  	dt_decl_t	*ddp;
>  
> diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
> index 4f8730b9..e1099d75 100644
> --- a/libdtrace/dt_probe.c
> +++ b/libdtrace/dt_probe.c
> @@ -1345,7 +1345,7 @@ dt_probe_add_clause(dtrace_hdl_t *dtp, dt_probe_t *prp, dt_ident_t *idp)
>  {
>  	dt_probe_clause_t	*pcp;
>  
> -	pcp = dt_zalloc(dtp, sizeof(dt_probe_clause_t));;
> +	pcp = dt_zalloc(dtp, sizeof(dt_probe_clause_t));
>  	if (pcp == NULL)
>  		return dt_set_errno(dtp, EDT_NOMEM);
>  
> @@ -1395,7 +1395,7 @@ dt_probe_add_dependent(dtrace_hdl_t *dtp, dt_probe_t *prp, dt_probe_t *dprp)
>  			return 0;
>  	}
>  
> -	pdp = dt_zalloc(dtp, sizeof(dt_probe_dependent_t));;
> +	pdp = dt_zalloc(dtp, sizeof(dt_probe_dependent_t));
>  	if (pdp == NULL)
>  		return dt_set_errno(dtp, EDT_NOMEM);
>  
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index fa888ed8..f9d81d9a 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -367,7 +367,7 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
>  			       EVENTSFS, FBT_GROUP_DATA, prp->desc->fun) + 1;
>  		fn = dt_alloc(dtp, len);
>  		if (fn == NULL)
> -			return -ENOENT;;
> +			return -ENOENT;
>  
>  		snprintf(fn, len, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS,
>  			 FBT_GROUP_DATA, prp->desc->fun);
> diff --git a/libdtrace/dt_prov_lockstat.c b/libdtrace/dt_prov_lockstat.c
> index e98287ca..c73edf9b 100644
> --- a/libdtrace/dt_prov_lockstat.c
> +++ b/libdtrace/dt_prov_lockstat.c
> @@ -288,7 +288,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  			return 1;
>  		} else {
>  			int	kind = 1;	/* reader (default) */
> -			uint_t	lbl_reset = dt_irlist_label(dlp);;
> +			uint_t	lbl_reset = dt_irlist_label(dlp);
>  
>  			if (strstr(uprp->desc->fun, "_write_") != NULL)
>  				kind = 0;	/* writer */
> @@ -337,7 +337,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  			copy_lockaddr_into_cpuinfo(dlp, 0);
>  			return 1;
>  		} else {
> -			uint_t	lbl_reset = dt_irlist_label(dlp);;
> +			uint_t	lbl_reset = dt_irlist_label(dlp);
>  
>  			if (strstr(uprp->desc->fun, "_trylock") != NULL) {
>  				/* The return value (arg1) must be 1. */
> diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
> index 86fc03cb..bc224348 100644
> --- a/libdtrace/dt_prov_profile.c
> +++ b/libdtrace/dt_prov_profile.c
> @@ -261,7 +261,7 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
>  {
>  	profile_probe_t		*pp = prp->prv_data;
>  	struct perf_event_attr	attr;
> -	int			i, nattach = 0;;
> +	int			i, nattach = 0;
>  	int			cnt = FDS_CNT(pp->kind);
>  
>  	memset(&attr, 0, sizeof(attr));
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

[PATCH 8/8] Turn some leading spaces into tabs

[eugene.loh]

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

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_bpf.c | 2 +-
 libdtrace/dt_pid.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index e1a09215..71c6a446 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -1140,7 +1140,7 @@ dt_bpf_load_prog(dtrace_hdl_t *dtp, const dt_probe_t *prp,
 
 		snprintf(msg, sizeof(msg),
 			 "BPF program load for '%s:%s:%s:%s' failed",
-		         pdp->prv, pdp->mod, pdp->fun, pdp->prb);
+			 pdp->prv, pdp->mod, pdp->fun, pdp->prb);
 		if (err == EPERM)
 			return dt_bpf_lockmem_error(dtp, msg);
 		dt_bpf_error(dtp, "%s: %s\n", msg,
diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
index 02ed7511..7c7d7e30 100644
--- a/libdtrace/dt_pid.c
+++ b/libdtrace/dt_pid.c
@@ -905,7 +905,7 @@ dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t
 		provider = (dof_parsed_t *) p;
 		if (!validate_dof_record(path, provider, DIT_PROVIDER, dof_buf_size,
 					 seen_size))
-                        goto parse_err;
+			goto parse_err;
 
 		prv = provider->provider.name;
 
@@ -915,16 +915,16 @@ dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t
 		probe = (dof_parsed_t *) p;
 		if (!validate_dof_record(path, probe, DIT_PROBE, dof_buf_size,
 					 seen_size))
-                        goto parse_err;
+			goto parse_err;
 
-                mod = probe->probe.name;
+		mod = probe->probe.name;
 		fun = mod + strlen(mod) + 1;
 		prb = fun + strlen(fun) + 1;
 
 		p += probe->size;
 		seen_size += probe->size;
 
-                /*
+		/*
 		 * Now the parsed DOF for this probe's tracepoints.
 		 */
 		for (size_t j = 0; j < probe->probe.ntp; j++) {
-- 
2.18.4

Re: [DTrace-devel] [PATCH 8/8] Turn some leading spaces into tabs

[Kris Van Hees]

On Tue, Jun 04, 2024 at 02:00:08PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  libdtrace/dt_bpf.c | 2 +-
>  libdtrace/dt_pid.c | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index e1a09215..71c6a446 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -1140,7 +1140,7 @@ dt_bpf_load_prog(dtrace_hdl_t *dtp, const dt_probe_t *prp,
>  
>  		snprintf(msg, sizeof(msg),
>  			 "BPF program load for '%s:%s:%s:%s' failed",
> -		         pdp->prv, pdp->mod, pdp->fun, pdp->prb);
> +			 pdp->prv, pdp->mod, pdp->fun, pdp->prb);
>  		if (err == EPERM)
>  			return dt_bpf_lockmem_error(dtp, msg);
>  		dt_bpf_error(dtp, "%s: %s\n", msg,
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index 02ed7511..7c7d7e30 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -905,7 +905,7 @@ dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t
>  		provider = (dof_parsed_t *) p;
>  		if (!validate_dof_record(path, provider, DIT_PROVIDER, dof_buf_size,
>  					 seen_size))
> -                        goto parse_err;
> +			goto parse_err;
>  
>  		prv = provider->provider.name;
>  
> @@ -915,16 +915,16 @@ dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t
>  		probe = (dof_parsed_t *) p;
>  		if (!validate_dof_record(path, probe, DIT_PROBE, dof_buf_size,
>  					 seen_size))
> -                        goto parse_err;
> +			goto parse_err;
>  
> -                mod = probe->probe.name;
> +		mod = probe->probe.name;
>  		fun = mod + strlen(mod) + 1;
>  		prb = fun + strlen(fun) + 1;
>  
>  		p += probe->size;
>  		seen_size += probe->size;
>  
> -                /*
> +		/*
>  		 * Now the parsed DOF for this probe's tracepoints.
>  		 */
>  		for (size_t j = 0; j < probe->probe.ntp; j++) {
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

Re: [PATCH 1/8] test: Allow aggpercpu test to omit some CPUs

[Kris Van Hees]

On Tue, Jun 04, 2024 at 02:00:01PM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> The aggpercpu test uses profile-* to fire aggregations on every CPU.
> But whether for good (offline CPUs) or bad (unreliable probe) reasons,
> a CPU might not aggregate any values.
> 
> Allow this test to skip over such CPUs.  That is, while a BPF agg map
> might have the correct default value for the aggregation function, it
> may not see any anticipated data.  The quality of profile-* probes
> should be checked by a different test.
> 
> Also, when computing avg() and stddev() in the awk check program,
> truncate results to ints more often to mimic the algorithms in DTrace
> for more robust checks.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  test/unittest/aggs/tst.aggpercpu.sh | 62 +++++++++++++++++++++--------
>  1 file changed, 45 insertions(+), 17 deletions(-)
> 
> diff --git a/test/unittest/aggs/tst.aggpercpu.sh b/test/unittest/aggs/tst.aggpercpu.sh
> index be74890a..6092bd17 100755
> --- a/test/unittest/aggs/tst.aggpercpu.sh
> +++ b/test/unittest/aggs/tst.aggpercpu.sh
> @@ -46,7 +46,8 @@ fi
>  awk '
>      # The expected value for the aggregation is aggval.
>      # The expected value on a CPU is (m * cpu + b).
> -    function check(label, aggval, m, b) {
> +    # The default value on a CPU that did not fire is defval.
> +    function check(label, aggval, m, b, defval) {
>          # Check the aggregation over all CPUs.
>          getline;
>          print "check:", $0;
> @@ -54,12 +55,38 @@ awk '
>  
>          # Check the per-CPU values.
>          for (i = 1; i <= ncpu; i++) {
> -            getline;
> -            print "check:", $0;
> -            if (match($0, "^    \\[CPU ") != 1 ||
> -                strtonum($2) != cpu[i] ||
> -                strtonum($3) != m * cpu[i] + b)
> -                printf("ERROR: %s, agg per cpu %d, line: %s\n", label, cpu[i], $0);
> +            do {
> +                getline;
> +                print "check:", $0;
> +
> +                # pass is +1 for pass, -1 for error, 0 for skip CPU
> +                pass = 0;
> +
> +                if (match($0, "^    \\[CPU ") != 1) {
> +                    print "ERROR: no CPU to read"
> +                    pass = -1;
> +                } else if (strtonum($2) > cpu[i]) {
> +                    print "ERROR: skipped over expected CPU"
> +                    pass = -1;
> +                } else if (strtonum($2) == cpu[i]) {
> +                    if (strtonum($3) != m * cpu[i] + b) {
> +                        print "ERROR: wrong value"
> +                        pass = -1;
> +                    } else {
> +                        # right value
> +                        pass = +1;
> +                    }
> +                } else if ($3 != defval) {
> +                    print "ERROR: wrong default value"
> +                    pass = -1;
> +                } else {
> +                    # skip over CPU that apparently did not fire
> +                    pass = 0;
> +                }
> +
> +                if (pass == -1)
> +                    printf("ERROR: %s, agg per cpu %d, line: %s\n", label, cpu[i], $0);
> +            } while (pass == 0);
>          }
>      }
>  
> @@ -99,13 +126,14 @@ awk '
>      {
>          # First we finish computing our estimates for avg and stddev.
>          # (The other results require no further action.)
> +        # (We keep truncating to ints to mimic DTrace algorithms.)
>  
> -        xavg /= xcnt;
> +        xavg /= xcnt;             xavg = int(xavg);
>  
> -        xstm /= xcnt;
> -        xstd /= xcnt;
> +        xstm /= xcnt;             xstm = int(xstm);
> +        xstd /= xcnt;             xstd = int(xstd);
>          xstd -= xstm * xstm;
> -        xstd = int(sqrt(xstd));
> +        xstd = sqrt(xstd);        xstd = int(xstd);
>  
>          # Sort the cpus.
>  
> @@ -113,12 +141,12 @@ awk '
>  
>          # Now read the results and compare.
>  
> -        check("cnt", xcnt,  0,   1);
> -        check("avg", xavg, 10,   3);
> -        check("std", xstd,  0,   0);
> -        check("min", xmin, 30, -10);
> -        check("max", xmax, 40, -15);
> -        check("sum", xsum, 50,   0);
> +        check("cnt", xcnt,  0,   1, "0");
> +        check("avg", xavg, 10,   3, "0");
> +        check("std", xstd,  0,   0, "0");
> +        check("min", xmin, 30, -10,  "9223372036854775807");
> +        check("max", xmax, 40, -15, "-9223372036854775808");
> +        check("sum", xsum, 50,   0, "0");
>  
>          printf("done\n");
>      }
> -- 
> 2.18.4
>