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

[Kubaj, Piotr]

Comments inline.

2026-01-20 (火) の 20:38 +0100 に Petr Vorel さんは書きました:
> Hi Piotr,
> 
> > --- /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.sh
> You rewritten test into C :).
> thermal_interrupt_events thermal_interrupt_events
> 
> I tried to run test under root:
> 
> # ./thermal_interrupt_events
> tst_test.c:1856: TINFO: Overall timeout per run is 0h 03m 30s
> thermal_interrupt_events.c:168: TBROK: Failed to close FILE
> '/sys/class/thermal/thermal_zone1/trip_point_1_temp': EINVAL (22)
> 
> But later it worked:
> tst_test.c:2028: TINFO: LTP version: 20250930-134-g02805b05a1
> tst_test.c:2031: TINFO: Tested kernel: 6.17.12+deb14-amd64 #1 SMP
> PREEMPT_DYNAMIC Debian 6.17.12-1 (2025-12-14) x86_64
> tst_kconfig.c:88: TINFO: Parsing kernel config '/boot/config-
> 6.17.12+deb14-amd64'
> tst_test.c:1856: TINFO: Overall timeout per run is 0h 03m 30s
> thermal_interrupt_events.c:181: TPASS: x86 package thermal interrupt
> triggered
> 
> I wonder what was wrong.
Can you check if the test fails again the 1st time after a reboot? If
I'm correct, it might be a reproduction of the bug due to be fixed in
https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/patch/?id=39b1bd1613b8f73994f654988ad75a72b633f5e2

