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

[Eugene Loh]

On 12/07/2020 01:07 PM, Kris Van Hees wrote:

> Given the design for the handling of aggregation data, there is no
> further need for cummulative data retrieval using snapshots.  When we
> are ready to display aggregation data, we can retrieve the data by
> performing a key 0 BPF map lookup on the 'aggs' map.  It gives us all
> data for all CPUs in a single buffer.

I finished taking a quick look through.  I cannot say I understand the 
code, but:
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

I do think it would be nice to explain, even if just briefly, why the 
consumer ignores the latch sequence number.  Ideally in both the commit 
message and the aggregation snapshot code.

And, a few unremarkable observations:

The following loop in dt_aggregate_snap_one(), at "accumulate:" in the 
switch statement in the default case:
     for (i = 0, cnt = aid->di_size / sizeof(int64_t);
          i < cnt; i++, dst++, src++)
         *dst += *src;
could more clearly be written as:
     cnt = aid->di_size / sizeof(int64_t);
     for (i = 0; i < cnt; i++)
         dst[i] += src[i];

In dt_aggregate_snap_cpu(), there are two consecutive return 
statements.  The second one is unreachable.

In the refactoring of pfprint_fp(), the introductory left curly bracket 
should be left on its own line to be consistent with the style that is 
otherwise employed.