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

[Petr Vorel <pvorel@suse.cz>]

Hi Piotr, all,

thanks for your work, v5 looks better. I'd like others have look into the code.
We have LTP release this week (hopefully, or next week if there is too much
work), then I ping others to have a look.

> This is a new test for checking thermal interrupt events.
> All but one points from v4 were addressed. The one that is still
> not addressed requires changing testing algorithm and is under
> discussion with our architect.

Thanks!

...
> diff --git a/testcases/kernel/thermal/thermal_interrupt_events.c b/testcases/kernel/thermal/thermal_interrupt_events.c
> new file mode 100644
> index 000000000..b4e457434
> --- /dev/null
> +++ b/testcases/kernel/thermal/thermal_interrupt_events.c
> @@ -0,0 +1,202 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/*
> + * Copyright (C) 2025-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 "tst_safe_stdio.h"
> +#include "tst_test.h"
> +
> +#define	RUNTIME		30
> +#define	SLEEP		10
> +#define	TEMP_INCREMENT	10
> +
> +static bool x86_pkg_temp_tz_found;
> +static bool *x86_pkg_temp_tz;
> +static char trip_path[NAME_MAX];
> +static int nproc, trip, tz_counter;
> +static uint64_t *interrupt_init, *interrupt_later;
> +
> +static void interrupts(uint64_t *interrupt_array, const int nproc)

FYI in previous version I meant to rename uint64_t *interrupt_array => uint64_t
*interrupts. Function name read_interrupts() was actually more descriptive.
But that's a minor detail.


> +static void test_zone(int i)
> +{
> +			char path[NAME_MAX], temp_path[NAME_MAX];
> +			int sleep_time = SLEEP, temp_high, temp;
> +			double run_time = RUNTIME;
> +
> +			snprintf(path, NAME_MAX, "/sys/class/thermal/thermal_zone%d/", i);
> +			strncpy(temp_path, path, NAME_MAX);
> +			strncat(temp_path, "temp", 4);
> +			tst_res(TINFO, "Testing %s", temp_path);
> +			SAFE_FILE_SCANF(temp_path, "%d", &temp);
> +			if (temp < 0)
> +				tst_brk(TINFO, "Unexpected zone temperature value %d", temp);
I noted that in v4 that this should be tst_brk(TBROK, ...);
This is really worth to fix.
But please wait with next version for other reviewers, 


> +			tst_res(TDEBUG, "Current temperature for %s: %d", path, temp);
> +
> +			temp_high = temp + TEMP_INCREMENT;
> +
> +			strncpy(trip_path, path, NAME_MAX);
> +			strncat(trip_path, "trip_point_1_temp", 17);
> +
> +			tst_res(TDEBUG, "Setting new trip_point_1_temp value: %d", temp_high);
> +			SAFE_FILE_SCANF(trip_path, "%d", &trip);
> +			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", path, temp);
> +
> +				if (temp > temp_high)
> +					break;
> +				sleep(sleep_time--);
> +				run_time -= 3;
> +			}
> +
> +			if (temp <= temp_high)
> +				tst_brk(TCONF, "Zone temperature is not rising as expected");

I hope not raising Zone is not an error. Otherwise there should be
tst_brk(TBROK), or tst_res(TFAIL) if it makes sense to verify the rest of the
zones.

> +}
> +
> +static void cleanup(void)
> +{
> +	SAFE_FILE_PRINTF(trip_path, "%d", trip);
> +	free(interrupt_init);
> +	free(interrupt_later);
> +}
> +
> +static void run(void)
> +{
> +	for (int i = 0; i < tz_counter; i++) {
> +		if (x86_pkg_temp_tz[i])
> +			test_zone(i);
> +	}
> +	interrupts(interrupt_later, nproc);
> +
> +	for (int i = 0; i < nproc; i++) {
> +		if (interrupt_later[i] < interrupt_init[i])
> +			tst_res(TFAIL, "CPU %d interrupt counter: %ld (previous: %ld)",
> +				i, interrupt_later[i], interrupt_init[i]);
You need some bool flag to remember the failure. Otherwise even if this fails...
> +	}
> +
> +	tst_res(TPASS, "x86 package thermal interrupt triggered");
... you still print TPASS (a bit confusing).

Kind regards,
Petr

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