Re: [PATCH v3 2/6] selftests/powercap: Add powercap hierarchy creation API tests

[Rafael J. Wysocki (Intel)]

On Tue, Oct 20, 2026 at 4:18 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxxxxxxxx> wrote:
>
> The powercap hierarchy API introduces generic helpers to duplicate,
> instantiate and destroy a complete powercap hierarchy from a static
> description.
>
> Add a kselftest exercising this API. The test builds a synthetic
> powercap hierarchy composed of a package, three CPU clusters and twelve
> CPUs, registers it as a powercap control type and checks the zone count
> and selected sysfs attribute values. It also checks that the controller
> directory disappears when the module is unloaded.
>
> Exercise direct static creation, duplication, skipped clusters and their
> descendants, reuse after partial destruction, and repeated creation
> returning -EBUSY without replacing existing zone pointers. Callback
> checks verify the controller, name and backend-private data supplied by
> the helpers.
>
> The test consists of:
>
> * a kernel module implementing a synthetic powercap hierarchy and
>   dummy callbacks;
> * a userspace kselftest script that loads the module, checks 16 zones
>   and their power and constraint attributes, unloads the module with an
>   attribute descriptor open, and checks controller removal.
>
> Use powercap-owned zone allocations so registration failures are cleaned
> up by the core. Build generated module files in the selftest output
> directory and extend the common clean target with Kbuild module cleanup.
>
> This provides a regression test for the powercap hierarchy helpers and
> their integration with the powercap core.
>
> Cc: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@xxxxxxxxxxxxxxxx>
> Link: https://patch.msgid.link/20260806110159.69690-3-daniel.lezcano@xxxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxxxxxxxx>

Sashiko is complaining about this one now:

https://sashiko.dev/#/patchset/cover.1792504834.git.daniel.lezcano%40oss.qualcomm.com

