On 2019-11-25 10:20 PM, Matthias Kaehlcke wrote:
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:
+/* 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?
Should check the return value from freq_qos_remove_request
- 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
Fixed
- 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
Both re-add and readd seem to be valid, I'll change to "added again".
-- Regards, Leonard