[DTrace-devel] [oracle/dtrace-utils] 356174: Clean up agg identifier storage data handling

Wed Dec 9 21:56:50 PST 2020

  Branch: refs/heads/2.0-branch-dev
  Home:   https://github.com/oracle/dtrace-utils
  Commit: 3561741721adf2246feb906e965963e1ed0a2683
      https://github.com/oracle/dtrace-utils/commit/3561741721adf2246feb906e965963e1ed0a2683
  Author: Kris Van Hees <kris.van.hees at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_cg.c
    M libdtrace/dt_ident.c
    M libdtrace/dt_ident.h

  Log Message:
  -----------
  Clean up agg identifier storage data handling

The storage layout for aggregations is based on storing offset and
size data in the identifier.  This will now be done using the new
dt_ident_set_storage() function.  The dt_ident_t structure gets two
new fields:
- di_size: storage size of the identifier (only used for aggregations)
- di_hash: idhash that the identifier belongs to

This patch also introduces a DT_CG_AGG_SET_STORAGE() macro to handle
the code for only setting the storage data the first time we encounter
an aggregation.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

  Commit: ecbe2271e2bbb32657e70a84c474bbc4be8475b0
      https://github.com/oracle/dtrace-utils/commit/ecbe2271e2bbb32657e70a84c474bbc4be8475b0
  Author: Kris Van Hees <kris.van.hees at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M cmd/dtrace.c
    M include/dtrace/metadesc.h
    M include/dtrace/universal.h
    M libdtrace/dt_aggregate.c
    M libdtrace/dt_bpf.c
    M libdtrace/dt_cg.c
    M libdtrace/dt_consume.c
    M libdtrace/dt_ident.c
    M libdtrace/dt_ident.h
    M libdtrace/dt_impl.h
    M libdtrace/dt_map.c
    M libdtrace/dt_printf.c
    M libdtrace/dt_printf.h
    M libdtrace/dt_work.c
    M libdtrace/dtrace.h
    A test/unittest/actions/printa/err.D_PRINTA_AGGARG.fmt_int_arg.d
    A test/unittest/actions/printa/err.D_PRINTA_AGGARG.fmt_mixed_args.d
    A test/unittest/actions/printa/err.D_PRINTA_AGGARG.int_arg.d
    A test/unittest/actions/printa/err.D_PRINTA_AGGARG.mixed_args.d
    A test/unittest/actions/printa/err.D_PRINTA_AGGKEY.d
    A test/unittest/actions/printa/err.D_PRINTA_AGGPROTO.d
    A test/unittest/actions/printa/err.D_PRINTA_PROTO.fmt_no_arg.d
    A test/unittest/actions/printa/err.D_PRINTF_ARG_PROTO.key_no_val.d
    A test/unittest/actions/printa/err.D_PROTO_LEN.no_arg.d
    A test/unittest/aggs/err.D_AGG_REDEF.idx_diff_funcs.d
    R test/unittest/aggs/err.D_LLQUANT_STEPVAL.lt_factor.d
    A test/unittest/aggs/err.D_LLQUANT_STEPVAL.steps_divides_factor.d
    M test/unittest/aggs/tst.count.d
    M test/unittest/aggs/tst.lquantround.d
    M test/unittest/printa/err.D_PRINTA_AGGARG.badagg.d
    M test/unittest/printa/err.D_PRINTA_AGGARG.badfmt.d
    M test/unittest/printa/err.D_PRINTA_AGGARG.badval.d
    M test/unittest/printa/err.D_PRINTA_PROTO.bad.d
    M test/unittest/printa/tst.def.d
    M test/unittest/printa/tst.fmt.d
    M test/unittest/printa/tst.manyval.d

  Log Message:
  -----------
  Implementation of the printa() action

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.

The consumer processes the per-CPU aggregation data and aggregates it
into its final values.

The retrieval of aggregation data from the kernel does not pose any
concurrent access issues and therefore does not make use of the latch
sequence mechanism that the producer provides.  This is because the
data is currently stored in a regular BPF map that requires the use of
the bpf_map_lookup_elem syscall option.  This copies out the data with
preemption disabled.

