Re: [PATCH v3 9/9] selftests/verification: add tlob selftests
From: Gabriele Monaco <gmonaco@redhat.com>
Date: 2026-06-17 15:09:20
Also in:
lkml
On Mon, 2026-06-08 at 00:13 +0800, wen.yang@linux.dev wrote:
From: Wen Yang <redacted> 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 <redacted>
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? ...
quoted hunk ↗ jump to hunk
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
}
...
quoted hunk ↗ jump to hunk
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 exercisea + * 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)?
quoted hunk ↗ jump to hunk
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 monitorfile, 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_unsupportedKind 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