> ---
> Changes in v3 (since v2):
> - Rebase on v7.3-rc7.
> - Use TEST_GEN_FILES and MO=$(OUTPUT); preserve the common clean recipe.
> - Use powercap-owned allocations for consistent registration cleanup.
> - Keep validation and counting in the main shell so failures propagate.
> - Add static creation, skipped branches, reuse and -EBUSY checks, and
>   hold a sysfs descriptor open across module removal.
> - Document the tests and describe their actual assertions.
> - Fix closing-brace indentation and __initdata placement.
> - Add Assisted-by attribution.
>
>  tools/testing/selftests/Makefile              |   1 +
>  tools/testing/selftests/powercap/Kbuild       |   3 +
>  tools/testing/selftests/powercap/Makefile     |  27 ++
>  .../selftests/powercap/powercap_hierarchy.c   | 305 ++++++++++++++++++
>  .../selftests/powercap/powercap_hierarchy.sh  | 126 ++++++++
>  5 files changed, 462 insertions(+)
>  create mode 100644 tools/testing/selftests/powercap/Kbuild
>  create mode 100644 tools/testing/selftests/powercap/Makefile
>  create mode 100644 tools/testing/selftests/powercap/powercap_hierarchy.c
>  create mode 100755 tools/testing/selftests/powercap/powercap_hierarchy.sh
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index bc4fdb6f5337..764cca6182ce 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -104,6 +104,7 @@ TARGETS += pidfd
>  TARGETS += pid_namespace
>  TARGETS += pipe
>  TARGETS += power_supply
> +TARGETS += powercap
>  TARGETS += powerpc
>  TARGETS += prctl
>  TARGETS += proc
> diff --git a/tools/testing/selftests/powercap/Kbuild b/tools/testing/selftests/powercap/Kbuild
> new file mode 100644
> index 000000000000..baa0d0ba9593
> --- /dev/null
> +++ b/tools/testing/selftests/powercap/Kbuild
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-m += powercap_hierarchy.o
> \ No newline at end of file
> diff --git a/tools/testing/selftests/powercap/Makefile b/tools/testing/selftests/powercap/Makefile
> new file mode 100644
> index 000000000000..5041ff1acee5
> --- /dev/null
> +++ b/tools/testing/selftests/powercap/Makefile
> @@ -0,0 +1,27 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# User-space test script
> +TEST_PROGS := powercap_hierarchy.sh
> +
> +# Kernel module built as part of the test
> +TEST_GEN_FILES := powercap_hierarchy.ko
> +
> +KDIR ?= $(if $(O),$(O),$(realpath ../../../..))
> +
> +.DEFAULT_GOAL := all
> +
> +include ../lib.mk
> +
> +.PHONY: modules
> +
> +$(OUTPUT)/powercap_hierarchy.ko: modules
> +
> +modules:
> +       $(MAKE) -C $(KDIR) M=$(CURDIR) MO=$(OUTPUT) modules
> +
> +.PHONY: clean_modules
> +
> +clean: clean_modules
> +
> +clean_modules:
> +       $(MAKE) -C $(KDIR) M=$(CURDIR) MO=$(OUTPUT) clean
> diff --git a/tools/testing/selftests/powercap/powercap_hierarchy.c b/tools/testing/selftests/powercap/powercap_hierarchy.c
> new file mode 100644
> index 000000000000..baabd2420f9c
> --- /dev/null
> +++ b/tools/testing/selftests/powercap/powercap_hierarchy.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + *
> + * Author: Daniel Lezcano <daniel.lezcano@xxxxxxxxxxxxxxxx>
> + *
> + * Powercap hierarchy description test module
> + *
> + * Test coverage:
> + * - Duplicate an init-only description for use after module initialization.
> + * - Create and destroy a hierarchy directly from a static initializer.
> + * - Skip a cluster and its children while creating independent branches.
> + * - Destroy the partial hierarchy and create the complete hierarchy again.
> + * - Reject repeated creation with -EBUSY and preserve existing zone pointers.
> + * - Check callback arguments and preservation of backend-private data.
> + *
> + * The complete hierarchy remains registered for powercap_hierarchy.sh to
> + * check its 16 zones and exported attribute values. Module removal destroys
> + * it so the script can check cleanup with a sysfs descriptor still open.
> + */
> +#include <linux/powercap.h>
> +#include <linux/string.h>
> +
> +struct pch_test_data {
> +       int value;
> +};
> +
> +static struct powercap_node pch_test_nodes[] __initdata = {
> +       [0] = { .name = "package",
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [1] = { .name = "cluster0", .parent = &pch_test_nodes[0],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [2] = { .name = "cluster1", .parent = &pch_test_nodes[0],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [3] = { .name = "cluster2", .parent = &pch_test_nodes[0],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [4] = { .name = "cpu0", .parent = &pch_test_nodes[1],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [5] = { .name = "cpu1", .parent = &pch_test_nodes[1],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [6] = { .name = "cpu2", .parent = &pch_test_nodes[1],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [7] = { .name = "cpu3", .parent = &pch_test_nodes[1],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [8] = { .name = "cpu4", .parent = &pch_test_nodes[2],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [9] = { .name = "cpu5", .parent = &pch_test_nodes[2],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [10] = { .name = "cpu6", .parent = &pch_test_nodes[2],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [11] = { .name = "cpu7", .parent = &pch_test_nodes[2],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [12] = { .name = "cpu8", .parent = &pch_test_nodes[3],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [13] = { .name = "cpu9", .parent = &pch_test_nodes[3],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [14] = { .name = "cpu10", .parent = &pch_test_nodes[3],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +       [15] = { .name = "cpu11", .parent = &pch_test_nodes[3],
> +               .data = &(struct pch_test_data) { .value = 0xDEADBEEF } },
> +};
> +
> +static struct powercap_hierarchy pch_test_hierarchy __initdata =
> +       POWERCAP_HIERARCHY_INIT(pch_test_hierarchy, pch_test_nodes,
> +                               ARRAY_SIZE(pch_test_nodes));
> +
> +static struct powercap_hierarchy *hierarchy;
> +
> +static struct powercap_control_type *pct;
> +
> +static int pch_test_get_max_power_range_uw(struct powercap_zone *pcz,
> +                                          u64 *power_uw)
> +{
> +       *power_uw = 0xBADC0FFEE;
> +       return 0;
> +}
> +
> +static int pch_test_get_power_uw(struct powercap_zone *pcz,
> +                                u64 *power_uw)
> +{
> +       *power_uw = 0xC0FFEE;
> +       return 0;
> +}
> +
> +static const struct powercap_zone_ops pch_test_ops = {
> +       .get_max_power_range_uw = pch_test_get_max_power_range_uw,
> +       .get_power_uw = pch_test_get_power_uw,
> +};
> +
> +static int pch_test_set_power_limit_uw(struct powercap_zone *pcz,
> +                                      int cid, u64 power_uw)
> +{
> +       return 0;
> +}
> +
> +static int pch_test_get_power_limit_uw(struct powercap_zone *pcz,
> +                                      int cid, u64 *power_uw)
> +{
> +       *power_uw = 0xDEADC0DE;
> +       return 0;
> +}
> +
> +static int pch_test_set_time_window_us(struct powercap_zone *pcz,
> +                                      int cid, u64 power_uw)
> +{
> +       return 0;
> +}
> +
> +static int pch_test_get_time_window_us(struct powercap_zone *pcz,
> +                                      int cid, u64 *power_uw)
> +{
> +       *power_uw = 0xDEADC0DE;
> +       return 0;
> +}
> +
> +static int pch_test_get_max_power_uw(struct powercap_zone *pcz,
> +                                    int cid, u64 *power_uw)
> +{
> +       *power_uw = 0xDEADC0DE;
> +       return 0;
> +}
> +
> +static const char *pch_test_get_name(struct powercap_zone *pcz, int cid)
> +{
> +       return "my constraint name";
> +}
> +
> +static const struct powercap_zone_constraint_ops pch_test_constraint_ops = {
> +       .set_power_limit_uw = pch_test_set_power_limit_uw,
> +       .get_power_limit_uw = pch_test_get_power_limit_uw,
> +       .set_time_window_us = pch_test_set_time_window_us,
> +       .get_time_window_us = pch_test_get_time_window_us,
> +       .get_max_power_uw = pch_test_get_max_power_uw,
> +       .get_name = pch_test_get_name,
> +};
> +
> +static struct powercap_zone *pch_test_create(struct powercap_control_type *pct,
> +                                            const char *name, void *data,
> +                                            struct powercap_zone *parent)
> +{
> +       struct pch_test_data *pcht_data = data;
> +       struct powercap_zone *pcz;
> +
> +       /* Validate the arguments and private data supplied by the helper. */
> +       if (!pct) {
> +               pr_err("Invalid NULL controller type\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (!name) {
> +               pr_err("Invalid NULL name\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (pcht_data->value != 0xDEADBEEF) {
> +               pr_err("Invalid pcht data != 0xDEADBEEF");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       pcz = powercap_register_zone(NULL, pct, name, parent,
> +                                    &pch_test_ops, 1, &pch_test_constraint_ops);
> +       if (IS_ERR(pcz)) {
> +               pr_err("Failed to register powercap zone '%s': %ld\n",
> +                      name, PTR_ERR(pcz));
> +       }
> +
> +       return pcz;
> +}
> +
> +static void pch_test_destroy(struct powercap_control_type *pct,
> +                            struct powercap_zone *zone,
> +                            void *data)
> +{
> +       struct pch_test_data *pcht_data = data;
> +
> +       /* Destruction must receive the original private data and a valid zone. */
> +       if (!pct) {
> +               pr_err("Invalid NULL controller type\n");
> +               return;
> +       }
> +
> +       if (!zone) {
> +               pr_err("Invalid NULL zone\n");
> +               return;
> +       }
> +
> +       if (pcht_data->value != 0xDEADBEEF) {
> +               pr_err("Invalid pcht data != 0xDEADBEEF");
> +               return;
> +       }
> +
> +       powercap_unregister_zone(pct, zone);
> +}
> +
> +static struct powercap_zone *
> +pch_test_skip_cluster(struct powercap_control_type *pct, const char *name,
> +                     void *data, struct powercap_zone *parent)
> +{
> +       if (!strcmp(name, "cluster0"))
> +               return NULL;
> +
> +       return pch_test_create(pct, name, data, parent);
> +}
> +
> +static int __init pch_test_init(void)
> +{
> +       struct powercap_zone *zones[ARRAY_SIZE(pch_test_nodes)];
> +       size_t i;
> +       int ret;
> +
> +       /* Keep a runtime copy whose parent pointers survive init memory release. */
> +       hierarchy = powercap_hierarchy_dup(&pch_test_hierarchy);
> +       if (IS_ERR(hierarchy)) {
> +               ret = PTR_ERR(hierarchy);
> +               pr_err("Failed to dup the hierarchy: %d\n", ret);
> +               return ret;
> +       }
> +
> +       pct = powercap_register_control_type(NULL, "powercap-test", NULL);
> +       if (IS_ERR(pct)) {
> +               ret = PTR_ERR(pct);
> +               pr_err("Failed to register control type: %d\n", ret);
> +               goto out_free_hierarchy;
> +       }
> +
> +       /* Exercise direct creation with a statically initialized hierarchy lock. */
> +       ret = powercap_hierarchy_create(pct, &pch_test_hierarchy,
> +                                       pch_test_create, pch_test_destroy);
> +       if (ret) {
> +               pr_err("Failed to create the static hierarchy: %d\n", ret);
> +               goto out_unregister_pct;
> +       }
> +
> +       powercap_hierarchy_destroy(pct, &pch_test_hierarchy, pch_test_destroy);
> +
> +       /* NULL skips cluster0 and its descendants, but not the other clusters. */
> +       ret = powercap_hierarchy_create(pct, hierarchy,
> +                                       pch_test_skip_cluster, pch_test_destroy);
> +       if (ret)
> +               goto out_unregister_pct;
> +
> +       for (i = 0; i < hierarchy->nr_nodes; i++) {
> +               /* Node 1 is cluster0; nodes 4 through 7 are its four CPUs. */
> +               bool skipped = i == 1 || (i >= 4 && i <= 7);
> +
> +               if (!!hierarchy->nodes[i].zone == skipped) {
> +                       pr_err("Unexpected zone state after skipping cluster0\n");
> +                       ret = -EINVAL;
> +                       goto out_destroy_hierarchy;
> +               }
> +       }
> +
> +       /* Partial destruction must leave the hierarchy reusable for a full tree. */
> +       powercap_hierarchy_destroy(pct, hierarchy, pch_test_destroy);
> +
> +       ret = powercap_hierarchy_create(pct, hierarchy, pch_test_create, pch_test_destroy);
> +       if (ret) {
> +               pr_err("Failed to create the hierarchy: %d\n", ret);
> +               goto out_unregister_pct;
> +       }
> +
> +       /* A second creation must return -EBUSY without replacing any zone. */
> +       for (i = 0; i < hierarchy->nr_nodes; i++)
> +               zones[i] = hierarchy->nodes[i].zone;
> +
> +       ret = powercap_hierarchy_create(pct, hierarchy, pch_test_create, pch_test_destroy);
> +       if (ret != -EBUSY) {
> +               pr_err("Expected -EBUSY when recreating the hierarchy, got %d\n", ret);
> +               ret = -EINVAL;
> +               goto out_destroy_hierarchy;
> +       }
> +
> +       for (i = 0; i < hierarchy->nr_nodes; i++) {
> +               if (hierarchy->nodes[i].zone != zones[i]) {
> +                       pr_err("Recreation changed an existing zone\n");
> +                       ret = -EINVAL;
> +                       goto out_destroy_hierarchy;
> +               }
> +       }
> +
> +       /* Leave the full tree available for the userspace sysfs checks. */
> +       return 0;
> +
> +out_destroy_hierarchy:
> +       powercap_hierarchy_destroy(pct, hierarchy, pch_test_destroy);
> +out_unregister_pct:
> +       powercap_unregister_control_type(pct);
> +out_free_hierarchy:
> +       powercap_hierarchy_free(hierarchy);
> +       return ret;
> +}
> +module_init(pch_test_init);
> +
> +static void __exit pch_test_exit(void)
> +{
> +       /* Remove zones before their controller; the script checks disappearance. */
> +       powercap_hierarchy_destroy(pct, hierarchy, pch_test_destroy);
> +       powercap_hierarchy_free(hierarchy);
> +       powercap_unregister_control_type(pct);
> +}
> +module_exit(pch_test_exit);
> +
> +MODULE_DESCRIPTION("Powercap hierarchy test driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano@xxxxxxxxxxxxxxxx");
> diff --git a/tools/testing/selftests/powercap/powercap_hierarchy.sh b/tools/testing/selftests/powercap/powercap_hierarchy.sh
> new file mode 100755
> index 000000000000..6d1394a7d56b
> --- /dev/null
> +++ b/tools/testing/selftests/powercap/powercap_hierarchy.sh
> @@ -0,0 +1,126 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# Loading the module runs its in-kernel hierarchy API checks. Then validate
> +# all 16 exported zones, their power readings and constraint attributes, and
> +# removal of the controller while a sysfs attribute descriptor remains open.
> +
> +ksft_pass=0
> +ksft_fail=1
> +ksft_skip=4
> +
> +MODULE=powercap_hierarchy
> +CONTROL=powercap-test
> +SYSFS=/sys/devices/virtual/powercap/$CONTROL
> +
> +fail()
> +{
> +       echo "FAIL: $*"
> +       exit $ksft_fail
> +}
> +
> +skip()
> +{
> +       echo "SKIP: $*"
> +       exit $ksft_skip
> +}
> +
> +cleanup()
> +{
> +       if lsmod | grep -q "^${MODULE}\b"; then
> +               if ! rmmod "$MODULE"; then
> +                       echo "WARNING: failed to unload $MODULE"
> +               fi
> +       fi
> +}
> +
> +trap cleanup EXIT INT TERM
> +
> +[ "$(id -u)" -eq 0 ] || skip "must be run as root"
> +
> +insmod ./powercap_hierarchy.ko || fail "failed to load module"
> +
> +[ -d "$SYSFS" ] || fail "missing $SYSFS"
> +
> +check_zone()
> +{
> +       zone=$1
> +
> +       # Check required attributes and the deterministic values from the module.
> +       [ -f "$zone/name" ] || fail "$zone/name missing"
> +
> +       [ -f "$zone/power_uw" ] || \
> +               fail "$zone/power_uw missing"
> +
> +       [ -f "$zone/max_power_range_uw" ] || \
> +               fail "$zone/max_power_range_uw missing"
> +
> +       power=$(cat "$zone/power_uw")
> +       [ "$power" = "12648430" ] || \
> +               fail "$zone: unexpected power_uw ($power)"
> +
> +       max=$(cat "$zone/max_power_range_uw")
> +       [ "$max" = "50159747054" ] || \
> +               fail "$zone: unexpected max_power_range_uw ($max)"
> +
> +       constraint="$zone/constraint_0"
> +
> +       name=$(cat "$constraint""_name")
> +       [ "$name" = "my constraint name" ] || \
> +               fail "$constraint: bad constraint name"
> +
> +       pl=$(cat "$constraint""_power_limit_uw")
> +       [ "$pl" = "3735929054" ] || \
> +               fail "$constraint: bad power limit"
> +
> +       mp=$(cat "$constraint""_max_power_uw")
> +       [ "$mp" = "3735929054" ] || \
> +               fail "$constraint: bad max power"
> +}
> +
> +zones=0
> +
> +namefiles=$(find "$SYSFS" -type f -name name) || fail "failed to list zones"
> +
> +while IFS= read -r namefile
> +do
> +       zone=$(dirname "$namefile")
> +
> +       case "$zone" in
> +               */constraint_*)
> +                       continue
> +                       ;;
> +       esac
> +
> +       check_zone "$zone"
> +
> +       zones=$((zones + 1))
> +done <<EOF
> +$namefiles
> +EOF
> +
> +#
> +# Count the number of powercap zones.
> +#
> +[ "$zones" -eq 16 ] || \
> +       fail "expected 16 zones, got $zones"
> +
> +#
> +# Keep an attribute open across module removal, then close it afterwards.
> +#
> +exec 3<"$zone/power_uw" || fail "failed to open power_uw before removal"
> +cleanup
> +exec 3<&-
> +
> +#
> +# Verify that the hierarchy disappeared.
> +#
> +if [ -d "$SYSFS" ]; then
> +       fail "$SYSFS still exists after module removal"
> +fi
> +
> +trap - EXIT INT TERM
> +
> +echo "PASS"
> +
> +exit $ksft_pass
> --
> 2.43.0
>