Re: [LTP] thermal: add new test group

Re: [LTP] thermal: add new test group

[linuxtestproject.agent]

Hi Piotr,

On Tue, 28 Apr 2026, Piotr Kubaj wrote:
> thermal: add new test group

> +# Copyright (c) 2025, Intel Corporation. All rights reserved.

Year should be 2026 (new file).

[...]

> +	FILE *fp = SAFE_FOPEN("/proc/interrupts", "r");
> [...]
> +			if (ptr == endptr)
> +				tst_brk(TBROK, "CPU %d: interrupt not found", i);
> +
> +			if (errno == ERANGE)
> +				tst_brk(TBROK, "CPU %d: interrupt out of range", i);

Both tst_brk() calls fire before SAFE_FCLOSE(fp). Close fp before calling
tst_brk() to avoid leaking the file descriptor.

[...]

> +		sleep(sleep_time--);

Sleep-based synchronization is not allowed (G2). Replace with a polling
loop using short yields or TST_RETRY_FUNC.

---
Note:

Our agent completed the review of the patch. The full review can be
found at: https://github.com/linux-test-project/ltp-agent/actions/runs/25059182102

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[linuxtestproject.agent]

Hi Piotr,

On Thu, 30 Apr 2026 10:42:40 +0200, Piotr Kubaj wrote:
> [PATCH] thermal: add new test group

> +# Copyright (c) 2025, Intel Corporation. All rights reserved.

Wrong year in the new Makefile -- should be 2026.

