[DTrace-devel] [PATCH 5/7] Consolidate like keys for @[mod()], @[sym()], etc.
Kris Van Hees
kris.van.hees at oracle.com
Thu Jul 25 01:20:58 UTC 2024
On Mon, Mar 11, 2024 at 09:00:55PM -0400, eugene.loh at oracle.com wrote:
>
> Certain actions like mod() and sym() take address arguments and
> report string values. These values can be used as aggregation
> keys.
>
> DTrace happens to pass the raw addresses from producer to consumer,
> for the consumer to convert to strings and combine like keys for
> aggregation. The reduction of common values had been omitted in the
> DTrace-to-Linux port.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_aggregate.c | 38 ++++++++++++++++++++-------
> test/unittest/aggs/tst.aggmod_full.sh | 4 +--
> test/unittest/profile-n/tst.ufunc.sh | 29 ++++++++++++++------
> test/unittest/profile-n/tst.umod.sh | 26 +++++++++++++-----
> test/unittest/profile-n/tst.usym.sh | 27 ++++++++++++++-----
> 5 files changed, 91 insertions(+), 33 deletions(-)
>
> diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
> index b0642e65..6472360b 100644
> --- a/libdtrace/dt_aggregate.c
> +++ b/libdtrace/dt_aggregate.c
> @@ -278,12 +278,11 @@ dt_aggregate_quantizedcmp(int64_t *lhs, int64_t *rhs)
> return 0;
> }
>
> -#ifdef FIXME
> static void
> dt_aggregate_usym(dtrace_hdl_t *dtp, uint64_t *data)
> {
> - uint64_t tgid = data[1];
> - uint64_t *pc = &data[2];
> + uint64_t tgid = data[0];
> + uint64_t *pc = &data[1];
> pid_t pid;
> GElf_Sym sym;
>
> @@ -304,8 +303,8 @@ dt_aggregate_usym(dtrace_hdl_t *dtp, uint64_t *data)
> static void
> dt_aggregate_umod(dtrace_hdl_t *dtp, uint64_t *data)
> {
> - uint64_t tgid = data[1];
> - uint64_t *pc = &data[2];
> + uint64_t tgid = data[0];
> + uint64_t *pc = &data[1];
> pid_t pid;
> const prmap_t *map;
>
> @@ -379,7 +378,6 @@ dt_aggregate_mod(dtrace_hdl_t *dtp, uint64_t *addr)
> }
> }
> }
> -#endif
>
> static dtrace_aggid_t
> dt_aggregate_aggid(dt_ahashent_t *ent)
> @@ -514,7 +512,7 @@ dt_aggregate_snap_one(dtrace_hdl_t *dtp, int aggid, int cpu, const char *key,
> uint64_t dgen;
> size_t ndx = hval % agh->dtah_size;
> size_t size;
> - int rval;
> + int rval, i;
>
> /* Data generation: skip if 0 */
> dgen = *(uint64_t *)data;
> @@ -526,6 +524,28 @@ dt_aggregate_snap_one(dtrace_hdl_t *dtp, int aggid, int cpu, const char *key,
> if (rval != 0)
> return rval;
>
> + /* Reduce addresses. */
> + for (i = 0; i < agg->dtagd_nkrecs; i++) {
> + uint64_t *p = (uint64_t *)&key[agg->dtagd_krecs[i].dtrd_offset];
> +
> + switch(agg->dtagd_krecs[i].dtrd_action) {
> + case DTRACEACT_USYM:
> + dt_aggregate_usym(dtp, p);
> + break;
> + case DTRACEACT_UMOD:
> + dt_aggregate_umod(dtp, p);
> + break;
> + case DTRACEACT_SYM:
> + dt_aggregate_sym(dtp, p);
> + break;
> + case DTRACEACT_MOD:
> + dt_aggregate_mod(dtp, p);
> + break;
> + default:
> + break;
> + }
> + }
> +
> /* See if we already have an entry for this aggregation. */
> for (h = agh->dtah_hash[ndx]; h != NULL; h = h->dtahe_next) {
> dt_ident_t *aid;
> @@ -654,10 +674,10 @@ dt_aggregate_snap_cpu(dtrace_hdl_t *dtp, processorid_t cpu, int fd)
>
> memcpy(key, nxt, ksize);
>
> - if (dt_bpf_map_lookup(fd, key, data) == -1)
> + if (dt_bpf_map_lookup(fd, nxt, data) == -1)
> return dt_set_errno(dtp, EDT_BPF);
>
> - rval = dt_aggregate_snap_one(dtp, *aggidp, cpu, key, data);
> + rval = dt_aggregate_snap_one(dtp, *aggidp, cpu, nxt, data);
> if (rval != 0)
> return rval;
> }
> diff --git a/test/unittest/aggs/tst.aggmod_full.sh b/test/unittest/aggs/tst.aggmod_full.sh
> index 9707340d..5a9286cf 100755
> --- a/test/unittest/aggs/tst.aggmod_full.sh
> +++ b/test/unittest/aggs/tst.aggmod_full.sh
> @@ -17,8 +17,6 @@
> #
> ##
>
> -# @@xfail: dtv2
> -
> # Big batches of addresses are tested at once in order to
> # - manage run times
> # - check that each module appears with a single name
> @@ -225,7 +223,7 @@ fi
> # if the timeout will soon be reached, just report how many we checked.
> # But do insist that some minimum number have been checked.
>
> -progress_done_min=3000
> +progress_done_min=800
> if [[ $progress_done -lt $progress_done_min ]]; then
> echo "ERROR: only $progress_done symbols checked"
> echo "ERROR: expect minimum of $progress_done_min"
> diff --git a/test/unittest/profile-n/tst.ufunc.sh b/test/unittest/profile-n/tst.ufunc.sh
> index 74ab6201..28b7c05b 100755
> --- a/test/unittest/profile-n/tst.ufunc.sh
> +++ b/test/unittest/profile-n/tst.ufunc.sh
> @@ -6,6 +6,8 @@
> # http://oss.oracle.com/licenses/upl.
> #
>
> +tmpfile=$tmpdir/tst.profile_ufunc.$$
> +
> script()
> {
> $dtrace $dt_flags -qs /dev/stdin <<EOF
> @@ -40,13 +42,24 @@ spinny &
> child=$!
> disown %+
>
> -#
> -# The only thing we can be sure of here is that we caught some function in
> -# bash doing work. (This actually goes one step further and assumes that we
> -# catch some non-static function in bash.)
> -#
> -script | tee /dev/fd/2 | grep 'bash`[a-zA-Z_]' > /dev/null
> -status=$?
> -
> +script >& $tmpfile
> kill $child
> +
> +# Check that some non-static function in bash is doing work.
> +status=0
> +if ! grep -q 'bash`[a-zA-Z_]' $tmpfile; then
> + echo ERROR: expected bash functions are missing
> + status=1
> +fi
> +
> +# Check that functions are unique. (Exclude shared libraries and unresolved addresses.)
> +if awk '!/^ *lib/ && !/^ *0x/ {print $1}' $tmpfile | sort | uniq -c | grep -qv " 1 "; then
> + echo ERROR: duplicate ufunc
> + status=1
> +fi
> +
> +if [ $status -ne 0 ]; then
> + cat $tmpfile
> +fi
> +
> exit $status
> diff --git a/test/unittest/profile-n/tst.umod.sh b/test/unittest/profile-n/tst.umod.sh
> index 6a7382ea..ea4f7ec5 100755
> --- a/test/unittest/profile-n/tst.umod.sh
> +++ b/test/unittest/profile-n/tst.umod.sh
> @@ -6,6 +6,8 @@
> # http://oss.oracle.com/licenses/upl.
> #
>
> +tmpfile=$tmpdir/tst.profile_umod.$$
> +
> script()
> {
> $dtrace $dt_flags -qs /dev/stdin <<EOF
> @@ -40,11 +42,23 @@ spinny &
> child=$!
> disown %+
>
> -#
> -# The only thing we can be sure of here is that bash is doing some work.
> -#
> -script | tee /dev/fd/2 | grep -w bash > /dev/null
> -status=$?
> -
> +script >& $tmpfile
> kill $child
> +
> +# Check that bash is doing work.
> +status=0
> +if ! grep -wq 'bash' $tmpfile; then
> + echo ERROR: expected bash functions are missing
> + status=1
> +fi
> +
> +# Check that modules are unique. (Exclude shared libraries and unresolved addresses.)
> +if awk '!/^ *lib/ && !/^ *0x/ {print $1}' $tmpfile | sort | uniq -c | grep -qv " 1 "; then
> + echo ERROR: duplicate umod
> + status=1
> +fi
> +
> +if [ $status -ne 0 ]; then
> + cat $tmpfile
> +fi
> exit $status
> diff --git a/test/unittest/profile-n/tst.usym.sh b/test/unittest/profile-n/tst.usym.sh
> index 5f2df717..72ff4fd4 100755
> --- a/test/unittest/profile-n/tst.usym.sh
> +++ b/test/unittest/profile-n/tst.usym.sh
> @@ -6,6 +6,8 @@
> # http://oss.oracle.com/licenses/upl.
> #
>
> +tmpfile=$tmpdir/tst.profile_usym.$$
> +
> script()
> {
> $dtrace $dt_flags -qs /dev/stdin <<EOF
> @@ -40,12 +42,23 @@ spinny &
> child=$!
> disown %+
>
> -#
> -# This test is essentially the same as that in the ufunc test; see that
> -# test for the rationale.
> -#
> -script | tee /dev/fd/2 | grep 'bash`[a-zA-Z_]' > /dev/null
> -status=$?
> -
> +script >& $tmpfile
> kill $child
> +
> +# Check that some non-static function in bash is doing work.
> +status=0
> +if ! grep -q 'bash`[a-zA-Z_]' $tmpfile; then
> + echo ERROR: expected bash functions are missing
> + status=1
> +fi
> +
> +# Check that symbols are unique. (Exclude shared libraries and unresolved addresses.)
> +if awk '!/^ *lib/ && !/^ *0x/ {print $1}' $tmpfile | sort | uniq -c | grep -qv " 1 "; then
> + echo ERROR: duplicate usym
> + status=1
> +fi
> +
> +if [ $status -ne 0 ]; then
> + cat $tmpfile
> +fi
> exit $status
> --
> 2.18.4
>
>
More information about the DTrace-devel
mailing list