[DTrace-devel] [PATCH] Implementation of the printa() action

[Eugene Loh]

Sorry for the piecemeal feedback.  Here are some more comments/questions:

1)  The test/unittest/aggs/tst.*.d tests all seem to use BEGIN and 
tick-n probes.  So, we are not really testing aggregation over CPUs, 
which would seem to be a big omission in testing.  Tests that require 
aggregation over CPUs must be added.

2)  It appears that aggregation over CPUs is missing.  Consider DTv1:

         # /usr/sbin/dtrace -V
         dtrace: Sun D 1.6.4
         # /usr/sbin/dtrace -qn 'profile-1 { @ = count(); } tick-1500ms 
{ exit(0) }'
                 8
        # grep processor /proc/cpuinfo
        processor    : 0
        processor    : 1
        processor    : 2
        processor    : 3
        processor    : 4
        processor    : 5
        processor    : 6
        processor    : 7

One count per CPU.  Gotcha.  How about with DTv2 (smaller system)?

        # build/run-dtrace -V
        dtrace: Oracle D 2.0
        # build/run-dtrace -qn 'profile-1 { @ = count() } tick-1500ms { 
exit(0) }'
                 1
        # grep processor /proc/cpuinfo
        processor    : 0
        processor    : 1
        processor    : 2
        processor    : 3

3)  The aggregations have a per-CPU latch sequence number.  I thought 
the consumer should use this number in two ways:
     > to check which aggregation copy to use
     > to confirm that the copy that was used was not written during the 
read
I do not see where the consumer employs the sequence number.  I imagine 
user-visible behavior could be quite tolerant of this omission, but it 
is "advertised" behavior.  So if its implementation is being deferred, 
we should at least note that.

4)  The second sentence in this comment block before 
dtrace_aggregate_snap() looks like it needs to be cleaned up (or even 
just omitted):

         /*
          * Retrieve all aggregation data from the enabled CPUs and 
aggregate it into
          * its global values.  Note that we do not clear the per-CPU 
aggregation data
          * is not reset after retrieval, so trievals are not cumulative.
          */

5)  Will dtada_percpu_delta someday be used?  Currently, I guess it is not.