> +	while (sleep_time > 0) {
> +		[...]
> +		tst_reap_children();
> +		[...]
> +		sleep(sleep_time--);

sleep() must not be used for synchronization (G2). The children are
already reaped with tst_reap_children(); this sleep is waiting for a
hardware thermal state change. Use a polling loop with
tst_timer_start()/tst_timer_expired_ms() instead, which you already
use in cpu_workload().

---
Note:

Our agent completed the review of the patch.

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[Kubaj, Piotr]

2026-04-30 (木) の 09:18 +0000 に linuxtestproject.agent@gmail.com
さんは書きました:
> Hi Piotr,
> 
> On Thu, 30 Apr 2026 10:42:40 +0200, Piotr Kubaj wrote:
> > [PATCH] thermal: add new test group
> 
> > +# Copyright (c) 2025, Intel Corporation. All rights reserved.
> 
> Wrong year in the new Makefile -- should be 2026.
> 
> > +	while (sleep_time > 0) {
> > +		[...]
> > +		tst_reap_children();
> > +		[...]
> > +		sleep(sleep_time--);
> 
> sleep() must not be used for synchronization (G2). The children are
> already reaped with tst_reap_children(); this sleep is waiting for a
> hardware thermal state change. Use a polling loop with
> tst_timer_start()/tst_timer_expired_ms() instead, which you already
> use in cpu_workload().
This sleep is not used for synchronization, but to wait until the CPU
cools down. There are some preproduction CPU's that have non-
functioning thermal sensors and may be damaged if this sleep is
omitted.

While I could use tst_timer_start()/tst_timer_expired_ms() instead, I
find it quite pointless. cpu_workload() uses it, because there I want
the whole loop to take no more than run_time. Here, I just want to wait
sleep_time.

> 
> ---
> Note:
> 
> Our agent completed the review of the patch.
> 
> The agent can sometimes produce false positives although often its
> findings are genuine. If you find issues with the review, please
> comment this email or ignore the suggestions.
> 
> Regards,
> LTP AI Reviewer
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v19] thermal: add new test group

[Piotr Kubaj]

Currently consists of only one test for the CPU package thermal sensor
interface for Intel platforms.
It works by checking the initial count of thermal interrupts. Then it
decreases the threshold for sending a thermal interrupt to just above
the current temperature and runs a workload on the CPU. Finally, it
restores the original thermal threshold and checks whether the number
of thermal interrupts increased.

Signed-off-by: Piotr Kubaj <piotr.kubaj@intel.com>
---
Correct year in the Makefile.
 runtest/thermal                               |   3 +
 testcases/kernel/Makefile                     |   1 +
 testcases/kernel/thermal/.gitignore           |   1 +
 testcases/kernel/thermal/Makefile             |   9 +
 .../kernel/thermal/thermal_interrupt_events.c | 242 ++++++++++++++++++
 5 files changed, 256 insertions(+)
 create mode 100644 runtest/thermal
 create mode 100644 testcases/kernel/thermal/.gitignore
 create mode 100644 testcases/kernel/thermal/Makefile
 create mode 100644 testcases/kernel/thermal/thermal_interrupt_events.c

diff --git a/runtest/thermal b/runtest/thermal
new file mode 100644
index 000000000..57e3d29f8
--- /dev/null
+++ b/runtest/thermal
@@ -0,0 +1,3 @@
+# Thermal driver API
+# https://docs.kernel.org/driver-api/thermal/
+thermal_interrupt_events thermal_interrupt_events
diff --git a/testcases/kernel/Makefile b/testcases/kernel/Makefile
index 98fd45a9d..ac816e4e8 100644
--- a/testcases/kernel/Makefile
+++ b/testcases/kernel/Makefile
@@ -36,6 +36,7 @@ SUBDIRS			+= connectors \
 			   sched \
 			   security \
 			   sound \
+			   thermal \
 			   tracing \
 			   uevents \
 			   watchqueue \
diff --git a/testcases/kernel/thermal/.gitignore b/testcases/kernel/thermal/.gitignore
new file mode 100644
index 000000000..1090bdad8
--- /dev/null
+++ b/testcases/kernel/thermal/.gitignore
@@ -0,0 +1 @@
+thermal_interrupt_events
diff --git a/testcases/kernel/thermal/Makefile b/testcases/kernel/thermal/Makefile
new file mode 100644
index 000000000..eda3a64f1
--- /dev/null
+++ b/testcases/kernel/thermal/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2026, Intel Corporation. All rights reserved.
+# Author:Piotr Kubaj <piotr.kubaj@intel.com>
+
+top_srcdir             ?= ../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/thermal/thermal_interrupt_events.c b/testcases/kernel/thermal/thermal_interrupt_events.c
new file mode 100644
index 000000000..1f24c3ae4
--- /dev/null
+++ b/testcases/kernel/thermal/thermal_interrupt_events.c
@@ -0,0 +1,242 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2026 Intel - http://www.intel.com/
+ */
+
+/*\
+ * Tests the CPU package thermal sensor interface for Intel platforms.
+
+ * Works by checking the initial count of thermal interrupts. Then it
+ * decreases the threshold for sending a thermal interrupt to just above
+ * the current temperature and runs a workload on the CPU. Finally, it restores
+ * the original thermal threshold and checks whether the number of thermal
+ * interrupts increased.
+ */
+
+#include <ctype.h>
+#include <inttypes.h>
+#include "tst_safe_stdio.h"
+#include "tst_test.h"
+#include "tst_timer_test.h"
+
+#define	TEST_RUNTIME	3
+#define	RUNTIME		30
+#define	SLEEP		10
+#define	TEMP_INCREMENT	10
+
+static bool x86_pkg_temp_tz_found, *x86_pkg_temp_tz;
+static char temp_path[PATH_MAX], trip_path[PATH_MAX];
+static int nproc, temp_high, temp, *trip_orig, tz_counter;
+static uint64_t *interrupt_init, *interrupt_later;
+
+static void read_interrupts(uint64_t *interrupts)
+{
+	bool interrupts_found = false;
+	char line[8192];
+
+	memset(interrupts, 0, nproc * sizeof(*interrupts));
+	FILE *fp = SAFE_FOPEN("/proc/interrupts", "r");
+
+	while (fgets(line, sizeof(line), fp)) {
+		if (!strstr(line, "Thermal event interrupts"))
+			continue;
+
+		interrupts_found = true;
+		char *ptr = strchr(line, ':');
+
+		for (int i = 0; i < nproc; i++) {
+			char *endptr;
+
+			while (*ptr && !isdigit(*ptr))
+				ptr++;
+
+			errno = 0;
+
+			interrupts[i] = strtoull(ptr, &endptr, 10);
+
+			if (ptr == endptr)
+				tst_brk(TBROK, "CPU %d: interrupt not found", i);
+
+			if (errno == ERANGE)
+				tst_brk(TBROK, "CPU %d: interrupt out of range", i);
+
+			ptr = endptr;
+			tst_res(TDEBUG, "interrupts[%d]: %" PRIu64, i, interrupts[i]);
+		}
+		break;
+	}
+	SAFE_FCLOSE(fp);
+	if (!interrupts_found)
+		tst_brk(TCONF, "No Thermal event interrupts line in /proc/interrupts");
+}
+
+static void setup(void)
+{
+	char line[8192];
+
+	nproc = tst_ncpus();
+	tst_set_runtime(nproc * TEST_RUNTIME);
+
+	tst_res(TDEBUG, "Number of logical cores: %d", nproc);
+	interrupt_init = SAFE_CALLOC(nproc, sizeof(uint64_t));
+	interrupt_later = SAFE_CALLOC(nproc, sizeof(uint64_t));
+
+	DIR *dir = SAFE_OPENDIR("/sys/class/thermal/");
+	struct dirent *entry;
+
+	while ((entry = SAFE_READDIR(dir))) {
+		if ((!strncmp(entry->d_name, "thermal_zone", sizeof("thermal_zone") - 1)))
+			tz_counter++;
+	}
+	SAFE_CLOSEDIR(dir);
+	tst_res(TDEBUG, "Found %d thermal zone(s)", tz_counter);
+
+	x86_pkg_temp_tz = SAFE_CALLOC(tz_counter, sizeof(bool));
+	trip_orig = SAFE_CALLOC(tz_counter, sizeof(int));
+
+	for (int i = 0; i < tz_counter; i++) {
+		char path[PATH_MAX];
+
+		snprintf(path, PATH_MAX, "/sys/class/thermal/thermal_zone%d/type", i);
+		tst_res(TDEBUG, "Checking whether %s is x86_pkg_temp", path);
+
+		SAFE_FILE_SCANF(path, "%8191s", line);
+		if (strstr(line, "x86_pkg_temp")) {
+			tst_res(TDEBUG, "Thermal zone %d uses x86_pkg_temp", i);
+			x86_pkg_temp_tz[i] = true;
+			x86_pkg_temp_tz_found = true;
+			snprintf(trip_path, PATH_MAX, "/sys/class/thermal/thermal_zone%d/trip_point_1_temp", i);
+			SAFE_ACCESS(trip_path, R_OK | W_OK);
+			SAFE_FILE_SCANF(trip_path, "%d", &trip_orig[i]);
+		}
+	}
+
+	if (!x86_pkg_temp_tz_found)
+		tst_brk(TCONF, "No thermal zone uses x86_pkg_temp");
+}
+
+static void cpu_workload(double run_time)
+{
+	tst_timer_start(CLOCK_MONOTONIC);
+	int num = 2;
+
+	while (!tst_timer_expired_ms(run_time * 1000)) {
+		for (int i = 2; i * i <= num; i++) {
+			if (num % i == 0)
+				break;
+		}
+		num++;
+		SAFE_FILE_SCANF(temp_path, "%d", &temp);
+
+		if (temp > temp_high)
+			break;
+	}
+}
+
+static void test_zone(int i)
+{
+	int sleep_time = SLEEP;
+	double run_time = RUNTIME;
+
+	snprintf(temp_path, PATH_MAX, "/sys/class/thermal/thermal_zone%d/temp", i);
+	tst_res(TINFO, "Testing %s", temp_path);
+	SAFE_FILE_SCANF(temp_path, "%d", &temp);
+	if (temp < 0)
+		tst_brk(TBROK, "Unexpected zone temperature value %d", temp);
+
+	tst_res(TDEBUG, "Current temperature for %s: %d", temp_path, temp);
+
+	temp_high = temp + TEMP_INCREMENT;
+
+	snprintf(trip_path, PATH_MAX, "/sys/class/thermal/thermal_zone%d/trip_point_1_temp", i);
+
+	tst_res(TDEBUG, "Setting new trip_point_1_temp value: %d", temp_high);
+	SAFE_FILE_PRINTF(trip_path, "%d", temp_high);
+
+	while (sleep_time > 0) {
+		tst_res(TDEBUG, "Running for %f seconds, then sleeping for %d seconds", run_time, sleep_time);
+
+		for (int j = 0; j < nproc; j++) {
+			if (!SAFE_FORK()) {
+				cpu_workload(run_time);
+				exit(0);
+			}
+		}
+
+		tst_reap_children();
+
+		SAFE_FILE_SCANF(temp_path, "%d", &temp);
+		tst_res(TDEBUG, "Temperature for %s after a test: %d", temp_path, temp);
+
+		if (temp > temp_high)
+			break;
+		sleep(sleep_time--);
+		run_time -= 3;
+	}
+}
+
+static void cleanup(void)
+{
+	if (x86_pkg_temp_tz_found) {
+		for (int i = 0; i < tz_counter; i++) {
+			if (x86_pkg_temp_tz[i]) {
+				snprintf(trip_path, PATH_MAX, "/sys/class/thermal/thermal_zone%d/trip_point_1_temp", i);
+				SAFE_FILE_PRINTF(trip_path, "%d", trip_orig[i]);
+			}
+		}
+	}
+
+	free(x86_pkg_temp_tz);
+	free(interrupt_init);
+	free(interrupt_later);
+	free(trip_orig);
+}
+
+static void run(void)
+{
+	for (int i = 0; i < tz_counter; i++) {
+		if (x86_pkg_temp_tz[i]) {
+			read_interrupts(interrupt_init);
+			test_zone(i);
+			read_interrupts(interrupt_later);
+
+			bool interrupt_increased = false;
+			for (int j = 0; j < nproc; j++) {
+				if (interrupt_later[j] > interrupt_init[j]) {
+					interrupt_increased = true;
+					tst_res(TINFO, "CPU %d interrupt counter: %" PRIu64 " (previous: %" PRIu64 ")",
+						j, interrupt_later[j], interrupt_init[j]);
+				}
+			}
+
+			if (!interrupt_increased)
+				tst_res(TFAIL, "No thermal interrupt increase detected");
+			else if (temp <= temp_high)
+				tst_res(TFAIL, "Thermal interrupts increased but temperature did not rise as expected");
+			else
+				tst_res(TPASS, "x86 package thermal interrupt triggered");
+		}
+	}
+}
+
+static struct tst_test test = {
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.needs_kconfigs = (const char *const []) {
+		"CONFIG_X86_PKG_TEMP_THERMAL",
+		NULL
+	},
+	.min_runtime = 5,
+	.needs_root = 1,
+	.setup = setup,
+	.supported_archs = (const char *const []) {
+		"x86",
+		"x86_64",
+		NULL
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "9635c586a559ba0e45b2bfbff79c937ddbaf1a62"},
+		{}
+	},
+	.test_all = run
+};
-- 
2.47.3

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[linuxtestproject.agent]

