On Mon, Nov 25, 2019 at 06:42:16PM +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. indeed, a unit test is useful in this case! > 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@xxxxxxx> > --- > drivers/base/Kconfig | 4 ++ > drivers/base/power/Makefile | 1 + > drivers/base/power/qos-test.c | 116 ++++++++++++++++++++++++++++++++++ > 3 files changed, 121 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..8267d91332a8 > --- /dev/null > +++ b/drivers/base/power/qos-test.c > @@ -0,0 +1,116 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 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); > + > + freq_qos_remove_request(&req2); > + KUNIT_EXPECT_EQ(test, ret, 1); This checks (again) the return value of the above freq_qos_add_request() call, which I suppose is not intended. Remove? > + KUNIT_EXPECT_EQ(test, freq_qos_read_value(&qos, FREQ_QOS_MIN), 1000); > + > + freq_qos_remove_request(&req1); > + KUNIT_EXPECT_EQ(test, ret, 1); ditto > + 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 2000, new max 1000 */ the code doesn't match the comment, max 1000 is removed > + 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); > +} > + > +/* > + * Test that a freq_qos_request can be readded after removal nit: 're-added'. It took me a few secs to figure this is not a about 'read'ing something