All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v9] cpufreq.c: add new test for cpufreq sysfs interface validation
@ 2026-06-11 11:42 Piotr Kubaj
  2026-06-11 12:32 ` [LTP] " linuxtestproject.agent
  2026-07-31 14:57 ` [LTP] [PATCH v9] " Cyril Hrubis
  0 siblings, 2 replies; 4+ messages in thread
From: Piotr Kubaj @ 2026-06-11 11:42 UTC (permalink / raw)
  To: ltp; +Cc: helena.anna.dubel, tomasz.ossowski, rafael.j.wysocki,
	daniel.niestepski

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.

Signed-off-by: Piotr Kubaj <piotr.kubaj@intel.com>
---
Addresses review from Andrea:
1. Single goto instead of duplicating free().
2. .save_restore for intel_pstate/status and intel_pstate/no_turbo.
3. TCONF is necessary, because even with the driver loaded, the sysfs
interface may not be available - e.g. when running on a VM, when using
unsupported CPU or when intel_pstate=disable is appended to the kernel
cmdline.
4. Replace strstr.
5. Use single quotes instead of escaping double quotes.
6. Add CONFIG_CPU_FREQ_GOV_SCHEDUTIL and CONFIG_CPU_FREQ_GOV_PERFORMANCE
to required kconfigs.
 runtest/power_management_tests               |   1 +
 testcases/kernel/power_management/.gitignore |   1 +
 testcases/kernel/power_management/cpufreq.c  | 424 +++++++++++++++++++
 3 files changed, 426 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..11896258e