Change summary:

  Global
    - General code cleanup
    - Replace dtrace_aggvarid_t by dtrace_aggid_t
    - Replace DTRACEAGG_* constants with DT_AGG_* constants
  cmd/dtrace.c
    - Remove disabling of default agg printing
  include/dtrace/metadesc.h
    struct dtrace_aggdesc
      - Removed dtagd_epid and dtagd_pad
      - Added dtagd_sig (lquantize and llquantize parameters)
      - Replaced dtagd_rec[1] with *dtagd_recs
  libdtrace/dt_aggregate.c
    - Switch from retrieving per-CPU aggregation data using ioctl() to
      retrieving the BPF map value for key 0 from the 'aggs' map.  The
      order of aggregation data blocks is defined by the order of the
      aggregations (by ID) in the dt_aggs idhash.
  libdtrace/dt_cg.c
    - Add support for storing aggregation IDs in dt_cg_store_val()
    - Implement printa()
  libdtrace/dt_consume.c
    - Remove obsolete support for reading parameters from first data value
    - Pass encoded agg function parameters to dt_print_datum()
    - Pass encoded lquantize parameters to dt_print_lquantize()
    - Pass encoded llquantize parameters to dt_print_llquantize()
    - Implement dt_printa()
  libdtrace/dt_map.c
    - Rewrite dt_aggid_add() to not try to retrieve data from the kernel
      using ioctl() calls but instead construct the data description
      based on the aggregation identifier.
  libdtrace/dt_printf.c
    - Adjust pfprint_*() functions to passing agg func parameters
  libdtrace/dt_work.c:
    - Move the call to dt_aggregate_go() to before the BEGIN probe to
      ensure that setting up the consumer side for aggregation handling
      does not affect whatever work is done in the BEGIN probe program.
  libdtrace/dtrace.h
    struct dtrace_aggdata
      - Renamed dtada_handle -> dtada_hdl
      - Removed dtada_ddesc and dtada_pdesc
  Tests
    - Various tests have been renamed to make their meaning more clear
    - Various tests have been adjusted to more accurately test things

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 245f5182d71a4b08f3b4f7790236a0a6e4d976f8
      https://github.com/oracle/dtrace-utils/commit/245f5182d71a4b08f3b4f7790236a0a6e4d976f8
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_dlibs.c

  Log Message:
  -----------
  Add BPF compiled functions even if no relocations

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 98cf062d8545cf8b112ace5f8cf2f4a57a9f158c
      https://github.com/oracle/dtrace-utils/commit/98cf062d8545cf8b112ace5f8cf2f4a57a9f158c
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M bpf/Build
    A bpf/agg_qbin.c
    M libdtrace/dt_cg.c
    M libdtrace/dt_dlibs.c

  Log Message:
  -----------
  Add code for quantize() aggregation function

First, quantize the value into a 0-based bin number.  While we could
generate BPF code in dt_cg.c to accomplish this, it is easier to
maintain C code in the bpf/ subdirectory to be cross-compiled into
BPF when DTrace is built.

Then, the "implementation" function -- which needs to run twice, once
per aggregation copy -- merely updates the value in the designated
bin.  This "implementation" can be used for lquantize() and llquantize()
as well.  For that matter, it could be used for other aggregations such
as sum(), though that special case has only one bin.  The function tests
the bin number against 0 and some maxbin, both as a safety check and
because the BPF verifier needs such assurances in certain cases.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: ca54e2b9ad6964a9335c811f20e6ddd22a332139
      https://github.com/oracle/dtrace-utils/commit/ca54e2b9ad6964a9335c811f20e6ddd22a332139
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M bpf/Build
    A bpf/agg_lqbin.c
    M libdtrace/dt_cg.c
    M libdtrace/dt_dlibs.c

  Log Message:
  -----------
  Compute lquantize() bin only once

First, quantize the value into a 0-based bin number.  As was done for
quantize(), use C code in the bpf/ subdirectory to be cross-compiled
into BPF when DTrace is built.

Then, the "implementation" function -- which needs to run twice, once
per aggregation copy -- is simply dt_cg_agg_quantize_impl().  That is,
we reuse the same function for lquantize() that was originally introduced
for quantize().

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 5db20ff1f447b03f764b6c114f7c25882722b616
      https://github.com/oracle/dtrace-utils/commit/5db20ff1f447b03f764b6c114f7c25882722b616
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_cg.c
    A test/unittest/aggs/err.D_LQUANT_MISMATCH.lqbadarg2.d

  Log Message:
  -----------
  Change dt_cg_agg_lquantize() arg check to agree with message

We check limitval<baseval, but the error message is "base must
be less than limit".  Make the check more stringent to agree
with the error message.

