All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v5] power_management: rewrite runpwtests04.sh in C
@ 2026-06-12 17:13 Jinseok Kim
  2026-06-12 19:09 ` [LTP] " linuxtestproject.agent
  2026-06-15  8:21 ` [LTP] [PATCH v5] " Andrea Cervesato via ltp
  0 siblings, 2 replies; 3+ messages in thread
From: Jinseok Kim @ 2026-06-12 17:13 UTC (permalink / raw)
  To: ltp

As part of the ongoing effort to reduce shell-based tests in LTP,
rewrite the cpuidle sysfs smoke test in C using the modern LTP test API.

The new implementation preserves the original test semantics while
removing shell dependencies.

Signed-off-by: Jinseok Kim <always.starving0@gmail.com>
---
Changes in v5:
- Remove runpwtests04.sh
- Link to v4: https://lore.kernel.org/ltp/20260612122045.14962-1-always.starving0@gmail.com

Changes in v4:
- Fix patch application failure reported by CI.
- Link to v3: https://lore.kernel.org/ltp/20260611145911.3752-1-always.starving0@gmail.com

Changes in v3:
- Replace SAFE_OPEN() with open() to convert ENOENT to TCONF.
- Add a cleanup function.
- Link to v2: https://lore.kernel.org/ltp/20260524154221.2064-1-always.starving0@gmail.com

Changes in v2:
- Update runtest entry
- Clarify commit message
- Link to v1: https://lore.kernel.org/ltp/20260516200015.12689-1-always.starving0@gmail.com
---
 runtest/power_management_tests                |  2 +-
 testcases/kernel/power_management/.gitignore  |  1 +
 testcases/kernel/power_management/pwtests01.c | 67 +++++++++++++++++++
 .../kernel/power_management/runpwtests04.sh   | 58 ----------------
 4 files changed, 69 insertions(+), 59 deletions(-)
 create mode 100644 testcases/kernel/power_management/pwtests01.c
 delete mode 100755 testcases/kernel/power_management/runpwtests04.sh

diff --git a/runtest/power_management_tests b/runtest/power_management_tests
index 4da57ee72..7a31cde36 100644
--- a/runtest/power_management_tests
+++ b/runtest/power_management_tests
@@ -1,5 +1,5 @@
 #POWER_MANAGEMENT
 high_freq_hwp_cap_cppc high_freq_hwp_cap_cppc
+pwtests01 pwtests01
 runpwtests03 runpwtests03.sh
-runpwtests04 runpwtests04.sh
 runpwtests06 runpwtests06.sh
diff --git a/testcases/kernel/power_management/.gitignore b/testcases/kernel/power_management/.gitignore
index 03f0c83e4..e07cda0c5 100644
--- a/testcases/kernel/power_management/.gitignore
+++ b/testcases/kernel/power_management/.gitignore
@@ -1 +1,2 @@
 high_freq_hwp_cap_cppc
