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 <[email protected]> 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. > 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. [ ... ] > 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/[email protected]?part=7
