[PATCH v2] rt-tests: hwlatdetect: Restore tracing parameters

[PATCH v2] rt-tests: hwlatdetect: Restore tracing parameters

[Costa Shulyupin]

hwlatdetect leaves tracing parameters changed after a run. These
parameters can affect later tests. When --cpu-list is omitted,
hwlatdetect reuses the existing tracing_cpumask.

Save the CPU mask, current tracer, threshold, window and width before
changing them. Restore them during cleanup, including after SIGTERM or
an unhandled exception. Leave tracing disabled, as before.

Add a shell test for successful and failing runs, SIGTERM handling,
--cpu-list changes.

Assisted-by: Codex:GPT-6
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>

---
v2:
Restore tracing parameters unconditionally
as suggested by Crystal Wood and Bart Wensley
---
 src/hwlatdetect/hwlatdetect.8       |  2 ++
 src/hwlatdetect/hwlatdetect.py      | 18 +++++++++-
 src/hwlatdetect/test_hwlatdetect.sh | 52 +++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100755 src/hwlatdetect/test_hwlatdetect.sh

diff --git a/src/hwlatdetect/hwlatdetect.8 b/src/hwlatdetect/hwlatdetect.8
index 560ff7cc0cc9..0a0c531502f9 100644
--- a/src/hwlatdetect/hwlatdetect.8
+++ b/src/hwlatdetect/hwlatdetect.8
@@ -57,6 +57,8 @@ The hwlatdetector script manages the mounting/unmounting of the
 debugfs as well as interacting with the ftrace hwlatdetector
 If the debugfs is already mounted then hwlatdetector will not
 unmount it after a run.
+The previous CPU mask, tracer, threshold, window, and width are
+restored after the test.
 
 .SH OPTIONS
 .TP
diff --git a/src/hwlatdetect/hwlatdetect.py b/src/hwlatdetect/hwlatdetect.py
index ee1e0171210d..0e9e3568f5ee 100755
--- a/src/hwlatdetect/hwlatdetect.py
+++ b/src/hwlatdetect/hwlatdetect.py
@@ -8,9 +8,11 @@
 # (C) 2009 Clark Williams <williams@redhat.com>
 
 import abc
+import atexit
 import errno
 import os
 import os.path
+import signal
 import subprocess
 import sys
 import time
@@ -183,6 +185,7 @@     def c_states_on(self):
         """ close the file descriptor and re-enable c-states """
         if self.dma_latency_handle:
             os.close(self.dma_latency_handle)
+            self.dma_latency_handle = None
             debug("c-states enabled")
 
     @abc.abstractmethod
@@ -323,6 +326,10 @@     def __init__(self):
         self.samples = []
         self.first = None
         self.last = None
+        fields = ("cpumask", "current_tracer", "threshold", "window", "width")
+        self.saved = {f: self.get(f).strip() for f in fields}
+        atexit.register(self.cleanup)
+        signal.signal(signal.SIGTERM, lambda sig, _: sys.exit(128 + sig))
         self.set("enable", 0)
         self.set('current_tracer', 'hwlat')
 
@@ -388,7 +395,15 @@     def display(self):
 
     def cleanup(self):
         self.set("tracing_on", "0")
-        self.set("current_tracer", "nop")
+        self.c_states_on()
+        debug("Restoring previous configuration")
+        old = self.saved
+        self.set("current_tracer", old["current_tracer"])
+        self.set("width", 0) # must be always width <= window
+        self.set("window", old["window"])
+        self.set("width", old["width"])
+        self.set("cpumask", old["cpumask"])
+        self.set("threshold", old["threshold"])
         if not self.debugfs.umount():
             raise RuntimeError("Failed to unmount debugfs")
 
@@ -606,4 +621,5 @@     def cleanup(self):
         detect.display()
 
     detect.cleanup()
+    atexit.unregister(detect.cleanup)
     sys.exit(maxlatency > hardlimit)