+pwtests01
diff --git a/testcases/kernel/power_management/pwtests01.c b/testcases/kernel/power_management/pwtests01.c
new file mode 100644
index 000000000..ae4ed0651
--- /dev/null
+++ b/testcases/kernel/power_management/pwtests01.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2026 Jinseok Kim <always.starving0@gmail.com>
+ */
+
+/*\
+ * Basic cpuidle sysfs smoke test.
+ *
+ * Verify that selected cpuidle sysfs files are readable.
+ */
+
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "tst_test.h"
+
+#define CPUIDLE_PATH "/sys/devices/system/cpu/cpuidle"
+
+static struct tcase {
+	const char *name;
+} tcases[] = {
+	{ "current_governor_ro" },
+	{ "current_driver" },
+};
+
+static int fd = -1;
+
+static void verify_cpuidle(unsigned int i)
+{
+	char path[PATH_MAX];
+	char buf[32];
+
+	snprintf(path, sizeof(path), "%s/%s", CPUIDLE_PATH, tcases[i].name);
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0) {
+		if (errno == ENOENT)
+			tst_res(TCONF, "%s not available", path);
+		else
+			tst_res(TFAIL | TERRNO, "open(%s) failed", path);
+		return;
+	}
+
+	SAFE_READ(0, fd, buf, sizeof(buf));
+	SAFE_CLOSE(fd);
+
+	tst_res(TPASS, "%s read successfully", path);
+}
+
+static void setup(void)
+{
+	if (access(CPUIDLE_PATH, R_OK))
+		tst_brk(TCONF, "%s is not available", CPUIDLE_PATH);
+}
+
+static void cleanup(void)
+{
+	if (fd != -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = verify_cpuidle,
+};
diff --git a/testcases/kernel/power_management/runpwtests04.sh b/testcases/kernel/power_management/runpwtests04.sh
deleted file mode 100755
index 6565320d2..000000000
--- a/testcases/kernel/power_management/runpwtests04.sh
+++ /dev/null
@@ -1,58 +0,0 @@
-#! /bin/sh
-#
-# Copyright (c) International Business Machines  Corp., 2001
-# Author: Nageswara R Sastry <nasastry@in.ibm.com>
-#
-# This program is free software;  you can redistribute it and#or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
-# or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
-# for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program;  if not, write to the Free Software Foundation,
-# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-#
-
-export TCID="Power_Management04"
-export TST_TOTAL=1
-
-. test.sh
-. pm_include.sh
-
-check_cpuidle_sysfs_files() {
-	RC=0
-	if [ -d /sys/devices/system/cpu/cpuidle ] ; then
-		for files in current_governor_ro current_driver
-		do
-			cat /sys/devices/system/cpu/cpuidle/${files} \
-				>/dev/null 2>&1
-			if [ $? -ne 0 ] ; then
-				echo "${0}: FAIL: cat ${files}"
-				RC=1
-			fi
-		done
-	fi
-	if [ ${RC} -eq 0 ] ; then
-		echo "${0}: PASS: Checking cpu idle sysfs files"
-	else
-		echo "${0}: FAIL: Checking cpu idle sysfs files"
-	fi
-	return $RC
-}
-
-# Checking test environment
-check_arch
-
-# Checking cpuidle sysfs interface files
-if check_cpuidle_sysfs_files ; then
-	tst_resm TPASS "CPUIDLE sysfs tests passed"
-else
-    tst_resm TFAIL "CPUIDLE sysfs tests failed"
-fi
-
-tst_exit
--
2.43.0

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

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

* Re: [LTP] power_management: rewrite runpwtests04.sh in C
  2026-06-12 17:13 [LTP] [PATCH v5] power_management: rewrite runpwtests04.sh in C Jinseok Kim
@ 2026-06-12 19:09 ` linuxtestproject.agent
  2026-06-15  8:21 ` [LTP] [PATCH v5] " Andrea Cervesato via ltp
  1 sibling, 0 replies; 3+ messages in thread
From: linuxtestproject.agent @ 2026-06-12 19:09 UTC (permalink / raw)
  To: Jinseok Kim; +Cc: ltp

Hi Jinseok,

On Sat, 13 Jun 2026 02:13:46 +0900, Jinseok Kim wrote:
> power_management: rewrite runpwtests04.sh in C

> The new implementation preserves the original test semantics while
> removing shell dependencies.

This claim is inaccurate in at least two ways:

1. The original shell script calls check_arch from pm_include.sh,
which gates the test to x86/x86_64 and reports TCONF on other
architectures. The new C test drops this gate entirely, relying
on runtime sysfs presence instead. That is arguably better
(runtime detection over compile-time assumptions), but it is a
semantic change that should be mentioned.

2. When /sys/devices/system/cpu/cpuidle does not exist, the
original script skips the loop and falls through to PASS. The
new test calls tst_brk(TCONF) in setup. Again an improvement,
but not "preserved semantics".

Consider rewording the commit body to acknowledge these
intentional behavioral changes rather than claiming equivalence.

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

* Re: [LTP] [PATCH v5] power_management: rewrite runpwtests04.sh in C
  2026-06-12 17:13 [LTP] [PATCH v5] power_management: rewrite runpwtests04.sh in C Jinseok Kim
  2026-06-12 19:09 ` [LTP] " linuxtestproject.agent
@ 2026-06-15  8:21 ` Andrea Cervesato via ltp
  1 sibling, 0 replies; 3+ messages in thread
From: Andrea Cervesato via ltp @ 2026-06-15  8:21 UTC (permalink / raw)
  To: Jinseok Kim; +Cc: ltp

Hi Jinseok,

I can't understand the utility of this test honestly. We are
checking if an obsolete ABI [1] such as
/sys/devices/system/cpu/cpuidle/current_governor_ro is actually
readable. That's ok, it's still there at least. Also, we are not
checking the return value or if the file is not writable.

We also have other ABI [2] to test.

Said so, it would be better to _at least_ verify read/write and
return value of `enough` ABI inside /sys/devices/system/cpu/cpuidle/
which are not subject to drivers behaviours. In particular:

- available_governors (ro) — lists registered idle governors
- current_driver (ro) — shows the active cpuidle driver name
- current_governor (rw) — shows/sets the active idle governor
- current_governor_ro (ro) — read-only view of current governor

> +static void setup(void)
> +{
> +	if (access(CPUIDLE_PATH, R_OK))
> +		tst_brk(TCONF, "%s is not available", CPUIDLE_PATH);

Instead of this check we should use .save_restore with TST_SR_TCONF.

[1] https://www.kernel.org/doc/html/latest/admin-guide/abi-obsolete-files.html#abi-file-obsolete-sysfs-cpuidle
[2] https://www.kernel.org/doc/html/latest/admin-guide/abi-testing.html#abi-sys-devices-system-cpu-cpuidle-available-governors


Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

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

end of thread, other threads:[~2026-06-15  8:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 17:13 [LTP] [PATCH v5] power_management: rewrite runpwtests04.sh in C Jinseok Kim
2026-06-12 19:09 ` [LTP] " linuxtestproject.agent
2026-06-15  8:21 ` [LTP] [PATCH v5] " Andrea Cervesato via ltp

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.