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

Petr Vorel
2026-01-20T19:39:22Z

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.

...
> --- /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

> + * 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.
> +
> + 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

> + 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.

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.

> + 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.

> + 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?
> +
> + 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

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;
> + }
...