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
