Re: [LTP] cpufreq.c: add new test for cpufreq sysfs interface validation

Re: [LTP] cpufreq.c: add new test for cpufreq sysfs interface validation

[linuxtestproject.agent]

Hi Piotr,

On 2026-04-22, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation

> +static long *previous_scaling_max_freq, *previous_scaling_min_freq;
> +
> +static void cleanup(void)
> +{
> [...]
> +               SAFE_FILE_PRINTF(path, "%ld", previous_scaling_max_freq[i]);
> [...]
> +               SAFE_FILE_PRINTF(path, "%ld", previous_scaling_min_freq[i]);

Both arrays are zeroed by SAFE_CALLOC in setup() and only populated
inside run(). If run() aborts before reaching the per-CPU save loops
(e.g. a SAFE_OPEN or SAFE_FILE_SCANF fails on an earlier cpufreq node),
cleanup() writes 0 to scaling_max_freq and scaling_min_freq for every
online CPU, leaving the system in an invalid state.

Read the original per-CPU values in setup() instead.

Pre-existing issues noticed in the surrounding code (not introduced
by this patch):

- cpufreq.c:116 — strstr(contents, "performance powersave") assumes
  a specific governor ordering that the kernel does not guarantee.
- cpufreq.c:170 — same issue for "performance schedutil".

---
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] cpufreq.c: add new test for cpufreq sysfs interface validation

[linuxtestproject.agent]

Hi Piotr,

On Mon, 4 May 2026 12:08:42 +0200, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation

> +		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor", i);
> +		...
> +		SAFE_FILE_PRINTF(path, "powersave");

scaling_governor is modified for every online CPU (in both the active-mode
and passive-mode loops) but is never saved in setup() nor restored in
cleanup(). After the test exits each CPU is left with scaling_governor set
to "schedutil". Please save the original value per CPU in setup() alongside
previous_scaling_max_freq and restore it in cleanup().

[...]

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

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] cpufreq.c: add new test for cpufreq sysfs interface validation

[linuxtestproject.agent]

Hi Piotr,

On Tue, 05 May 2026 11:48:54 +0200, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation
>
> Runs various sanity checks for intel_pstate cpufreq sysfs interface.
>
> Checks whether intel_pstate driver is used, whether it is possible
> to switch scaling_governor to powersave and performance,
> whether it is possible to disable turbo frequency and
> whether it is possible to change scaling_min_freq and scaling_max_freq
> to its minimal valid and maximal valid values.

The body only describes what the test does, not why it is needed. Add a
sentence stating the motivation — e.g. lack of regression coverage for the
intel_pstate sysfs interface, or a specific issue that prompted this test.

Pre-existing issues noticed in the surrounding code (not introduced
by this patch):

- cpufreq.c:19 — `static char path[PATH_MAX]` used as a shared scratch
  buffer across setup/run/cleanup; a function-local buffer in run() would
  be cleaner.

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

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] cpufreq.c: add new test for cpufreq sysfs interface validation

[linuxtestproject.agent]

Hi Piotr,

On Tue, 5 May 2026 13:02:09 +0200, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation

> +/*\
> + * Runs various sanity checks for intel_pstate cpufreq sysfs interface.
> + * ...
> + */

The doc comment does not state why root is needed. Per ground-rules rule 4,
document the reason in the doc block — e.g. "Requires root to write to
/sys/devices/system/cpu/intel_pstate/ and per-CPU cpufreq sysfs attributes."

[...]

> +	SAFE_FILE_SCANF("/sys/devices/system/cpu/intel_pstate/no_turbo", "%d", &no_turbo);
> +	SAFE_FILE_SCANF("/sys/devices/system/cpu/intel_pstate/status", "%15s", intel_pstate_status);

CONFIG_X86_INTEL_PSTATE=y does not guarantee the driver is active at runtime
(acpi_cpufreq may take over, or the driver may be in "off" mode). If the
sysfs files are absent, SAFE_FILE_SCANF calls tst_brk(TBROK). Guard with
an existence check and use tst_brk(TCONF, "intel_pstate driver not active")
instead.

---
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] cpufreq.c: add new test for cpufreq sysfs interface validation

[linuxtestproject.agent]

Hi Piotr,

On Wed, 6 May 2026, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation

> +	tst_res(TDEBUG, "Checking whether scaling_available_governors contains \"performance\" and \"schedutil\" after switching /sys/devices/system/cpu/intel_pstate/status to \"passive\"");
> +	snprintf(path, sizeof(path), "/sys/devices/system/cpu/intel_pstate/status");
> +	SAFE_FILE_PRINTF(path, "passive");
> +
> +	for (int i = 0; i < nproc; i++) {
> +		...
> +		SAFE_FILE_PRINTF(path, "schedutil");
> +		...
> +	}
> +}

run() exits with intel_pstate/status still "passive" and each CPU's
scaling_governor still "schedutil". On the second iteration (-i 2) the
status check fires TFAIL immediately, and the governor branch hits
tst_brk(TBROK, "Unknown scaling_governor: schedutil").

Restore the status to "active" and the governors to previous_scaling_governor[i]
at the end of run() before it returns.

[...]

---
Note:

Our agent completed the review of the patch. The full review can be
found at: https://patchwork.ozlabs.org/project/ltp/list/?series=502943

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

[LTP] [PATCH v7] cpufreq.c: add new test for cpufreq sysfs interface validation

[Piotr Kubaj]

Runs various sanity checks for intel_pstate cpufreq sysfs interface.

Checks whether intel_pstate driver is used, whether it is possible
to switch scaling_governor to powersave and performance,
whether it is possible to disable turbo frequency and
whether it is possible to change scaling_min_freq and scaling_max_freq
to its minimal valid and maximal valid values.

Signed-off-by: Piotr Kubaj <piotr.kubaj@intel.com>
---
Address review to fix -i 2 runs.
 runtest/power_management_tests               |   1 +
 testcases/kernel/power_management/.gitignore |   1 +
 testcases/kernel/power_management/cpufreq.c  | 413 +++++++++++++++++++
 3 files changed, 415 insertions(+)
 create mode 100644 testcases/kernel/power_management/.gitignore
 create mode 100644 testcases/kernel/power_management/cpufreq.c

diff --git a/runtest/power_management_tests b/runtest/power_management_tests
index b670da6ec..0bc7dd0c8 100644
--- a/runtest/power_management_tests
+++ b/runtest/power_management_tests
@@ -1,4 +1,5 @@
 #POWER_MANAGEMENT
+cpufreq cpufreq
 runpwtests03 runpwtests03.sh
 runpwtests04 runpwtests04.sh
 runpwtests06 runpwtests06.sh