Hi Piotr,

On Wed, 6 May 2026 13:24:21 +0200, Piotr Kubaj wrote:
> [PATCH] thermal: add new test group

> +/*\
> + * Tests the CPU package thermal sensor interface for Intel platforms.
> +
> + * Works by checking the initial count of thermal interrupts. Then it

The blank line separating the two paragraphs must carry the " *" prefix,
i.e. " *" on its own, not an empty line.

> +		if (temp > temp_high)
> +			break;
> +		sleep(sleep_time--);
> +		run_time -= 3;

sleep() for synchronisation is not allowed (LTP ground rule G2). Replace
with a polling loop using tst_timer_start()/tst_timer_expired_ms(), or
remove the sleep and let the workload loop duration do the pacing.

---
Note:

Our agent completed the review of the patch.

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[Andrea Cervesato via ltp]

Hi Piotr,

you can ignore the AI agent review in this case. We already
talked about these issues and the patch looks ok to me and
others. Hopefully we can merge this soon.

Acked-by: Andrea Cervesato <andrea.cervesato@suse.com>

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v19] thermal: add new test group

[Petr Vorel]

Hi Piotr,

> Currently consists of only one test for the CPU package thermal sensor
> interface for Intel platforms.
> It works by checking the initial count of thermal interrupts. Then it
> decreases the threshold for sending a thermal interrupt to just above
> the current temperature and runs a workload on the CPU. Finally, it
> restores the original thermal threshold and checks whether the number
> of thermal interrupts increased.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

While I consider this ready and Andrea already acked this (I really want to get
this merged before LTP new release in the end of May), let's wait for Cyril's
ack (you're on his TODO list).

Thanks for your work and patience.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v19] thermal: add new test group

[Cyril Hrubis]

Hi!
> +static void setup(void)
> +{
> +	char line[8192];
> +
> +	nproc = tst_ncpus();
> +	tst_set_runtime(nproc * TEST_RUNTIME);

The last thing that I do not think is correct is this part. We are
setting the runtime here based on the number of CPUs and not based on
the number of actuall test iterations.

I guess that we need to:

  Turn the x86_pkg_temp_tz_found into a counter and collect the number
  of thermal zones found and set the runtime at the end of the setup
  with the counter instead.

And at the same time the TEST_RUNTIME appears to be significantly
smaller than the actual runtime.

  The upper bound for the runtime is a sum of all possible iterations,
  if we simplify it a bit it would be roughly ((RUNTIME+SLEEPTIME) *
  (SLEEPTIME/2))

  As we are doing SLEEPTIME iterations with SLEEPTIME and RUNTIME
  getting smaller in each of them so it will average out to SLEEPTIME/2
  iterations with full (RUNTIME+SLEEPTIME)

  So I guess that we need:

  #define TEST_RUNTIME ((RUNTIME+SLEEPTIME) * (SLEEPTIME/2))


The rest looks good to me.

With the runtime calculation fixed:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v19] thermal: add new test group

[Petr Vorel]

> Hi!
> > +static void setup(void)
> > +{
> > +	char line[8192];
> > +
> > +	nproc = tst_ncpus();
> > +	tst_set_runtime(nproc * TEST_RUNTIME);

> The last thing that I do not think is correct is this part. We are
> setting the runtime here based on the number of CPUs and not based on
> the number of actuall test iterations.

> I guess that we need to:

>   Turn the x86_pkg_temp_tz_found into a counter and collect the number
>   of thermal zones found and set the runtime at the end of the setup
>   with the counter instead.

> And at the same time the TEST_RUNTIME appears to be significantly
> smaller than the actual runtime.

>   The upper bound for the runtime is a sum of all possible iterations,
>   if we simplify it a bit it would be roughly ((RUNTIME+SLEEPTIME) *
>   (SLEEPTIME/2))

>   As we are doing SLEEPTIME iterations with SLEEPTIME and RUNTIME
>   getting smaller in each of them so it will average out to SLEEPTIME/2
>   iterations with full (RUNTIME+SLEEPTIME)

>   So I guess that we need:

>   #define TEST_RUNTIME ((RUNTIME+SLEEPTIME) * (SLEEPTIME/2))

And other thing which is on my machine broken is to run with more iterations, it
randomly fails or get stuck after first iteration:

./thermal_interrupt_events -i3
thermal_interrupt_events.c:207: TINFO: CPU 23 interrupt counter: 23 (previous: 21)
thermal_interrupt_events.c:217: TPASS: x86 package thermal interrupt triggered
thermal_interrupt_events.c:142: TINFO: Testing /sys/class/thermal/thermal_zone9/temp
thermal_interrupt_events.c:213: TFAIL: No thermal interrupt increase detected

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v19] thermal: add new test group

[Cyril Hrubis]

Hi!
> And other thing which is on my machine broken is to run with more iterations, it
> randomly fails or get stuck after first iteration:
> 
> ./thermal_interrupt_events -i3
> thermal_interrupt_events.c:207: TINFO: CPU 23 interrupt counter: 23 (previous: 21)
> thermal_interrupt_events.c:217: TPASS: x86 package thermal interrupt triggered
> thermal_interrupt_events.c:142: TINFO: Testing /sys/class/thermal/thermal_zone9/temp
> thermal_interrupt_events.c:213: TFAIL: No thermal interrupt increase detected

That may as well be because the CPU heatsink accumulated too much heat
and cannot increase it further. Interacting with the physical world
outside is a bit tricky. The test may need a cooldown period
(e.g. sleep(300)) at the end of the test iteration.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[linuxtestproject.agent]

Hi Piotr,

On 2026-05-14, Piotr Kubaj wrote:
> thermal: add new test group

> +static void run(void)
> +{
> +	static bool first_zone = true;
> +	...
> +		if (!first_zone) {
> +			tst_res(TINFO, "Cooling down for %d seconds", COOLDOWN);
> +			sleep(COOLDOWN);
> +		}
> +		first_zone = false;

The `static` qualifier means `first_zone` is initialized only once, at
program start. On the second iteration (`-i 2`), it will already be
`false`, causing the first zone to incur an unnecessary 300-second
cooldown. Drop `static` so it resets to `true` on every call.

[...]

---
Note:

Our agent completed the review of the patch. The agent can sometimes
produce false positives although often its findings are genuine. If you
find issues with the review, please comment this email or ignore the
suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[linuxtestproject.agent]

Hi Piotr,

On Thu, 14 May 2026 15:34:47 +0200, Piotr Kubaj wrote:
> thermal: add new test group

> +	while (sleep_time > 0) {
> +		...
> +		if (temp > temp_high)
> +			break;
> +		sleep(sleep_time--);

G2: sleep() used for synchronization. This is not a timer API test so
the G2 exemption does not apply. Consider polling with
tst_timer_expired_ms() or discuss with maintainers if a thermal-specific
exception is appropriate.

> +			tst_res(TINFO, "Cooling down for %d seconds", COOLDOWN);
> +			sleep(COOLDOWN);

G2: Same issue — sleep() used as a synchronization barrier for CPU
cooldown between zone tests. If a non-sleep alternative is genuinely
infeasible for thermal testing, this needs explicit justification in the
commit message.

---
Note:

Our agent completed the review of the patch. The agent can sometimes
produce false positives although often its findings are genuine. If you
find issues with the review, please comment this email or ignore the
suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[Petr Vorel]

Hi Piotr,

> Hi Piotr,

> On 2026-05-14, Piotr Kubaj wrote:
> > thermal: add new test group

> > +static void run(void)
> > +{
> > +	static bool first_zone = true;
> > +	...
> > +		if (!first_zone) {
> > +			tst_res(TINFO, "Cooling down for %d seconds", COOLDOWN);
> > +			sleep(COOLDOWN);
> > +		}
> > +		first_zone = false;

> The `static` qualifier means `first_zone` is initialized only once, at
> program start. On the second iteration (`-i 2`), it will already be
> `false`, causing the first zone to incur an unnecessary 300-second
> cooldown. Drop `static` so it resets to `true` on every call.

AI is wrong, that was actually the point. IMHO this v20 is better than following
v21. I just wonder if 300 sec isn't too much.

Also, it can still fail if one runs test more times without iterations. But I'm
ok with this known issue.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

If I get Cyril's ack, I'll merge this version with minor formatting diff.

Kind regards,
Petr

diff --git testcases/kernel/thermal/thermal_interrupt_events.c testcases/kernel/thermal/thermal_interrupt_events.c
index b282180314..9d9d9d7e4c 100644
--- testcases/kernel/thermal/thermal_interrupt_events.c
+++ testcases/kernel/thermal/thermal_interrupt_events.c
@@ -201,7 +201,7 @@ static void run(void)
 	for (int i = 0; i < tz_counter; i++) {
 		if (x86_pkg_temp_tz[i]) {
 			if (!first_zone) {
-				tst_res(TINFO, "Cooling down for %d seconds", COOLDOWN);
+				tst_res(TINFO, "Cooling down for %d sec", COOLDOWN);
 				sleep(COOLDOWN);
 			}
 			first_zone = false;
@@ -215,8 +215,8 @@ static void run(void)
 			for (int j = 0; j < nproc; j++) {
 				if (interrupt_later[j] > interrupt_init[j]) {
 					interrupt_increased = true;
-					tst_res(TINFO, "CPU %d interrupt counter: %" PRIu64 " (previous: %" PRIu64 ")",
-						j, interrupt_later[j], interrupt_init[j]);
+					tst_res(TINFO, "CPU %d interrupt counter: %" PRIu64 " => %" PRIu64,
+						j, interrupt_init[j], interrupt_later[j]);
 				}
 			}
 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] thermal: add new test group

[Petr Vorel]

Hi all,

actually v20 is better than this v21 => setting it changes requested.
I'd prefer to merge v20.

> Hi Piotr,

> On Thu, 14 May 2026 15:34:47 +0200, Piotr Kubaj wrote:
> > thermal: add new test group

> > +	while (sleep_time > 0) {
> > +		...
> > +		if (temp > temp_high)
> > +			break;
> > +		sleep(sleep_time--);

> G2: sleep() used for synchronization. This is not a timer API test so
> the G2 exemption does not apply. Consider polling with
> tst_timer_expired_ms() or discuss with maintainers if a thermal-specific
> exception is appropriate.

> > +			tst_res(TINFO, "Cooling down for %d seconds", COOLDOWN);
> > +			sleep(COOLDOWN);

> G2: Same issue — sleep() used as a synchronization barrier for CPU
> cooldown between zone tests. If a non-sleep alternative is genuinely
> infeasible for thermal testing, this needs explicit justification in the
> commit message.

Yes, cooldown could have been mentioned in the commit message, I'll add it for
v20.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp