On Mon, Aug 17, 2026 at 02:05:14PM +0000, [email protected] wrote: > 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.
I guess I could check for /sys/module/printk/parameters/time ... > > [ ... ] > > > 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 -- Vincent
