| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0gK9vRZtL4e9BJx+eR8nj7-FpRj3of8iQOLMNEwoz9oTQ@mail.gmail.com>
Date: Sat, 27 Sep 2025 13:44:07 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Brian Norris <briannorris@...omium.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Pavel Machek <pavel@...nel.org>,
"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>, Len Brown <lenb@...nel.org>,
linux-kernel@...r.kernel.org, Sakari Ailus <sakari.ailus@...ux.intel.com>,
kunit-dev@...glegroups.com, linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 1/3] PM: runtime: Add basic kunit tests for API contracts
On Thu, Sep 25, 2025 at 9:43 PM Brian Norris <briannorris@...omium.org> wrote:
>
> In exploring the various return codes and failure modes of runtime PM
> APIs, I found it helpful to verify and codify many of them in unit
> tests, especially given that even the kerneldoc can be rather complex to
> reason through, and it also has had subtle errors of its own.
>
> Notably, I avoid testing the return codes for pm_runtime_put() and
> pm_runtime_put_autosuspend(), since code that checks them is probably
> wrong, and we're considering making them return 'void' altogether. I
> still test the sync() variants, since those have a bit more meaning to
> them.
>
> Signed-off-by: Brian Norris <briannorris@...omium.org>
> ---
>
> Changes in v2:
> * Improve comments, per review suggestions
> * Minor sequence changes, per review suggestions
> * Stop testing for pm_runtime_put() and pm_runtime_put_autosuspend()
> return codes
>
> drivers/base/Kconfig | 6 +
> drivers/base/power/Makefile | 1 +
> drivers/base/power/runtime-test.c | 253 ++++++++++++++++++++++++++++++
> 3 files changed, 260 insertions(+)
> create mode 100644 drivers/base/power/runtime-test.c
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 064eb52ff7e2..1786d87b29e2 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -167,6 +167,12 @@ config PM_QOS_KUNIT_TEST
> depends on KUNIT=y
> default KUNIT_ALL_TESTS
>
> +config PM_RUNTIME_KUNIT_TEST
> + tristate "KUnit Tests for runtime PM" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + depends on PM
> + default KUNIT_ALL_TESTS
> +
> config HMEM_REPORTING
> bool
> default n
> diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
> index 01f11629d241..2989e42d0161 100644
> --- a/drivers/base/power/Makefile
> +++ b/drivers/base/power/Makefile
> @@ -4,5 +4,6 @@ obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o wakeup_stats.o
> obj-$(CONFIG_PM_TRACE_RTC) += trace.o
> obj-$(CONFIG_HAVE_CLK) += clock_ops.o
> obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o
> +obj-$(CONFIG_PM_RUNTIME_KUNIT_TEST) += runtime-test.o
>
> ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
> diff --git a/drivers/base/power/runtime-test.c b/drivers/base/power/runtime-test.c
> new file mode 100644
> index 000000000000..2e966fd96664
> --- /dev/null
> +++ b/drivers/base/power/runtime-test.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 Google, Inc.
> + */
> +
> +#include <linux/cleanup.h>
> +#include <linux/pm_runtime.h>
> +#include <kunit/device.h>
> +#include <kunit/test.h>
> +
> +#define DEVICE_NAME "pm_runtime_test_device"
> +
> +static void pm_runtime_depth_test(struct kunit *test)
> +{
> + struct device *dev = kunit_device_register(test, DEVICE_NAME);
> +
> + KUNIT_ASSERT_PTR_NE(test, NULL, dev);
> +
> + pm_runtime_enable(dev);
> +
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> + KUNIT_EXPECT_EQ(test, 1, pm_runtime_get_sync(dev)); /* "already active" */
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +}
> +
> +/* Test pm_runtime_put() and friends when already suspended. */
> +static void pm_runtime_already_suspended_test(struct kunit *test)
> +{
> + struct device *dev = kunit_device_register(test, DEVICE_NAME);
> +
> + KUNIT_ASSERT_PTR_NE(test, NULL, dev);
> +
> + pm_runtime_enable(dev);
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +
> + pm_runtime_get_noresume(dev);
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_barrier(dev)); /* no wakeup needed */
> + pm_runtime_put(dev);
> +
> + pm_runtime_get_noresume(dev);
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_put_sync(dev));
> +
> + KUNIT_EXPECT_EQ(test, 1, pm_runtime_suspend(dev));
> + KUNIT_EXPECT_EQ(test, 1, pm_runtime_autosuspend(dev));
> + KUNIT_EXPECT_EQ(test, 1, pm_request_autosuspend(dev));
> +
> + pm_runtime_get_noresume(dev);
> + KUNIT_EXPECT_EQ(test, 1, pm_runtime_put_sync_autosuspend(dev));
> +
> + pm_runtime_get_noresume(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> + /* Grab 2 refcounts */
> + pm_runtime_get_noresume(dev);
> + pm_runtime_get_noresume(dev);
> + /* The first put() sees usage_count 1 */
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync_autosuspend(dev));
> + /* The second put() sees usage_count 0 but tells us "already suspended". */
> + KUNIT_EXPECT_EQ(test, 1, pm_runtime_put_sync_autosuspend(dev));
> +
> + /* Should have remained suspended the whole time. */
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +}
> +
> +static void pm_runtime_idle_test(struct kunit *test)
> +{
> + struct device *dev = kunit_device_register(test, DEVICE_NAME);
> +
> + KUNIT_ASSERT_PTR_NE(test, NULL, dev);
> +
> + pm_runtime_enable(dev);
> +
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_idle(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> + pm_runtime_put_noidle(dev);
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_idle(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_idle(dev));
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_request_idle(dev));
> +}
> +
> +static void pm_runtime_disabled_test(struct kunit *test)
> +{
> + struct device *dev = kunit_device_register(test, DEVICE_NAME);
> +
> + KUNIT_ASSERT_PTR_NE(test, NULL, dev);
> +
> + /* Never called pm_runtime_enable() */
> + KUNIT_EXPECT_FALSE(test, pm_runtime_enabled(dev));
> +
> + /* "disabled" is treated as "active" */
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> + KUNIT_EXPECT_FALSE(test, pm_runtime_suspended(dev));
> +
> + /*
> + * Note: these "fail", but they still acquire/release refcounts, so
> + * keep them balanced.
> + */
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_get(dev));
> + pm_runtime_put(dev);
> +
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_get_sync(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_put_sync(dev));
> +
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_get(dev));
> + pm_runtime_put_autosuspend(dev);
> +
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_resume_and_get(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_idle(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_request_idle(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_request_resume(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_request_autosuspend(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_suspend(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_resume(dev));
> + KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_autosuspend(dev));
> +
> + /* Still disabled */
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> + KUNIT_EXPECT_FALSE(test, pm_runtime_enabled(dev));
> +}
> +
> +static void pm_runtime_error_test(struct kunit *test)
> +{
> + struct device *dev = kunit_device_register(test, DEVICE_NAME);
> +
> + KUNIT_ASSERT_PTR_NE(test, NULL, dev);
> +
> + pm_runtime_enable(dev);
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +
> + /* Fake a .runtime_resume() error */
> + dev->power.runtime_error = -EIO;
> +
> + /*
> + * Note: these "fail", but they still acquire/release refcounts, so
> + * keep them balanced.
> + */
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_get(dev));
> + pm_runtime_put(dev);
> +
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_get_sync(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_put_sync(dev));
> +
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_get(dev));
> + pm_runtime_put_autosuspend(dev);
> +
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_get(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_put_sync_autosuspend(dev));
> +
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_resume_and_get(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_idle(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_request_idle(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_request_resume(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_request_autosuspend(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_suspend(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_resume(dev));
> + KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_autosuspend(dev));
> +
> + /* Error is still pending */
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> + KUNIT_EXPECT_EQ(test, -EIO, dev->power.runtime_error);
> + /* Clear error */
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_set_suspended(dev));
> + KUNIT_EXPECT_EQ(test, 0, dev->power.runtime_error);
> + /* Still suspended */
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_get(dev));
> + KUNIT_EXPECT_EQ(test, 1, pm_runtime_barrier(dev)); /* resume was pending */
> + pm_runtime_put(dev);
> + pm_runtime_suspend(dev); /* flush the put(), to suspend */
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
> +
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
> + pm_runtime_put_autosuspend(dev);
> +
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_resume_and_get(dev));
> +
> + /*
> + * The following should all return -EAGAIN (usage is non-zero) or 1
> + * (already resumed).
> + */
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_idle(dev));
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_request_idle(dev));
> + KUNIT_EXPECT_EQ(test, 1, pm_request_resume(dev));
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_request_autosuspend(dev));
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_suspend(dev));
> + KUNIT_EXPECT_EQ(test, 1, pm_runtime_resume(dev));
> + KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_autosuspend(dev));
> +
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
> +
> + /* Suspended again */
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +}
> +
> +/*
> + * Explore a typical probe() sequence in which a device marks itself powered,
> + * but doesn't hold any runtime PM reference, so it suspends as soon as it goes
> + * idle.
> + */
> +static void pm_runtime_probe_active_test(struct kunit *test)
> +{
> + struct device *dev = kunit_device_register(test, DEVICE_NAME);
> +
> + KUNIT_ASSERT_PTR_NE(test, NULL, dev);
> +
> + KUNIT_EXPECT_TRUE(test, pm_runtime_status_suspended(dev));
> +
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_set_active(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> +
> + pm_runtime_enable(dev);
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> +
> + /* Nothing to flush. We stay active. */
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_barrier(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
> +
> + /* Ask for idle? Now we suspend. */
> + KUNIT_EXPECT_EQ(test, 0, pm_runtime_idle(dev));
> + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +}
> +
> +static struct kunit_case pm_runtime_test_cases[] = {
> + KUNIT_CASE(pm_runtime_depth_test),
> + KUNIT_CASE(pm_runtime_already_suspended_test),
> + KUNIT_CASE(pm_runtime_idle_test),
> + KUNIT_CASE(pm_runtime_disabled_test),
> + KUNIT_CASE(pm_runtime_error_test),
> + KUNIT_CASE(pm_runtime_probe_active_test),
> + {}
> +};
> +
> +static struct kunit_suite pm_runtime_test_suite = {
> + .name = "pm_runtime_test_cases",
> + .test_cases = pm_runtime_test_cases,
> +};
> +
> +kunit_test_suite(pm_runtime_test_suite);
> +MODULE_DESCRIPTION("Runtime power management unit test suite");
> +MODULE_LICENSE("GPL");
> --
Applied as 6.18 material along with patches [2-3/3].
Thanks!
Powered by blists - more mailing lists