On Tue, Nov 26, 2019 at 05:17:10PM +0200, Leonard Crestez wrote:
The pm_qos family of APIs are used in relatively difficult to reproduce scenarios such as thermal throttling so they benefit from unit testing.
Start by adding basic tests from the the freq_qos APIs. It includes tests for issues that were brought up on mailing lists:
https://patchwork.kernel.org/patch/11252425/#23017005 https://patchwork.kernel.org/patch/11253421/
Signed-off-by: Leonard Crestez leonard.crestez@nxp.com
drivers/base/Kconfig | 4 ++ drivers/base/power/Makefile | 1 + drivers/base/power/qos-test.c | 117 ++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+) create mode 100644 drivers/base/power/qos-test.c
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index e37d37684132..d4ae1c1adf69 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -155,10 +155,14 @@ config DEBUG_TEST_DRIVER_REMOVE This option is expected to find errors and may render your system unusable. You should say N here unless you are explicitly looking to test this functionality. +config PM_QOS_KUNIT_TEST
- bool "KUnit Test for PM QoS features"
- depends on KUNIT
config HMEM_REPORTING bool default n depends on NUMA help diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile index ec5bb190b9d0..8fdd0073eeeb 100644 --- a/drivers/base/power/Makefile +++ b/drivers/base/power/Makefile @@ -2,7 +2,8 @@ obj-$(CONFIG_PM) += sysfs.o generic_ops.o common.o qos.o runtime.o wakeirq.o obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o wakeup_stats.o obj-$(CONFIG_PM_TRACE_RTC) += trace.o obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o domain_governor.o obj-$(CONFIG_HAVE_CLK) += clock_ops.o +obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG diff --git a/drivers/base/power/qos-test.c b/drivers/base/power/qos-test.c new file mode 100644 index 000000000000..3115db08d56b --- /dev/null +++ b/drivers/base/power/qos-test.c @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright 2019 NXP
- */
+#include <kunit/test.h> +#include <linux/pm_qos.h>
+/* Basic test for aggregating two "min" requests */ +static void freq_qos_test_min(struct kunit *test) +{
- struct freq_constraints qos;
- struct freq_qos_request req1, req2;
- int ret;
- freq_constraints_init(&qos);
- memset(&req1, 0, sizeof(req1));
- memset(&req2, 0, sizeof(req2));
- ret = freq_qos_add_request(&qos, &req1, FREQ_QOS_MIN, 1000);
- KUNIT_EXPECT_EQ(test, ret, 1);
- ret = freq_qos_add_request(&qos, &req2, FREQ_QOS_MIN, 2000);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN), 2000);
- ret = freq_qos_remove_request(&req2);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN), 1000);
- ret = freq_qos_remove_request(&req1);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN),
FREQ_QOS_MIN_DEFAULT_VALUE);+}
+/* Test that requests for MAX_DEFAULT_VALUE have no effect */ +static void freq_qos_test_maxdef(struct kunit *test) +{
- struct freq_constraints qos;
- struct freq_qos_request req1, req2;
- int ret;
- freq_constraints_init(&qos);
- memset(&req1, 0, sizeof(req1));
- memset(&req2, 0, sizeof(req2));
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MAX),
FREQ_QOS_MAX_DEFAULT_VALUE);- ret = freq_qos_add_request(&qos, &req1, FREQ_QOS_MAX,
FREQ_QOS_MAX_DEFAULT_VALUE);- KUNIT_EXPECT_EQ(test, ret, 0);
- ret = freq_qos_add_request(&qos, &req2, FREQ_QOS_MAX,
FREQ_QOS_MAX_DEFAULT_VALUE);- KUNIT_EXPECT_EQ(test, ret, 0);
- /* Add max 1000 */
- ret = freq_qos_update_request(&req1, 1000);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MAX), 1000);
- /* Add max 2000, no impact */
- ret = freq_qos_update_request(&req2, 2000);
- KUNIT_EXPECT_EQ(test, ret, 0);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MAX), 1000);
- /* Remove max 1000, new max 2000 */
- ret = freq_qos_remove_request(&req1);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MAX), 2000);
nit: this last part isn't really related with MAX_DEFAULT_VALUE. It's a worthwhile test, but not necessarily in this test case. It might make more sense to set one of the constraints to FREQ_QOS_MAX_DEFAULT_VALUE again, and verify it doesn't have an impact.
Just a comment, there's nothing really wrong with how it is.
+}
+/*
- Test that a freq_qos_request can be added again after removal
- This issue was solved by commit 05ff1ba412fd ("PM: QoS: Invalidate frequency
- QoS requests after removal")
- */
+static void freq_qos_test_readd(struct kunit *test) +{
- struct freq_constraints qos;
- struct freq_qos_request req;
- int ret;
- freq_constraints_init(&qos);
- memset(&req, 0, sizeof(req));
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN),
FREQ_QOS_MIN_DEFAULT_VALUE);
nit: you could do this check once in a dedicated test and omit it in other tests to de-clutter
- /* Add */
- ret = freq_qos_add_request(&qos, &req, FREQ_QOS_MIN, 1000);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN), 1000);
similar here, this test validates re-adding, another dedicated test could verify once that the aggregate value is correct after adding a single request. Checking the return value still is sensible, just in case.
I guess it can be argued either way, checking the values every time is extra-safe, omitting the checks reduces clutter and might help to make it clearer what the test really intends to verify.
- /* Remove */
- ret = freq_qos_remove_request(&req);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN),
FREQ_QOS_MIN_DEFAULT_VALUE);
ditto
- /* Add again */
- ret = freq_qos_add_request(&qos, &req, FREQ_QOS_MIN, 2000);
- KUNIT_EXPECT_EQ(test, ret, 1);
- KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN), 2000);
Here the explicit check makes sense, since we verify re-adding.
Anyway, my comments are just about possible improvements, it's also fine as is:
Reviewed-by: Matthias Kaehlcke mka@chromium.org