--- /dev/null
+++ b/testcases/kernel/power_management/cpufreq.c
@@ -0,0 +1,424 @@
+// 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, setup_done;
+static char (*previous_scaling_governor)[16];
+static int nproc;
+static long *previous_scaling_max_freq, *previous_scaling_min_freq;
+
+static int governor_available(const char *governors, const char *governor)
+{
+	const char *ptr = governors;
+	int len = strlen(governor);
+
+	while (*ptr) {
+		while (*ptr == ' ' || *ptr == '\t' || *ptr == '\n')
+			ptr++;
+
+		if (!strncmp(ptr, governor, len) &&
+		    (ptr[len] == '\0' || ptr[len] == ' ' || ptr[len] == '\t' || ptr[len] == '\n'))
+			return 1;
+
+		while (*ptr && *ptr != ' ' && *ptr != '\t' && *ptr != '\n')
+			ptr++;
+	}
+
+	return 0;
+}
+
+static void cleanup(void)
+{
+	char path[PATH_MAX];
+
+	if (!setup_done)
+		goto out;
+
+	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]);
+
+		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]);
+	}
+
+out:
+	free(online);
+	free(previous_scaling_max_freq);
+	free(previous_scaling_min_freq);
+	free(previous_scaling_governor);
+}
+
+static void setup(void)
+{
+	char path[PATH_MAX];
+
+	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;
+	}
+
+	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_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]);
+	}
+
+	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 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 (!strncmp(contents, "intel_pstate", sizeof("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));
+		SAFE_FILE_SCANF(path, "%255[^\n]", contents);
+
+		tst_res(TDEBUG, "Checking whether %s contains 'performance' and 'powersave'", path);
+		if (governor_available(contents, "performance") && governor_available(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 (!strncmp(contents, "performance", sizeof("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 (!strncmp(contents, "powersave", sizeof("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 (!strncmp(contents, "powersave", sizeof("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 (!strncmp(contents, "performance", sizeof("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 (!strncmp(contents, "active", sizeof("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 (!strncmp(contents, "passive", sizeof("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 (!strncmp(contents, "active", sizeof("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));
+		SAFE_FILE_SCANF(path, "%255[^\n]", contents);
+
+		tst_res(TDEBUG, "Checking whether %s contains 'performance' and 'schedutil'", path);
+		if (governor_available(contents, "performance") && governor_available(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 (!strncmp(contents, "performance", sizeof("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 (!strncmp(contents, "schedutil", sizeof("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",
+		"CONFIG_CPU_FREQ_GOV_SCHEDUTIL",
+		"CONFIG_CPU_FREQ_GOV_PERFORMANCE",
+		NULL
+	},
+	.needs_root = 1,
+	.save_restore = (const struct tst_path_val[]) {
+		{"/sys/devices/system/cpu/intel_pstate/status", NULL, TST_SR_TCONF},
+		{"/sys/devices/system/cpu/intel_pstate/no_turbo", NULL, TST_SR_TCONF},
+		{}
+	},
+	.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

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

* Re: [LTP] cpufreq.c: add new test for cpufreq sysfs interface validation
  2026-06-11 11:42 [LTP] [PATCH v9] cpufreq.c: add new test for cpufreq sysfs interface validation Piotr Kubaj
@ 2026-06-11 12:32 ` linuxtestproject.agent
  2026-06-11 13:05   ` Kubaj, Piotr
  2026-07-31 14:57 ` [LTP] [PATCH v9] " Cyril Hrubis
  1 sibling, 1 reply; 4+ messages in thread
From: linuxtestproject.agent @ 2026-06-11 12:32 UTC (permalink / raw)
  To: Piotr Kubaj; +Cc: ltp

Hi Piotr,

On Thu, 11 Jun 2026 13:42:20 +0200, Piotr Kubaj wrote:
> cpufreq.c: add new test for cpufreq sysfs interface validation

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

The frequency write-back checks throughout run() use one-directional
comparisons. Here the test writes cpuinfo_max_freq to scaling_max_freq
and reads it back, but only fails when the read value is *higher*
than what was written. If the kernel rounded the value *down* (or the
write silently failed to a lower value), the test would still report
TPASS with "Successfully set up".

The same pattern applies to the scaling_min_freq checks further down,
which only fail when the read value is *lower* than what was written.

Since the purpose of these checks is to validate that exact boundary
values can be written, would it not be more correct to use != instead
of < / > here?

> +		if (cpuinfo_max_freq < scaling_min_freq) {

Same issue: only catches upward deviations.

> +		if (cpuinfo_min_freq > scaling_min_freq) {

Same issue: only catches downward deviations.

> +		if (cpuinfo_min_freq > scaling_max_freq) {

Same issue: only catches downward deviations.

Verdict: Needs revision

---
Note:

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] 4+ messages in thread

* Re: [LTP] cpufreq.c: add new test for cpufreq sysfs interface validation
  2026-06-11 12:32 ` [LTP] " linuxtestproject.agent
@ 2026-06-11 13:05   ` Kubaj, Piotr
  0 siblings, 0 replies; 4+ messages in thread
From: Kubaj, Piotr @ 2026-06-11 13:05 UTC (permalink / raw)
  To: linuxtestproject.agent@gmail.com; +Cc: ltp@lists.linux.it

The current check is correct. This is exactly the way our architect
suggested. This is because the values written by the user are not
written raw, but kernel uses them only as a suggestion and can be lower
or higher than specified, but need to always abide the values of
scaling_min_freq and scaling_max_freq - which is exactly what this test
checks.

2026-06-11 (木) の 12:32 +0000 に linuxtestproject.agent@gmail.com
さんは書きました:
> Hi Piotr,
> 
> On Thu, 11 Jun 2026 13:42:20 +0200, Piotr Kubaj wrote:
> > cpufreq.c: add new test for cpufreq sysfs interface validation
> 
> > +		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);
> > +		}
> 
> The frequency write-back checks throughout run() use one-directional
> comparisons. Here the test writes cpuinfo_max_freq to
> scaling_max_freq
> and reads it back, but only fails when the read value is *higher*
> than what was written. If the kernel rounded the value *down* (or the
> write silently failed to a lower value), the test would still report
> TPASS with "Successfully set up".
> 
> The same pattern applies to the scaling_min_freq checks further down,
> which only fail when the read value is *lower* than what was written.
> 
> Since the purpose of these checks is to validate that exact boundary
> values can be written, would it not be more correct to use != instead
> of < / > here?
> 
> > +		if (cpuinfo_max_freq < scaling_min_freq) {
> 
> Same issue: only catches upward deviations.
> 
> > +		if (cpuinfo_min_freq > scaling_min_freq) {
> 
> Same issue: only catches downward deviations.
> 
> > +		if (cpuinfo_min_freq > scaling_max_freq) {
> 
> Same issue: only catches downward deviations.
> 
> Verdict: Needs revision
> 
> ---
> Note:
> 
> 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] 4+ messages in thread

* Re: [LTP] [PATCH v9] cpufreq.c: add new test for cpufreq sysfs interface validation
  2026-06-11 11:42 [LTP] [PATCH v9] cpufreq.c: add new test for cpufreq sysfs interface validation Piotr Kubaj
  2026-06-11 12:32 ` [LTP] " linuxtestproject.agent
@ 2026-07-31 14:57 ` Cyril Hrubis
  1 sibling, 0 replies; 4+ messages in thread
From: Cyril Hrubis @ 2026-07-31 14:57 UTC (permalink / raw)
  To: Piotr Kubaj
  Cc: daniel.niestepski, tomasz.ossowski, helena.anna.dubel,
	rafael.j.wysocki, ltp

Hi!
I've renamed the test to cpufreq_intel.c since it's intel specific and
the name was too generic and pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

end of thread, other threads:[~2026-07-31 14:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-11 11:42 [LTP] [PATCH v9] cpufreq.c: add new test for cpufreq sysfs interface validation Piotr Kubaj
2026-06-11 12:32 ` [LTP] " linuxtestproject.agent
2026-06-11 13:05   ` Kubaj, Piotr
2026-07-31 14:57 ` [LTP] [PATCH v9] " Cyril Hrubis

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.