> 
> ...
> > --- /dev/null
> > +++ b/testcases/kernel/thermal/thermal_interrupt_events.c
> 
> Whole C code asks for cleanup and simplification.
> 
> > @@ -0,0 +1,195 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +/*
> > + * Copyright (C) 2025-2026 Intel - http://www.intel.com/
> > + */
> > +
> > +/*
> Please use /*\ instead of /* to get test description into our test
> catalog:
> https://linux-test-project.readthedocs.io/en/latest/users/test_catalog.html
OK, will do.

> 
> > + * 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 "tst_safe_stdio.h"
> > +#include "tst_test.h"
> > +#include <ctype.h>
> > +#include <pthread.h>
> > +#include <regex.h>
> > +#define	PATH_LEN	69
> > +#define	STRING_LEN	23
> > +
> > +static void *cpu_workload(void *arg)
> > +{
> > +	time_t start_time = time(NULL);
> > +	int num = 2;
> > +
> > +	while (difftime(time(NULL), start_time) < *(double *)arg)
> > {
> > +		for (int i = 2; i * i <= num; i++) {
> > +			if (num % i == 0)
> > +				break;
> > +		}
> > +		num++;
> > +	}
> > +	return NULL;
> > +}
> > +
> > +static void read_interrupts(uint64_t *interrupt_array, const
> > uint16_t nproc)
> > +{
> > +	bool interrupts_found = 0;
> > +	char line[8192];
> very nit: IMHO 1024 would be more than enough, but whatever.
And I wasn't even sure 8192 would be enough. The reason is that, since
it's a string, every digit is a single array element. With new 2S or 4S
systems with hundreds of cores and each interrupt being up to 2^64,
even 8192 might not be enough.

> > +
> > +	memset(interrupt_array, 0, nproc *
> > sizeof(*interrupt_array));
> > +	FILE *fp = SAFE_FOPEN("/proc/interrupts", "r");
> > +
> > +	while (fgets(line, sizeof(line), fp)) {
> > +		if (strstr(line, "Thermal event interrupts")) {
> Can't we use FILE_LINES_SCANF() or SAFE_FILE_LINES_SCANF() to instead
> of whole
> while() block to simplify?
> 
> See example code
> https://github.com/linux-test-project/ltp/tree/master/lib/newlib_tests/tst_safe_fileops.c
It's quite unclear to me. SAFE_FILE_LINES_SCANF() seems fine for
reading a single interrupt number or even multiple, but only when well
known how many cores we have. Here the number of elements in the array
is equal to the number of logical cores the system has, in my case
it's:
 TRM:       7795       7795       7795       7795       7800       7800
7797       7797       7795       7795       7886       7886       7860
7860       7863       7863       7795       7795       7795       7795
7795       7795       7795       7795   Thermal event interrupts

I tried:
SAFE_FILE_LINES_SCANF("/proc/interrupts", " TRM:%sThermal event
interrupts", line);

to fit all the numbers in "line" string and then later parse them, but
only the 1st number is read. Here we need to have all of them. 

> 
> > +			interrupts_found = 1;
> > +			char *token = strtok(line, " ");
> > +
> > +			token = strtok(NULL, " ");
> > +			int i = 0;
> > +
> > +			while (!!strncmp(token, "Thermal", 7)) {
> > +				interrupt_array[i++] =
> > atoll(token);
> > +				token = strtok(NULL, " ");
> > +				tst_res(TDEBUG, "Current value of
> > interrupt_array[%d]: %ld", i - 1, interrupt_array[i - 1]);
> > +			}
> > +		}
> > +	}
> > +	SAFE_FCLOSE(fp);
> > +	if (!interrupts_found)
> > +		tst_brk(TCONF, "No Thermal event interrupts line
> > in /proc/interrupts");
> > +}
> > +
> > +static void run(void)
> > +{
> > +	bool status = 1;
> > +	char line[8192];
> > +	const uint16_t nproc = tst_ncpus();
> I'd say just use int.
OK.

> 
> https://github.com/linux-test-project/ltp/blob/master/doc/old/C-Test-API.asciidoc#11-basic-test-structure
> 	The overall test initialization is done in the setup()
> function.
> => this would apply to tst_ncpus() result.
> 
> If there is any preliminary checking before testing (e.g. content of
> /proc/interrupts) it should be in setup() function.
> 
> => but this can be used for checking as well.
> One of these would be tst_ncpus() result.
> 
> This helps to quit early.
I know, but nproc variable is later used to launch the exactly $nproc
instances of cpu workload.

> 
> > +	uint64_t interrupt_init[nproc], interrupt_later[nproc];
> > +
> > +	tst_res(TDEBUG, "Number of logical cores: %d", nproc);
> > +	read_interrupts(interrupt_init, nproc);
> > +
> > +	DIR *dir;
> > +
> > +	dir = SAFE_OPENDIR("/sys/class/thermal/");
> > +	struct dirent *entry;
> > +	regex_t regex;
> > +	uint8_t tz_counter = 0;
> > +
> > +	if (regcomp(&regex, "thermal_zone", REG_EXTENDED) != 0)
> regex is overkill.  Because we search for
> /sys/class/thermal/thermal_zone[0-9],
> we can simply traverse dirent and check it with strncmp().
> Other option would be to use glob() but even that is IMHO too much.
> And this searching should be in setup() right? (does not change
> during test run,
> why to do it more than once when run test with -i, e.g. -i3.
OK, will fix.

> 
> > +		tst_res(TINFO | TTERRNO, "regcomp");
> > +
> > +	while ((entry = readdir(dir)) != NULL) {
> > +		if (regexec(&regex, entry->d_name, 0, NULL, 0) ==
> > 0)
> > +			tz_counter++;
> > +	}
> > +	SAFE_CLOSEDIR(dir);
> > +	regfree(&regex);
> > +	tst_res(TDEBUG, "Found %d thermal zone(s)", tz_counter);
> > +
> > +	bool x86_pkg_temp_tz[tz_counter], x86_pkg_temp_tz_found =
> > 0;
> > +
> > +	memset(x86_pkg_temp_tz, 0, sizeof(x86_pkg_temp_tz));
> > +
> > +	for (uint8_t i = 0; i < tz_counter; i++) {
> > +		char path[PATH_LEN];
> > +
> > +		snprintf(path, PATH_LEN,
> > "/sys/class/thermal/thermal_zone%d/type", i);
> > +		FILE *fp = SAFE_FOPEN(path, "r");
> You print too many TDEBUG. Why don't you print opened
> /sys/class/thermal/thermal_zone%d/type file?
In case of failure, I want the tester to be able to find the issue
without modifying the test code. Here I chose not to print it, since
this test only touches x86_pkg_temp-type zones, but following your
suggestion I will change it.


> > +
> > +		if (fgets(line, sizeof(line), fp) && strstr(line,
> > "x86_pkg_temp")) {
> Please use SAFE_FILE_SCANF() or FILE_SCANF()
> https://github.com/linux-test-project/ltp/blob/master/doc/old/C-Test-API.asciidoc#14-safe-macros
OK, will fix.

> 
> Kind regards,
> Petr
> 
> > +			tst_res(TDEBUG, "Thermal zone %d uses
> > x86_pkg_temp", i);
> > +			x86_pkg_temp_tz[i] = 1;
> > +			x86_pkg_temp_tz_found = 1;
> > +		}
> > +		SAFE_FCLOSE(fp);
> > +	}
> > +	if (!x86_pkg_temp_tz_found) {
> > +		tst_res(TINFO, "No thermal zone uses
> > x86_pkg_temp");
> > +		status = 0;
> > +	}
> ...
---------------------------------------------------------------------
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.