Note that this behavior dates back to DTrace v1:
     # dtrace -n 'BEGIN {@ = lquantize(8, 4, 4); exit(0) }'
     dtrace: invalid probe specifier:
     lquantize( ) step (argument #3) too large:
     must have at least one quantization level
That is, the arguments pass the check but are then caught by a
later, rather confusing, message.

Add a test to catch this case.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 8ddd048c9d8534ca4de8009466b9918e36e3ff62
      https://github.com/oracle/dtrace-utils/commit/8ddd048c9d8534ca4de8009466b9918e36e3ff62
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_cg.c
    A test/unittest/aggs/err.D_LLQUANT_STEPTYPE.llqstepszero.d
    A test/unittest/aggs/err.D_LLQUANT_STEPTYPE.neg.d
    R test/unittest/aggs/err.D_LLQUANT_STEPVAL.neg.d

  Log Message:
  -----------
  Handle steps==0 error for llquantize

We can get divide-by-zero failures if we do not explicitly check.

KVH: Renamed err.D_LLQUANT_STEPVAL.neg.d to err.D_LLQUANT_STEPTYPE.neg.d
     to accomodate this change.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: bca0562023d7fdfdff329e028444e084e53b5153
      https://github.com/oracle/dtrace-utils/commit/bca0562023d7fdfdff329e028444e084e53b5153
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_cg.c
    A test/unittest/aggs/err.D_LLQUANT_STEPVAL.llqdividefactor.d
    A test/unittest/aggs/err.D_LLQUANT_STEPVAL.llqmultiplefactor.d

  Log Message:
  -----------
  Add code for llquantize() aggregation function

The value is converted to a bin number with manually written BPF code,
in function dt_cg_agg_llquantize_bin(), rather than with C code that
is pre-compiled to BPF.  The reason is that the bin computation
depends on a number of parameters (factor, lmag, hmag, steps) and
combinations thereof.  Those combinations are a lot of run-time
computations, and the BPF code would put additional, combinatorially
growing load on the BPF verifier -- e.g., for quantities like
powl(factor, lmag).  Meanwhile, these parameters are actually known
at code-generation time.  So, we can do a lot of the computations at
code-generation time and just load final values into BPF registers,
saving a lot of run-time computation.  In contrast, the C-to-BPF
pre-compiled code would impose an undue burden on the verifier
because it needs to be generic code that needs to compute these
values that can be computed at script compile time.

The llquantize function is rather complex.  There is a big block of
comments to explain what we're trying to achieve.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: e9bc753267fae8e715b3fc12f66ccfd78251bee8
      https://github.com/oracle/dtrace-utils/commit/e9bc753267fae8e715b3fc12f66ccfd78251bee8
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_consume.c
    A test/unittest/aggs/tst.quantlast.d
    A test/unittest/aggs/tst.quantlast.r

  Log Message:
  -----------
  No data is reported for quantize() when it's all in the last bin

The search for a nonzero bin is broken for quantize(), causing
no data to be reported when it's all in the last bin.

Fix the search for the "first bin".  Make a few other small
changes so that the three *quantize() functions are more similar
and such coding errors are more apparent.

Add a test to catch this error.  The test is XFAIL for now,
because we do not have a consumer yet.

Orabug: 32148161
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 4f188336987e34a702e1b1f3700fa8c2918dd4a0
      https://github.com/oracle/dtrace-utils/commit/4f188336987e34a702e1b1f3700fa8c2918dd4a0
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_cg.c

  Log Message:
  -----------
  Add code for avg() aggregation function

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 5b459d0d3f3590f78adf856896344e0b03768037
      https://github.com/oracle/dtrace-utils/commit/5b459d0d3f3590f78adf856896344e0b03768037
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_cg.c
    M test/unittest/arithmetic/tst.basics.d
    M test/unittest/arithmetic/tst.basics.r
    M test/unittest/types/tst.assignops.d
    M test/unittest/types/tst.unaryop.d

  Log Message:
  -----------
  Fix bitwise negation

Orabug: 32125018
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: c7f41f0c4e3ecf4503d6a925039425515624ce5c
      https://github.com/oracle/dtrace-utils/commit/c7f41f0c4e3ecf4503d6a925039425515624ce5c
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_cg.c
    A test/unittest/aggs/err.D_CG_EXPR.from_expr.d

  Log Message:
  -----------
  Restore error message

An error message was "#if 0"ed out during development,
producing behavior like this:
    # dtrace -n 'BEGIN {avg(1); exit(0)}'
    dt_cg_node() - FUNC aggregating function avg()
    dtrace: description 'BEGIN ' matched 1 probe
    BPF: call to invalid destination
    BPF: verification time 14 usec
    BPF: stack depth 0+0
    BPF: processed 0 insns (limit 1000000) max_states_per_insn 0 ...
    dtrace: could not enable tracing: BPF program load for 'dtrac...

Restore the message.  Add a test.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 80c97f8b9158c60404e79dcd941e3aff1490ad26
      https://github.com/oracle/dtrace-utils/commit/80c97f8b9158c60404e79dcd941e3aff1490ad26
  Author: David Mc Lean <david.mclean at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M libdtrace/dt_aggregate.c
    M libdtrace/dt_cg.c

  Log Message:
  -----------
  Implement aggregation functions: max(), min(), sum(), and stddev()

The min() and max() functions require initialization of their initial
value.  That is, data for min() is initialized with the highest
possible value and data for max() is initialized with the lowest
possible value.  This initialization happens in dt_aggregate_go().

The stddev() function records the number of values, the sum of values,
and the 128-bit sum of the squares of each value.  This information is
processed by the consumer to determine the standard deviation value.

Signed-off-by: David Mc Lean <david.mclean at oracle.com>
Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: b0c64206abe9bb227a25de510601b12ef89cd786
      https://github.com/oracle/dtrace-utils/commit/b0c64206abe9bb227a25de510601b12ef89cd786
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    A test/unittest/aggs/tst.multicpus.sh

  Log Message:
  -----------
  Add a multi-CPU aggregation test

A key characteristic of DTrace aggregations is that they are
gathered per CPU and then aggregated across CPUs.  Nevertheless,
the test suite uses only BEGIN and tick-n probes for aggregation
tests.  So, aggregation across CPUs is never really tested.

Add a test for the simplest aggregations with a probe that fires
across all CPUs.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 2590c80fb0fa8981bb00d480ba100e5ea8265a25
      https://github.com/oracle/dtrace-utils/commit/2590c80fb0fa8981bb00d480ba100e5ea8265a25
  Author: Kris Van Hees <kris.van.hees at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M test/unittest/aggs/tst.allquant.d
    M test/unittest/aggs/tst.avg.d
    M test/unittest/aggs/tst.avg_neg.d
    M test/unittest/aggs/tst.llquantincr.d
    M test/unittest/aggs/tst.llquantize.d
    M test/unittest/aggs/tst.llquantneg.d
    M test/unittest/aggs/tst.llquantoverflow.d
    M test/unittest/aggs/tst.llquantround.d
    M test/unittest/aggs/tst.max.d
    M test/unittest/aggs/tst.max_neg.d
    M test/unittest/aggs/tst.min.d
    M test/unittest/aggs/tst.min_neg.d
    M test/unittest/aggs/tst.multiaggs2.d
    M test/unittest/aggs/tst.quantlast.d
    M test/unittest/aggs/tst.quantround.d
    M test/unittest/aggs/tst.stddev.d
    M test/unittest/aggs/tst.sum.d
    M test/unittest/printa/tst.walltimestamp.sh

  Log Message:
  -----------
  Remove @@xfail marker for tests that should pass now

With the implementation of basic aggregation support and the printa()
action, more tests should now be able to pass.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Reviewed-by: Elena Zannoni <elena.zannoni at oracle.com>

  Commit: b3e21df5911b76f6fcb3df0c6f07e8a9b814a1d8
      https://github.com/oracle/dtrace-utils/commit/b3e21df5911b76f6fcb3df0c6f07e8a9b814a1d8
  Author: Eugene Loh <eugene.loh at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M test/unittest/syscall/tst.entry.r
    A test/unittest/syscall/tst.entry.r.p

  Log Message:
  -----------
  Allow for changing number of syscalls

DTrace reports how many probes were matched.  Since the number of
syscalls can depend on the kernel, the number of matched syscalls
expected should not be hardcoded in our test result files.  This
problem seems to impact only test/unittest/syscall/tst.entry.d .

Add a .r.p post-processing file to mask out the exact number of
matched probes for this test.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

  Commit: 187c56aa3f781c63e8e393010682f1414c42fa95
      https://github.com/oracle/dtrace-utils/commit/187c56aa3f781c63e8e393010682f1414c42fa95
  Author: Kris Van Hees <kris.van.hees at oracle.com>
  Date:   2020-12-09 (Wed, 09 Dec 2020)

  Changed paths:
    M NEWS
    M dtrace.spec

  Log Message:
  -----------
  Update NEWS and spec file for errata release 2.0.0-1.4

Orabug: 32254734

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>

Compare: https://github.com/oracle/dtrace-utils/compare/4510a4b4f95d...187c56aa3f78