On Mon, 2026-06-08 at 00:13 +0800, [email protected] wrote:
> From: Wen Yang <[email protected]>
>
> Add selftest coverage for the tlob uprobe monitoring interface under
> tools/testing/selftests/verification/.
>
> test.d/tlob/ contains both the helper sources (tlob_target, tlob_sym)
> and the seven test scripts so the test suite is self-contained.
> tlob_target provides busy-spin, sleep, and preempt workloads;
> tlob_sym
> resolves ELF symbol offsets for uprobe registration.
>
> Seven test scripts exercise uprobe binding management, budget
> violation
> detection, and per-state time accounting (running_ns, waiting_ns,
> sleeping_ns).
>
> Signed-off-by: Wen Yang <[email protected]>
Tests look fine and coverage is good, thanks!
Minor comments follow.
> ---
> .../testing/selftests/verification/.gitignore | 2 +
> tools/testing/selftests/verification/Makefile | 19 +-
> .../verification/test.d/tlob/Makefile | 20 ++
> .../verification/test.d/tlob/test.d/functions | 1 +
> .../verification/test.d/tlob/tlob_sym.c | 189
> ++++++++++++++++++
> .../verification/test.d/tlob/tlob_target.c | 138 +++++++++++++
> .../verification/test.d/tlob/uprobe_bind.tc | 37 ++++
> .../test.d/tlob/uprobe_detail_running.tc | 51 +++++
> .../test.d/tlob/uprobe_detail_sleeping.tc | 50 +++++
> .../test.d/tlob/uprobe_detail_waiting.tc | 66 ++++++
Not sure if this would work, but just to lower the maintenance burden,
couldn't we put these 3 in the same test case? You could define a bash
function and pass "running", "sleeping" or "waiting" and whether to launch the
hog to that.
Only waiting uses a taskset and a slightly different ordering, but wouldn't
they all work fine like that?
...
> a/tools/testing/selftests/verification/test.d/tlob/tlob_sym.c
> b/tools/testing/selftests/verification/test.d/tlob/tlob_sym.c
> new file mode 100644
> index 000000000000..1b7ba1c6d95b
> --- /dev/null
> +++ b/tools/testing/selftests/verification/test.d/tlob/tlob_sym.c
> @@ -0,0 +1,189 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * tlob_sym.c - ELF symbol-to-file-offset utility for tlob selftests
> + *
> + * Usage: tlob_sym sym_offset <binary> <symbol>
> + *
> + * Prints the ELF file offset of <symbol> in <binary> to stdout.
> + *
> + * Exit: 0 = found, 1 = error / not found.
> + */
I wonder if instead of maintaining a pure C solution we couldn't live
with something like:
sym_offset() { # target symbol
readelf -W -S -s $1 | awk -v symbol="$2" '
{ gsub(/\[ /, "[") } # normalise section markers
$1 ~ /^\[[0-9]+\]$/ { sections[$1]="0x"$4; offsets[$1]="0x"$5 }
$1 ~ /^[0-9]+:$/ && $NF == symbol { addr="0x"$2; sec="["$7"]" }
END { printf "printf \"0x%%x\\n\" $((%s - %s + %s))\n", addr,
sections[sec], offsets[sec] }
' | sh
}
...
> diff --git
> a/tools/testing/selftests/verification/test.d/tlob/tlob_target.c
> b/tools/testing/selftests/verification/test.d/tlob/tlob_target.c
> new file mode 100644
> index 000000000000..0fdbc575d71d
> --- /dev/null
> +++ b/tools/testing/selftests/verification/test.d/tlob/tlob_target.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * tlob_target.c - uprobe target binary for tlob selftests.
> + *
> + * Provides three start/stop probe pairs, each designed to exercise
> a
> + * different dominant component of the detail_env_tlob ns breakdown:
> + *
> + * tlob_busy_work / tlob_busy_work_done - busy-spin:
> running_ns dominates
> + * tlob_sleep_work / tlob_sleep_work_done - nanosleep:
> sleeping_ns dominates
> + * tlob_preempt_work / tlob_preempt_work_done - busy-spin:
> waiting_ns dominates
> + * (needs an RT
In short tlob_preempt_work is the same as tlob_busy_work, isn't it? Do we need
them both? Cannot you just have a hog in the test and keep using the same
function?
...
> +
> + do {
> + if (strcmp(mode, "sleep") == 0)
> + tlob_sleep_work(200);
> + else if (strcmp(mode, "preempt") == 0)
> + tlob_preempt_work(200);
> + else
> + tlob_busy_work(200 * 1000000UL);
The only difference I see is that you multiply by 1000000UL here for busy and
in the function for preempt.
Cannot we make them all consistent (call with 200 and do the math inside)?
> diff --git
> a/tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc
> b/tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc
> new file mode 100644
> index 000000000000..1ac3db6ca7bb
> --- /dev/null
> +++ b/tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc
> @@ -0,0 +1,37 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# description: Test tlob monitor uprobe binding (visible in monitor
> file, removable, duplicate rejected)
> +# requires: tlob:monitor
> +
> +RV_BINDIR="${RV_BINDIR:-$(realpath "$(dirname "${1:-$0}")")}"
> +UPROBE_TARGET="${RV_BINDIR}/tlob_target"
> +TLOB_SYM="${RV_BINDIR}/tlob_sym"
> +[ -x "$UPROBE_TARGET" ] || exit_unsupported
> +[ -x "$TLOB_SYM" ] || exit_unsupported
If those aren't ready, the build system didn't work, I don't think we need to
check here, it's just a clear error.
> +TLOB_MONITOR=monitors/tlob/monitor
> +
> +busy_offset=$("$TLOB_SYM" sym_offset "$UPROBE_TARGET" tlob_busy_work
> 2>/dev/null)
> +stop_offset=$("$TLOB_SYM" sym_offset "$UPROBE_TARGET"
> tlob_busy_work_done 2>/dev/null)
> +[ -n "$busy_offset" ] || exit_unsupported
> +[ -n "$stop_offset" ] || exit_unsupported
Kind of the same here, the rest of the test should probably fail (EINVAL in the
monitor or whatever). The script will print everything with set -x and it
should be clear what was missing.
> +command -v chrt > /dev/null || exit_unsupported
> +command -v taskset > /dev/null || exit_unsupported
Not sure how common it is not to have those, but this is exactly what the
:program under requires: is for (see rv_wwnr_printk with stress-ng).
Thanks,
Gabriele
