Thread (28 messages) flat view 28 messages, 2 authors, 3d ago

Re: [PATCH v6 07/18] tracing/remotes: selftests: Add a test for the dmesg tracefs file

From: sashiko-bot@kernel.org
Date: 2026-08-17 14:05:15

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Unconditional write to the newly introduced `dmesg` tracefs file in `setup_remote()` breaks existing remote tests on older kernels lacking this feature.
- [Medium] The dmesg test implicitly depends on dmesg timestamps and silently fails if they are disabled.
--
--- Patch [7]: [PATCH v6 07/18] tracing/remotes: selftests: Add a test for the dmesg tracefs file ---
commit f74a3f691a6b4375c6156c109a1d6336670de341
Author: Vincent Donnefort [off-list ref]

tracing/remotes: selftests: Add a test for the dmesg tracefs file

This commit adds a new selftest to exercise the newly introduced dmesg
tracefs file. It checks the ability to turn on and off dmesg redirection.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/ftrace/test.d/remotes/dmesg.tc b/tools/testing/selftests/ftrace/test.d/remotes/dmesg.tc
new file mode 100644
index 0000000000000..74f801a5e39ea
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/remotes/dmesg.tc
[ ... ]
+    sleep 1
+    output=$(mktemp $TMPDIR/remote_test.XXXXXX)
+    dmesg | awk -v ts=$last_ts '
+    /^\[ *[0-9]+\.[0-9]+\]/ {
+        t=$0; sub(/^\[[ \t]*/, "", t); sub(/].*/, "", t);
+        if (t+0 > ts+0) print $0
+    }' | grep "selftest id=" | sed 's/^[^]]*] //'> $output
[Severity: Medium]
Does this implicitly depend on dmesg timestamps being enabled?

If the kernel is configured with CONFIG_PRINTK_TIME=n, or if the user-space
dmesg utility suppresses timestamps by default, the output won't have the
[ X.Y] prefix.

The awk script strictly expects this format and drops lines that do not
match the prefix. This would result in an empty temporary file. Because
the script executes with set -e, an empty file might cause the subsequent
check_trace validation to abort the test.

[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/ftrace/test.d/remotes/functions b/tools/testing/selftests/ftrace/test.d/remotes/functions
index 05224fac3653d..4a14aa72fdf09 100644
--- a/tools/testing/selftests/ftrace/test.d/remotes/functions
+++ b/tools/testing/selftests/ftrace/test.d/remotes/functions
@@ -8,6 +8,7 @@ setup_remote()
 
 	cd remotes/$name/
 	echo 0 > tracing_on
+	echo 0 > dmesg
[Severity: Medium]
Will this unconditional write break existing remote tests when run on older
kernels?

Since this test suite frequently runs with set -e enabled in the caller
scripts, if the dmesg tracefs file does not exist on an older kernel, this
echo command will fail with a "No such file or directory" error and abort
the script.

Could we check if the dmesg file exists before attempting to write to it?
 	clear_trace
 	echo 7 > buffer_size_kb
 	echo 0 > events/enable
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817135517.3919534-1-vdonnefort@google.com?part=7
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help