diff --git a/testcases/kernel/power_management/.gitignore b/testcases/kernel/power_management/.gitignore
new file mode 100644
index 000000000..7f34e4c2f
--- /dev/null
+++ b/testcases/kernel/power_management/.gitignore
@@ -0,0 +1 @@
+cpufreq
diff --git a/testcases/kernel/power_management/cpufreq.c b/testcases/kernel/power_management/cpufreq.c
new file mode 100644
index 000000000..07f6dec90
--- /dev/null
+++ b/testcases/kernel/power_management/cpufreq.c
@@ -0,0 +1,413 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2026 Piotr Kubaj <piotr.kubaj@intel.com>
+ */
+
+/*\
+ * Runs various sanity checks for intel_pstate cpufreq sysfs interface.
+ *
+ * Checks whether intel_pstate driver is used, whether it is possible to switch
+ * scaling_governor to powersave and performance, whether it is possible
+ * to disable turbo frequency and whether it is possible to change
+ * scaling_min_freq and scaling_max_freq to its minimal valid and maximal valid
+ * values.
+ *
+ * Requires root to write to /sys/devices/system/cpu/intel_pstate/ (status,
+ * no_turbo) and per-CPU /sys/devices/system/cpu/cpu*\/cpufreq/ attributes
+ * (scaling_governor, scaling_min_freq, scaling_max_freq).
+ *
+ * Motivation: LTP has no regression coverage for the intel_pstate cpufreq
+ * sysfs interface, so breakage in any of these attributes (driver exposure,
+ * governor switching, active/passive mode transitions, no_turbo toggling or
+ * scaling_{min,max}_freq writes) can reach users undetected. This test
+ * exercises the interface end-to-end to catch such regressions.
+ */
+
+#include "tst_test.h"
+
+static bool *online;
+static bool setup_done;
+static char intel_pstate_status[16];
+static char (*previous_scaling_governor)[16];
+static int no_turbo, nproc;
+static long *previous_scaling_max_freq, *previous_scaling_min_freq;
+
+static void cleanup(void)
+{
+	char path[PATH_MAX];
+
+	if (!setup_done) {
+		free(online);
+		free(previous_scaling_max_freq);
+		free(previous_scaling_min_freq);
+		free(previous_scaling_governor);
+		return;
+	}
+
+	SAFE_FILE_PRINTF("/sys/devices/system/cpu/intel_pstate/status", "%s", intel_pstate_status);
+	SAFE_FILE_PRINTF("/sys/devices/system/cpu/intel_pstate/no_turbo", "%d", no_turbo);
+
+	for (int i = 0; i < nproc; i++) {
+		if (!online[i])
+			continue;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor", i);
+		SAFE_FILE_PRINTF(path, "%s", previous_scaling_governor[i]);
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_max_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", previous_scaling_max_freq[i]);
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_min_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", previous_scaling_min_freq[i]);
+	}
+
+	free(online);
+	free(previous_scaling_max_freq);
+	free(previous_scaling_min_freq);
+	free(previous_scaling_governor);
+}
+
+static void setup(void)
+{
+	char path[PATH_MAX];
+
+	if (access("/sys/devices/system/cpu/intel_pstate/status", F_OK) == -1)
+		tst_brk(TCONF, "intel_pstate driver not active");
+
+	SAFE_FILE_SCANF("/sys/devices/system/cpu/intel_pstate/no_turbo", "%d", &no_turbo);
+	SAFE_FILE_SCANF("/sys/devices/system/cpu/intel_pstate/status", "%15s", intel_pstate_status);
+
+	nproc = tst_ncpus_conf();
+	online = SAFE_CALLOC(nproc, sizeof(*online));
+	previous_scaling_max_freq = SAFE_CALLOC(nproc, sizeof(*previous_scaling_max_freq));
+	previous_scaling_min_freq = SAFE_CALLOC(nproc, sizeof(*previous_scaling_min_freq));
+	previous_scaling_governor = SAFE_CALLOC(nproc, sizeof(*previous_scaling_governor));
+	online[0] = true;
+	for (int i = 1; i < nproc; i++) {
+		int tmp;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/online", i);
+		SAFE_FILE_SCANF(path, "%d", &tmp);
+		online[i] = (bool)tmp;
+	}
+	for (int i = 0; i < nproc; i++) {
+		if (!online[i])
+			continue;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor", i);
+		SAFE_FILE_SCANF(path, "%15s", previous_scaling_governor[i]);
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_max_freq", i);
+		SAFE_FILE_SCANF(path, "%ld", &previous_scaling_max_freq[i]);
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_min_freq", i);
+		SAFE_FILE_SCANF(path, "%ld", &previous_scaling_min_freq[i]);
+	}
+
+	SAFE_FILE_PRINTF("/sys/devices/system/cpu/intel_pstate/status", "active");
+
+	setup_done = true;
+}
+
+static void run(void)
+{
+	const char * const cpufreq_nodes[] = {
+		"affected_cpus",
+		"cpuinfo_max_freq",
+		"cpuinfo_min_freq",
+		"cpuinfo_transition_latency",
+		"related_cpus",
+		"scaling_available_governors",
+		"scaling_cur_freq",
+		"scaling_driver",
+		"scaling_governor",
+		"scaling_max_freq",
+		"scaling_min_freq",
+		"scaling_setspeed",
+		NULL
+	};
+	char contents[256] = {0}, path[PATH_MAX], path_cpuinfo_min_freq[PATH_MAX];
+	int fd, no_turbo_val;
+	long cpuinfo_max_freq = 0, cpuinfo_min_freq = 0, scaling_max_freq, scaling_min_freq;
+
+	for (int i = 0; i < nproc; i++) {
+		if (!online[i])
+			continue;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_driver", i);
+
+		SAFE_FILE_SCANF(path, "%255s", contents);
+		tst_res(TDEBUG, "Checking whether %s is \"intel_pstate\"", path);
+		if (strstr(contents, "intel_pstate")) {
+			tst_res(TPASS, "%s is intel_pstate", path);
+		} else {
+			tst_res(TINFO, "%s contains: %s", path, contents);
+			tst_res(TFAIL, "%s is not intel_pstate", path);
+		}
+
+		for (int j = 0; cpufreq_nodes[j]; j++) {
+			struct stat stats;
+
+			snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/%s", i, cpufreq_nodes[j]);
+			tst_res(TDEBUG, "Checking whether %s is a regular file", path);
+			SAFE_STAT(path, &stats);
+			if (!S_ISREG(stats.st_mode)) {
+				tst_res(TINFO, "%s mode: %o", path, stats.st_mode);
+				tst_res(TFAIL, "%s is not a regular file", path);
+			} else {
+				tst_res(TPASS, "%s is a regular file", path);
+			}
+		}
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_available_governors", i);
+		memset(contents, 0, sizeof(contents));
+		fd = SAFE_OPEN(path, O_RDONLY);
+		SAFE_READ(0, fd, contents, sizeof(contents));
+		SAFE_CLOSE(fd);
+
+		tst_res(TDEBUG, "Checking whether %s contains \"performance\" and \"powersave\"", path);
+		if (strstr(contents, "performance") && strstr(contents, "powersave")) {
+			tst_res(TPASS, "%s contains: %s", path, contents);
+		} else {
+			tst_res(TINFO, "%s contains: %s", path, contents);
+			tst_res(TFAIL, "%s is not \"performance powersave\"", path);
+		}
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor", i);
+		tst_res(TDEBUG, "Checking %s", path);
+		memset(contents, 0, sizeof(contents));
+		SAFE_FILE_SCANF(path, "%255s", contents);
+		if (strstr(contents, "performance")) {
+			tst_res(TDEBUG, "%s is \"performance\"", path);
+			tst_res(TDEBUG, "Checking whether %s can be switched to \"powersave\"", path);
+			SAFE_FILE_PRINTF(path, "powersave");
+			memset(contents, 0, sizeof(contents));
+			SAFE_FILE_SCANF(path, "%255s", contents);
+			if (strstr(contents, "powersave"))
+				tst_res(TPASS, "Changing scaling_governor from performance to powersave succeeded");
+			else
+				tst_res(TFAIL, "%s: failed to change scaling_governor from performance to powersave, current scaling_governor: %s", path, contents);
+
+		} else if (strstr(contents, "powersave")) {
+			tst_res(TDEBUG, "%s is \"powersave\"", path);
+			tst_res(TDEBUG, "Checking whether %s can be switched to \"performance\"", path);
+			SAFE_FILE_PRINTF(path, "performance");
+			memset(contents, 0, sizeof(contents));
+			SAFE_FILE_SCANF(path, "%255s", contents);
+			if (strstr(contents, "performance"))
+				tst_res(TPASS, "Changing scaling_governor from powersave to performance succeeded");
+			else
+				tst_res(TFAIL, "%s: failed to change scaling_governor from powersave to performance, current scaling_governor: %s", path, contents);
+
+		} else {
+			tst_brk(TBROK, "Unknown scaling_governor: %s", contents);
+		}
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq", i);
+		snprintf(path_cpuinfo_min_freq, sizeof(path_cpuinfo_min_freq), "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_min_freq", i);
+		tst_res(TDEBUG, "Checking whether %s is bigger than cpuinfo_min_freq", path);
+		SAFE_FILE_SCANF(path, "%ld", &cpuinfo_max_freq);
+
+		SAFE_FILE_SCANF(path_cpuinfo_min_freq, "%ld", &cpuinfo_min_freq);
+
+		if (cpuinfo_min_freq > cpuinfo_max_freq)
+			tst_res(TFAIL, "Value in %s: %ld, should be bigger than cpuinfo_min_freq: %ld", path, cpuinfo_max_freq, cpuinfo_min_freq);
+		else
+			tst_res(TPASS, "Value in %s: %ld, is bigger than cpuinfo_min_freq: %ld", path, cpuinfo_max_freq, cpuinfo_min_freq);
+	}
+
+	snprintf(path, sizeof(path), "/sys/devices/system/cpu/intel_pstate/status");
+	tst_res(TDEBUG, "Checking whether %s contains \"active\"", path);
+	memset(contents, 0, sizeof(contents));
+	SAFE_FILE_SCANF(path, "%255s", contents);
+
+	if (strstr(contents, "active"))
+		tst_res(TPASS, "%s is \"active\"", path);
+	else
+		tst_res(TFAIL, "%s is not \"active\", but %s", path, contents);
+
+	tst_res(TDEBUG, "Checking whether %s can be switched to \"passive\"", path);
+	SAFE_FILE_PRINTF(path, "passive");
+	memset(contents, 0, sizeof(contents));
+	SAFE_FILE_SCANF(path, "%255s", contents);
+
+	if (strstr(contents, "passive"))
+		tst_res(TPASS, "%s is \"passive\"", path);
+	else
+		tst_res(TFAIL, "%s is not \"passive\", but %s", path, contents);
+
+	tst_res(TDEBUG, "Checking whether %s can be switched back to \"active\"", path);
+	SAFE_FILE_PRINTF(path, "active");
+	memset(contents, 0, sizeof(contents));
+	SAFE_FILE_SCANF(path, "%255s", contents);
+
+	if (strstr(contents, "active"))
+		tst_res(TPASS, "%s is \"active\"", path);
+	else
+		tst_res(TFAIL, "%s is not \"active\", but %s", path, contents);
+
+	snprintf(path, sizeof(path), "/sys/devices/system/cpu/intel_pstate/no_turbo");
+	tst_res(TDEBUG, "Checking whether %s can be switched to 1", path);
+	SAFE_FILE_PRINTF(path, "1");
+	SAFE_FILE_SCANF(path, "%d", &no_turbo_val);
+
+	if (no_turbo_val == 1)
+		tst_res(TPASS, "%s is \"1\"", path);
+	else
+		tst_res(TFAIL, "%s is not \"1\", but %d", path, no_turbo_val);
+
+	tst_res(TDEBUG, "Checking whether %s can be switched back to 0", path);
+	SAFE_FILE_PRINTF(path, "0");
+	SAFE_FILE_SCANF(path, "%d", &no_turbo_val);
+
+	if (no_turbo_val == 0)
+		tst_res(TPASS, "%s is \"0\"", path);
+	else
+		tst_res(TFAIL, "%s is not \"0\", but %d", path, no_turbo_val);
+
+	tst_res(TDEBUG, "Checking whether scaling_available_governors contains \"performance\" and \"schedutil\" after switching /sys/devices/system/cpu/intel_pstate/status to \"passive\"");
+	snprintf(path, sizeof(path), "/sys/devices/system/cpu/intel_pstate/status");
+	SAFE_FILE_PRINTF(path, "passive");
+
+	for (int i = 0; i < nproc; i++) {
+		if (!online[i])
+			continue;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_available_governors", i);
+		memset(contents, 0, sizeof(contents));
+		fd = SAFE_OPEN(path, O_RDONLY);
+		SAFE_READ(0, fd, contents, sizeof(contents));
+		SAFE_CLOSE(fd);
+
+		tst_res(TDEBUG, "Checking whether %s contains \"performance\" and \"schedutil\"", path);
+		if (strstr(contents, "performance") && strstr(contents, "schedutil")) {
+			tst_res(TPASS, "%s contains: %s", path, contents);
+		} else {
+			tst_res(TINFO, "%s contains: %s", path, contents);
+			tst_res(TFAIL, "%s does not contain \"performance\" and \"schedutil\"", path);
+		}
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor", i);
+		tst_res(TDEBUG, "Checking whether %s can be changed to performance", path);
+		SAFE_FILE_PRINTF(path, "performance");
+		memset(contents, 0, sizeof(contents));
+		SAFE_FILE_SCANF(path, "%255s", contents);
+
+		if (strstr(contents, "performance"))
+			tst_res(TPASS, "%s is \"performance\"", path);
+		else
+			tst_res(TFAIL, "%s is not \"performance\", but %s", path, contents);
+
+		tst_res(TDEBUG, "Checking whether %s can be changed to schedutil", path);
+		SAFE_FILE_PRINTF(path, "schedutil");
+		memset(contents, 0, sizeof(contents));
+		SAFE_FILE_SCANF(path, "%255s", contents);
+
+		if (strstr(contents, "schedutil"))
+			tst_res(TPASS, "%s is \"schedutil\"", path);
+		else
+			tst_res(TFAIL, "%s is not \"schedutil\", but %s", path, contents);
+	}
+
+	for (int i = 0; i < nproc; i++) {
+		if (!online[i])
+			continue;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq", i);
+		tst_res(TDEBUG, "Checking whether %s can be assigned to scaling_max_freq and scaling_min_freq", path);
+		SAFE_FILE_SCANF(path, "%ld", &cpuinfo_max_freq);
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_min_freq", i);
+		SAFE_FILE_SCANF(path, "%ld", &cpuinfo_min_freq);
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_max_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", cpuinfo_max_freq);
+		SAFE_FILE_SCANF(path, "%ld", &scaling_max_freq);
+
+		if (cpuinfo_max_freq < scaling_max_freq) {
+			tst_res(TINFO, "cpuinfo_max_freq: %ld", cpuinfo_max_freq);
+			tst_res(TINFO, "scaling_max_freq: %ld", scaling_max_freq);
+			tst_res(TFAIL, "Failure setting %s", path);
+		} else {
+			tst_res(TPASS, "Successfully set up %s", path);
+		}
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_min_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", cpuinfo_max_freq);
+		SAFE_FILE_SCANF(path, "%ld", &scaling_min_freq);
+
+		if (cpuinfo_max_freq < scaling_min_freq) {
+			tst_res(TINFO, "cpuinfo_max_freq: %ld", cpuinfo_max_freq);
+			tst_res(TINFO, "scaling_min_freq: %ld", scaling_min_freq);
+			tst_res(TFAIL, "Failure setting %s", path);
+		} else {
+			tst_res(TPASS, "Successfully set up %s", path);
+		}
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_min_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", previous_scaling_min_freq[i]);
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_max_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", previous_scaling_max_freq[i]);
+	}
+
+	for (int i = 0; i < nproc; i++) {
+		if (!online[i])
+			continue;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq", i);
+		tst_res(TDEBUG, "Checking whether %s can be assigned to scaling_max_freq and scaling_min_freq", path);
+		SAFE_FILE_SCANF(path, "%ld", &cpuinfo_max_freq);
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_min_freq", i);
+		SAFE_FILE_SCANF(path, "%ld", &cpuinfo_min_freq);
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_min_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", cpuinfo_min_freq);
+		SAFE_FILE_SCANF(path, "%ld", &scaling_min_freq);
+
+		if (cpuinfo_min_freq > scaling_min_freq) {
+			tst_res(TINFO, "cpuinfo_min_freq: %ld", cpuinfo_min_freq);
+			tst_res(TINFO, "scaling_min_freq: %ld", scaling_min_freq);
+			tst_res(TFAIL, "Failure setting %s", path);
+		} else {
+			tst_res(TPASS, "Successfully set up %s", path);
+		}
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_max_freq", i);
+		SAFE_FILE_PRINTF(path, "%ld", cpuinfo_min_freq);
+		SAFE_FILE_SCANF(path, "%ld", &scaling_max_freq);
+
+		if (cpuinfo_min_freq > scaling_max_freq) {
+			tst_res(TINFO, "cpuinfo_min_freq: %ld", cpuinfo_min_freq);
+			tst_res(TINFO, "scaling_max_freq: %ld", scaling_max_freq);
+			tst_res(TFAIL, "Failure setting %s", path);
+		} else {
+			tst_res(TPASS, "Successfully set up %s", path);
+		}
+	}
+
+	SAFE_FILE_PRINTF("/sys/devices/system/cpu/intel_pstate/status", "active");
+
+	for (int i = 0; i < nproc; i++) {
+		if (!online[i])
+			continue;
+
+		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor", i);
+		SAFE_FILE_PRINTF(path, "%s", previous_scaling_governor[i]);
+	}
+}
+
+static struct tst_test test = {
+	.cleanup = cleanup,
+	.needs_kconfigs = (const char *const []) {
+		"CONFIG_X86_INTEL_PSTATE",
+		NULL
+	},
+	.needs_root = 1,
+	.setup = setup,
+	.supported_archs = (const char *const []) {
+		"x86",
+		"x86_64",
+		NULL
+	},
+	.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] cpufreq.c: add new test for cpufreq sysfs interface validation

