All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [LTP] thermal: add new test group
  2026-04-28 12:46 [LTP] [PATCH v17] " Piotr Kubaj
@ 2026-04-28 14:41 ` linuxtestproject.agent
  0 siblings, 0 replies; 10+ messages in thread
From: linuxtestproject.agent @ 2026-04-28 14:41 UTC (permalink / raw)
  To: Piotr Kubaj; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-04-30  8:42 [LTP] [PATCH v18] " Piotr Kubaj
@ 2026-04-30  9:18 ` linuxtestproject.agent
  2026-04-30 11:28   ` Kubaj, Piotr
  0 siblings, 1 reply; 10+ messages in thread
From: linuxtestproject.agent @ 2026-04-30  9:18 UTC (permalink / raw)
  To: Piotr Kubaj; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-04-30  9:18 ` [LTP] " linuxtestproject.agent
@ 2026-04-30 11:28   ` Kubaj, Piotr
  0 siblings, 0 replies; 10+ messages in thread
From: Kubaj, Piotr @ 2026-04-30 11:28 UTC (permalink / raw)
  To: linuxtestproject.agent@gmail.com; +Cc: ltp@lists.linux.it

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-05-06 11:24 [LTP] [PATCH v19] " Piotr Kubaj
@ 2026-05-06 14:16 ` linuxtestproject.agent
  2026-05-06 14:23   ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 10+ messages in thread
From: linuxtestproject.agent @ 2026-05-06 14:16 UTC (permalink / raw)
  To: Piotr Kubaj; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-05-06 14:16 ` [LTP] " linuxtestproject.agent
@ 2026-05-06 14:23   ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 10+ messages in thread
From: Andrea Cervesato via ltp @ 2026-05-06 14:23 UTC (permalink / raw)
  To: linuxtestproject.agent; +Cc: 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-05-14  9:36 [LTP] [PATCH v20] " Piotr Kubaj
@ 2026-05-14 12:00 ` linuxtestproject.agent
  2026-05-15 14:39   ` Petr Vorel
  0 siblings, 1 reply; 10+ messages in thread
From: linuxtestproject.agent @ 2026-05-14 12:00 UTC (permalink / raw)
  To: Piotr Kubaj; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [LTP] [PATCH v21] thermal: add new test group
@ 2026-05-14 13:34 Piotr Kubaj
  2026-05-14 14:19 ` [LTP] " linuxtestproject.agent
  0 siblings, 1 reply; 10+ messages in thread
From: Piotr Kubaj @ 2026-05-14 13:34 UTC (permalink / raw)
  To: ltp; +Cc: helena.anna.dubel, tomasz.ossowski, rafael.j.wysocki,
	daniel.niestepski

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>
---
Drop static from first_zone.
 runtest/thermal                               |   3 +
 testcases/kernel/Makefile                     |   1 +
 testcases/kernel/thermal/.gitignore           |   1 +
 testcases/kernel/thermal/Makefile             |   9 +
 .../kernel/thermal/thermal_interrupt_events.c | 253 ++++++++++++++++++
 5 files changed, 267 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..42e1532f6
--- /dev/null
+++ b/testcases/kernel/thermal/thermal_interrupt_events.c
@@ -0,0 +1,253 @@
+// 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	((RUNTIME + SLEEPTIME) * (SLEEPTIME / 2) + COOLDOWN)
+#define	RUNTIME		30
+#define	SLEEPTIME	10
+#define	COOLDOWN	300
+#define	TEMP_INCREMENT	10
+
+static bool *x86_pkg_temp_tz;
+static char temp_path[PATH_MAX], trip_path[PATH_MAX];
+static int x86_pkg_temp_tz_found, 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_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++;
+			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");
+
+	tst_set_runtime(x86_pkg_temp_tz_found * TEST_RUNTIME);
+}
+
+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 = SLEEPTIME;
+	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)
+{
+	bool first_zone = true;
+
+	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);
+				sleep(COOLDOWN);
+			}
+			first_zone = false;
+
+			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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-05-14 13:34 [LTP] [PATCH v21] thermal: add new test group Piotr Kubaj
@ 2026-05-14 14:19 ` linuxtestproject.agent
  2026-05-15 14:42   ` Petr Vorel
  0 siblings, 1 reply; 10+ messages in thread
From: linuxtestproject.agent @ 2026-05-14 14:19 UTC (permalink / raw)
  To: Piotr Kubaj; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-05-14 12:00 ` [LTP] " linuxtestproject.agent
@ 2026-05-15 14:39   ` Petr Vorel
  0 siblings, 0 replies; 10+ messages in thread
From: Petr Vorel @ 2026-05-15 14:39 UTC (permalink / raw)
  To: linuxtestproject.agent; +Cc: ltp

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [LTP] thermal: add new test group
  2026-05-14 14:19 ` [LTP] " linuxtestproject.agent
@ 2026-05-15 14:42   ` Petr Vorel
  0 siblings, 0 replies; 10+ messages in thread
From: Petr Vorel @ 2026-05-15 14:42 UTC (permalink / raw)
  To: linuxtestproject.agent; +Cc: ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-05-15 14:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14 13:34 [LTP] [PATCH v21] thermal: add new test group Piotr Kubaj
2026-05-14 14:19 ` [LTP] " linuxtestproject.agent
2026-05-15 14:42   ` Petr Vorel
  -- strict thread matches above, loose matches on Subject: below --
2026-05-14  9:36 [LTP] [PATCH v20] " Piotr Kubaj
2026-05-14 12:00 ` [LTP] " linuxtestproject.agent
2026-05-15 14:39   ` Petr Vorel
2026-05-06 11:24 [LTP] [PATCH v19] " Piotr Kubaj
2026-05-06 14:16 ` [LTP] " linuxtestproject.agent
2026-05-06 14:23   ` Andrea Cervesato via ltp
2026-04-30  8:42 [LTP] [PATCH v18] " Piotr Kubaj
2026-04-30  9:18 ` [LTP] " linuxtestproject.agent
2026-04-30 11:28   ` Kubaj, Piotr
2026-04-28 12:46 [LTP] [PATCH v17] " Piotr Kubaj
2026-04-28 14:41 ` [LTP] " linuxtestproject.agent

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.