diff --git a/src/hwlatdetect/test_hwlatdetect.sh b/src/hwlatdetect/test_hwlatdetect.sh
new file mode 100755
index 000000000000..187536f95264
--- /dev/null
+++ b/src/hwlatdetect/test_hwlatdetect.sh
@@ -0,0 +1,52 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+set -o errexit
+
+out=
+trap 'echo -e "FAIL: ${BASH_SOURCE[0]}:${LINENO}: $BASH_COMMAND -> $?\noutput:\n$out" >&2' ERR
+
+[[ $(id -u) == 0 ]]
+
+d=$(dirname -- "${BASH_SOURCE[0]}")
+
+tracing=/sys/kernel/debug/tracing
+mask=$tracing/tracing_cpumask
+settings=("$mask" "$tracing/current_tracer" "$tracing/tracing_thresh"
+	"$tracing/hwlat_detector/window" "$tracing/hwlat_detector/width")
+all=$(PYTHONPATH="$d" python3 -B -c '
+from hwlatdetect import group_sep
+m = 0
+for part in open("/sys/devices/system/cpu/online").read().strip().split(","):
+    a, *b = map(int, part.split("-"))
+    for i in range(a, (b[0] if b else a) + 1):
+        m |= 1 << i
+print(group_sep(f"{m:x}"))
+')
+
+out=$("$d/hwlatdetect.py" --duration 1 --hardlimit 999 2>&1)
+[[ $out == *"test finished"* ]]
+
+! out=$("$d/hwlatdetect.py" --duration 1 --threshold 0 --hardlimit 0 2>&1)
+[ $? -eq 0 ]
+[[ $out == *"test finished"* ]]
+
+echo $all > $mask
+baseline=$(cat "${settings[@]}")
+
+! out=$("$d/hwlatdetect.py" --cpu-list 1 --duration 1 2>&1)
+[[ $out == *"test finished"* ]]
+[[ $(cat "${settings[@]}") == "$baseline" ]]
+
+rc=0
+out=$(timeout --signal=TERM 2 \
+	"$d/hwlatdetect.py" --cpu-list 1 --duration 60 2>&1) || rc=$?
+[[ $rc -eq 124 ]]
+[[ $out == *"Starting test"* ]]
+[[ $(cat "${settings[@]}") == "$baseline" ]]
+
+! out=$("$d/hwlatdetect.py" --cpu-list 1 --duration 1 2>&1)
+[[ $out == *"test finished"* ]]
+[[ $(cat "${settings[@]}") == "$baseline" ]]
+
+echo Passed
-- 
2.56.0

Re: [PATCH v2] rt-tests: hwlatdetect: Restore tracing parameters

[Crystal Wood]

On Mon, 2026-10-19 at 13:29 +0300, Costa Shulyupin wrote:

>  
> @@ -606,4 +621,5 @@     def cleanup(self):
>          detect.display()
>  
>      detect.cleanup()
> +    atexit.unregister(detect.cleanup)
>      sys.exit(maxlatency > hardlimit)

It's not a huge deal, but you could just remove the explicit
detect.cleanup() at leave the atexit registered.  It'd be a bit simpler
and wouldn't leave a window where cleanup() could be executed twice.

-Crystal

Re: [PATCH v2] rt-tests: hwlatdetect: Restore tracing parameters

[Sebastian Andrzej Siewior]

On 2026-10-19 13:29:09 [+0300], Costa Shulyupin wrote:
>  src/hwlatdetect/hwlatdetect.py      | 18 +++++++++-

I've been looking at this and figured out that this is just a wrapper
around the tracer that we have for this.
Could you please get rid of the DebugFS class in it?
The tracing folder should be auto-mounted at /sys/kernel/tracing by the
distribution. Currently it is also auto mounted at
/sys/kernel/debug/tracing if something is accessing this folder and this
is subject to vanish. Mounting it does not help then either.

Sebastian

Re: [PATCH v2] rt-tests: hwlatdetect: Restore tracing parameters

[Costa Shulyupin]

On Tue, 20 Oct 2026 at 13:16, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> Could you please get rid of the DebugFS class in it?
> The tracing folder should be auto-mounted at /sys/kernel/tracing by the
> distribution. Currently it is also auto mounted at
> /sys/kernel/debug/tracing if something is accessing this folder and this
> is subject to vanish. Mounting it does not help then either.

In the OpenShift/Kubernetes containers tracefs by default is at
/host/sys/kernel/tracing and /host/sys/kernel/debug/tracing.
Therefore, only the DebugFS mount and umount are redundant.

Costa

Re: [PATCH v2] rt-tests: hwlatdetect: Restore tracing parameters

[Sebastian Andrzej Siewior]

On 2026-10-20 20:39:27 [+0300], Costa Shulyupin wrote:
> On Tue, 20 Oct 2026 at 13:16, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > Could you please get rid of the DebugFS class in it?
> > The tracing folder should be auto-mounted at /sys/kernel/tracing by the
> > distribution. Currently it is also auto mounted at
> > /sys/kernel/debug/tracing if something is accessing this folder and this
> > is subject to vanish. Mounting it does not help then either.
> 
> In the OpenShift/Kubernetes containers tracefs by default is at
> /host/sys/kernel/tracing and /host/sys/kernel/debug/tracing.
> Therefore, only the DebugFS mount and umount are redundant.

Interesting.
But how does this even work without extra glue around it? Is this the
global tracefs or per-namespace tracefs?
Does this still work with CONFIG_TRACEFS_AUTOMOUNT_DEPRECATED=n ?

Be aware that this is the only tool in rt-tests that still uses the
debugfs path.

> Costa

Sebastian

Re: [PATCH v2] rt-tests: hwlatdetect: Restore tracing parameters

[Sebastian Andrzej Siewior]

On 2026-10-21 08:48:45 [+0200], To Costa Shulyupin wrote:
> On 2026-10-20 20:39:27 [+0300], Costa Shulyupin wrote:
> > On Tue, 20 Oct 2026 at 13:16, Sebastian Andrzej Siewior
> > <bigeasy@linutronix.de> wrote:
> > > Could you please get rid of the DebugFS class in it?
> > > The tracing folder should be auto-mounted at /sys/kernel/tracing by the
> > > distribution. Currently it is also auto mounted at
> > > /sys/kernel/debug/tracing if something is accessing this folder and this
> > > is subject to vanish. Mounting it does not help then either.
> > 
> > In the OpenShift/Kubernetes containers tracefs by default is at
> > /host/sys/kernel/tracing and /host/sys/kernel/debug/tracing.
> > Therefore, only the DebugFS mount and umount are redundant.
> 
> Interesting.
> But how does this even work without extra glue around it? Is this the
> global tracefs or per-namespace tracefs?
> Does this still work with CONFIG_TRACEFS_AUTOMOUNT_DEPRECATED=n ?
> 
> Be aware that this is the only tool in rt-tests that still uses the
> debugfs path.

There is src/hwlatdetect/hwlat.txt which refers to debugfs at / and more
importantly to hwlat.ko which is not a thing since commit 7c10eeecf93f4
("hwlatdetect: modify hwlatdetector.py to use the ftrace
hwlatdetector").
The whole usage paragraph looks outdated and
https://docs.kernel.org/trace/hwlat_detector.html is probably a better
source.
I don't know how accurate the remaining part of the document is. It
starts with "The module hwlat" which is not the case.  It does
refer to itself as SMI detector which was its thing before the rename.
I don't think it ever did detect SMIs the way cyclictest does (by
reading the MSR (well on x86)) it only did try to find execution gaps
the way the hwlat-traces does it today. This might be misleading.
From a quick look, the man-page looks up-to-date. It mentions at
hwlat_detector.ko at the bottom - other than that, it looks okay.

> > Costa

Sebastian