[linuxtestproject.agent]

Hi Piotr,

On Wed, 6 May 2026, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation
>
> Runs various sanity checks for intel_pstate cpufreq sysfs interface.
> [...]

The commit body describes what the test does but not why it is needed.
The motivation ("LTP has no regression coverage for the intel_pstate
cpufreq sysfs interface...") belongs in the commit body, not only in
the in-source doc comment.

> +	fd = SAFE_OPEN(path, O_RDONLY);
> +	SAFE_READ(0, fd, contents, sizeof(contents));
> +	SAFE_CLOSE(fd);

If SAFE_READ fails it calls tst_brk(), skipping SAFE_CLOSE(). cleanup()
has no fd handling so the descriptor leaks. Use SAFE_FILE_SCANF() instead.
This pattern appears twice (~line 184 and ~line 298).

> +static bool setup_done;
[...]
> +	setup_done = true;

setup_done is never reset in cleanup(). With -i, a partial setup failure
on a later iteration sees stale 'true' and incorrectly enters the restore
path. Reset it at the start of setup() or end of cleanup().

---
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] cpufreq.c: add new test for cpufreq sysfs interface validation

[linuxtestproject.agent]

Hi Piotr,

On Thu, 7 May 2026, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation

Reviewed-by: LTP AI Reviewer <ltp-ai@noreply.github.com>

---
Note:

Our agent completed the review of the patch. The full review can be
found at: https://patchwork.ozlabs.org/project/ltp/list/?series=503158

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