[PATCH v4] lib: add basic KUnit test for lib/math

6 views
Skip to first unread message

Daniel Latypov

unread,
Apr 8, 2021, 9:40:15 PM4/8/21
to andriy.s...@linux.intel.com, brendan...@google.com, davi...@google.com, linux-...@vger.kernel.org, kuni...@googlegroups.com, linux-k...@vger.kernel.org, sk...@linuxfoundation.org, Daniel Latypov
Add basic test coverage for files that don't require any config options:
* gcd.c
* lcm.c
* int_sqrt.c
* reciprocal_div.c
(Ignored int_pow.c since it's a simple textbook algorithm.)

These tests aren't particularly interesting, but they
* provide short and simple examples of parameterized tests
* provide a place to add tests for any new files in this dir
* are written so adding new test cases to cover edge cases should be easy

Signed-off-by: Daniel Latypov <dlat...@google.com>
---
Changes since v3:
* fix `checkpatch.pl --strict` warnings
* add test cases for gcd(0,0) and lcm(0,0)
* minor: don't test both gcd(a,b) and gcd(b,a) when a == b

Changes since v2: mv math_test.c => math_kunit.c

Changes since v1:
* Rebase and rewrite to use the new parameterized testing support.
* misc: fix overflow in literal and inline int_sqrt format string.
* related: commit 1f0e943df68a ("Documentation: kunit: provide guidance
for testing many inputs") was merged explaining the patterns shown here.
* there's an in-flight patch to update it for parameterized testing.

v1: https://lore.kernel.org/lkml/20201019224556.3...@google.com/
---
lib/math/Kconfig | 5 +
lib/math/Makefile | 2 +
lib/math/math_kunit.c | 214 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 221 insertions(+)
create mode 100644 lib/math/math_kunit.c

diff --git a/lib/math/Kconfig b/lib/math/Kconfig
index f19bc9734fa7..6ba8680439c1 100644
--- a/lib/math/Kconfig
+++ b/lib/math/Kconfig
@@ -15,3 +15,8 @@ config PRIME_NUMBERS

config RATIONAL
bool
+
+config MATH_KUNIT_TEST
+ tristate "KUnit test for lib/math" if !KUNIT_ALL_TESTS
+ default KUNIT_ALL_TESTS
+ depends on KUNIT
diff --git a/lib/math/Makefile b/lib/math/Makefile
index be6909e943bd..30abb7a8d564 100644
--- a/lib/math/Makefile
+++ b/lib/math/Makefile
@@ -4,3 +4,5 @@ obj-y += div64.o gcd.o lcm.o int_pow.o int_sqrt.o reciprocal_div.o
obj-$(CONFIG_CORDIC) += cordic.o
obj-$(CONFIG_PRIME_NUMBERS) += prime_numbers.o
obj-$(CONFIG_RATIONAL) += rational.o
+
+obj-$(CONFIG_MATH_KUNIT_TEST) += math_kunit.o
diff --git a/lib/math/math_kunit.c b/lib/math/math_kunit.c
new file mode 100644
index 000000000000..fed15ade8fb2
--- /dev/null
+++ b/lib/math/math_kunit.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Simple KUnit suite for math helper funcs that are always enabled.
+ *
+ * Copyright (C) 2020, Google LLC.
+ * Author: Daniel Latypov <dlat...@google.com>
+ */
+
+#include <kunit/test.h>
+#include <linux/gcd.h>
+#include <linux/kernel.h>
+#include <linux/lcm.h>
+#include <linux/reciprocal_div.h>
+
+/* Generic test case for unsigned long inputs. */
+struct test_case {
+ unsigned long a, b;
+ unsigned long result;
+};
+
+static struct test_case gcd_cases[] = {
+ {
+ .a = 0, .b = 0,
+ .result = 0,
+ },
+ {
+ .a = 0, .b = 1,
+ .result = 1,
+ },
+ {
+ .a = 2, .b = 2,
+ .result = 2,
+ },
+ {
+ .a = 2, .b = 4,
+ .result = 2,
+ },
+ {
+ .a = 3, .b = 5,
+ .result = 1,
+ },
+ {
+ .a = 3 * 9, .b = 3 * 5,
+ .result = 3,
+ },
+ {
+ .a = 3 * 5 * 7, .b = 3 * 5 * 11,
+ .result = 15,
+ },
+ {
+ .a = 1 << 21,
+ .b = (1 << 21) - 1,
+ .result = 1,
+ },
+};
+
+KUNIT_ARRAY_PARAM(gcd, gcd_cases, NULL);
+
+static void gcd_test(struct kunit *test)
+{
+ const char *message_fmt = "gcd(%lu, %lu)";
+ const struct test_case *test_param = test->param_value;
+
+ KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+ gcd(test_param->a, test_param->b),
+ message_fmt, test_param->a,
+ test_param->b);
+
+ if (test_param->a == test_param->b)
+ return;
+
+ /* gcd(a,b) == gcd(b,a) */
+ KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+ gcd(test_param->b, test_param->a),
+ message_fmt, test_param->b,
+ test_param->a);
+}
+
+static struct test_case lcm_cases[] = {
+ {
+ .a = 0, .b = 0,
+ .result = 0,
+ },
+ {
+ .a = 0, .b = 1,
+ .result = 0,
+ },
+ {
+ .a = 1, .b = 2,
+ .result = 2,
+ },
+ {
+ .a = 2, .b = 2,
+ .result = 2,
+ },
+ {
+ .a = 3 * 5, .b = 3 * 7,
+ .result = 3 * 5 * 7,
+ },
+};
+
+KUNIT_ARRAY_PARAM(lcm, lcm_cases, NULL);
+
+static void lcm_test(struct kunit *test)
+{
+ const char *message_fmt = "lcm(%lu, %lu)";
+ const struct test_case *test_param = test->param_value;
+
+ KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+ lcm(test_param->a, test_param->b),
+ message_fmt, test_param->a,
+ test_param->b);
+
+ if (test_param->a == test_param->b)
+ return;
+
+ /* lcm(a,b) == lcm(b,a) */
+ KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+ lcm(test_param->b, test_param->a),
+ message_fmt, test_param->b,
+ test_param->a);
+}
+
+static struct test_case int_sqrt_cases[] = {
+ {
+ .a = 0,
+ .result = 0,
+ },
+ {
+ .a = 1,
+ .result = 1,
+ },
+ {
+ .a = 4,
+ .result = 2,
+ },
+ {
+ .a = 5,
+ .result = 2,
+ },
+ {
+ .a = 8,
+ .result = 2,
+ },
+ {
+ .a = 1UL << 30,
+ .result = 1UL << 15,
+ },
+};
+
+KUNIT_ARRAY_PARAM(int_sqrt, int_sqrt_cases, NULL);
+
+static void int_sqrt_test(struct kunit *test)
+{
+ const struct test_case *test_param = test->param_value;
+
+ KUNIT_EXPECT_EQ_MSG(test, int_sqrt(test_param->a),
+ test_param->result, "sqrt(%lu)",
+ test_param->a);
+}
+
+struct reciprocal_test_case {
+ u32 a, b;
+ u32 result;
+};
+
+static struct reciprocal_test_case reciprocal_div_cases[] = {
+ {
+ .a = 0, .b = 1,
+ .result = 0,
+ },
+ {
+ .a = 42, .b = 20,
+ .result = 2,
+ },
+ {
+ .a = 42, .b = 9999,
+ .result = 0,
+ },
+ {
+ .a = (1 << 16), .b = (1 << 14),
+ .result = 1 << 2,
+ },
+};
+
+KUNIT_ARRAY_PARAM(reciprocal_div, reciprocal_div_cases, NULL);
+
+static void reciprocal_div_test(struct kunit *test)
+{
+ const struct reciprocal_test_case *test_param = test->param_value;
+ struct reciprocal_value rv = reciprocal_value(test_param->b);
+
+ KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+ reciprocal_divide(test_param->a, rv),
+ "reciprocal_divide(%u, %u)",
+ test_param->a, test_param->b);
+}
+
+static struct kunit_case math_test_cases[] = {
+ KUNIT_CASE_PARAM(gcd_test, gcd_gen_params),
+ KUNIT_CASE_PARAM(lcm_test, lcm_gen_params),
+ KUNIT_CASE_PARAM(int_sqrt_test, int_sqrt_gen_params),
+ KUNIT_CASE_PARAM(reciprocal_div_test, reciprocal_div_gen_params),
+ {}
+};
+
+static struct kunit_suite math_test_suite = {
+ .name = "lib-math",
+ .test_cases = math_test_cases,
+};
+
+kunit_test_suites(&math_test_suite);
+
+MODULE_LICENSE("GPL v2");

base-commit: 4fa56ad0d12e24df768c98bffe9039f915d1bc02
--
2.31.1.295.g9ea45b61b8-goog

Andy Shevchenko

unread,
Apr 9, 2021, 11:30:21 AM4/9/21
to Daniel Latypov, brendan...@google.com, davi...@google.com, linux-...@vger.kernel.org, kuni...@googlegroups.com, linux-k...@vger.kernel.org, sk...@linuxfoundation.org
On Thu, Apr 08, 2021 at 06:40:01PM -0700, Daniel Latypov wrote:
> Add basic test coverage for files that don't require any config options:
> * gcd.c
> * lcm.c
> * int_sqrt.c
> * reciprocal_div.c
> (Ignored int_pow.c since it's a simple textbook algorithm.)

What about adding math.h test cases?

We have some macros there and it might be a good idea to test them, for example
that round_up() and roundup() produces the same output for the same (power of
two divisor) input.

> These tests aren't particularly interesting, but they
> * provide short and simple examples of parameterized tests
> * provide a place to add tests for any new files in this dir
> * are written so adding new test cases to cover edge cases should be easy

Yes, that's why I think macros also can be a good example how to test *macro*.

--
With Best Regards,
Andy Shevchenko


Daniel Latypov

unread,
Apr 9, 2021, 12:29:23 PM4/9/21
to Andy Shevchenko, Brendan Higgins, David Gow, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Fri, Apr 9, 2021 at 8:30 AM Andy Shevchenko
<andriy.s...@linux.intel.com> wrote:
>
> On Thu, Apr 08, 2021 at 06:40:01PM -0700, Daniel Latypov wrote:
> > Add basic test coverage for files that don't require any config options:
> > * gcd.c
> > * lcm.c
> > * int_sqrt.c
> > * reciprocal_div.c
> > (Ignored int_pow.c since it's a simple textbook algorithm.)
>
> What about adding math.h test cases?
>
> We have some macros there and it might be a good idea to test them, for example
> that round_up() and roundup() produces the same output for the same (power of
> two divisor) input.

I completely overlooked the macros in math.h, sounds like a good idea.

Grepping around, seems like abs() and DIV_ROUND_UP/CLOSEST() are among
the more popular macros:
$ ag -s '\bDIV_ROUND_UP\(' | wc -l
2946
$ ag -s '\babs\(' | wc -l
923
$ ag -s '\bDIV_ROUND_CLOSEST\(' | wc -l
864
$ ag -s '\bround_up\(' | wc -l
727
$ ag -s '\broundup\(' | wc -l
620
$ ag -s '\bround_down\(' | wc -l
371
$ ag -s 'rounddown\(' | wc -l
131


>
> > These tests aren't particularly interesting, but they
> > * provide short and simple examples of parameterized tests
> > * provide a place to add tests for any new files in this dir
> > * are written so adding new test cases to cover edge cases should be easy
>
> Yes, that's why I think macros also can be a good example how to test *macro*.

Yeah, there's more to cover there since they have a range of types
they can work on.

On another note, the parameterized test arrays all use unsigned long,
so abs() sticks out even more.
I'm thinking of something like

static void test_abs(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, abs('a'), 'a');
KUNIT_EXPECT_EQ(test, abs(-'a'), 'a');
...
}

and then maybe use parameters for the other macros but also throw in
an additional test case like

static void test_div_round_up_diff_types(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, DIV_ROUND_UP((char) 42, (char) 10), (char) 4);
KUNIT_EXPECT_EQ(test, DIV_ROUND_UP((int) 42, (int) 10), (int) 4);
KUNIT_EXPECT_EQ(test, DIV_ROUND_UP((long) 42, (long) 10), (long) 4);
...
}

>
> --
> With Best Regards,
> Andy Shevchenko
>
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/YHBzA7SwH194ywRv%40smile.fi.intel.com.

Daniel Latypov

unread,
Apr 12, 2021, 3:07:21 PM4/12/21
to andriy.s...@linux.intel.com, brendan...@google.com, davi...@google.com, linux-...@vger.kernel.org, kuni...@googlegroups.com, linux-k...@vger.kernel.org, sk...@linuxfoundation.org, Daniel Latypov
Add basic test coverage for files that don't require any config options:
* part of math.h (what seem to be the most commonly used macros)
* gcd.c
* lcm.c
* int_sqrt.c
* reciprocal_div.c
(Ignored int_pow.c since it's a simple textbook algorithm.)

These tests aren't particularly interesting, but they
* provide short and simple examples of parameterized tests
* provide a place to add tests for any new files in this dir
* are written so adding new test cases to cover edge cases should be easy
* looking at code coverage, we hit all the branches in the .c files

Signed-off-by: Daniel Latypov <dlat...@google.com>
---
Changes since v4:
* add in test cases for some math.h macros (abs, round_up/round_down,
div_round_down/closest)
* use parameterized testing less to keep things terser

Changes since v3:
* fix `checkpatch.pl --strict` warnings
* add test cases for gcd(0,0) and lcm(0,0)
* minor: don't test both gcd(a,b) and gcd(b,a) when a == b

Changes since v2: mv math_test.c => math_kunit.c

Changes since v1:
* Rebase and rewrite to use the new parameterized testing support.
* misc: fix overflow in literal and inline int_sqrt format string.
* related: commit 1f0e943df68a ("Documentation: kunit: provide guidance
for testing many inputs") was merged explaining the patterns shown here.
* there's an in-flight patch to update it for parameterized testing.

v1: https://lore.kernel.org/lkml/20201019224556.3...@google.com/
---
lib/math/Kconfig | 5 +
lib/math/Makefile | 2 +
lib/math/math_kunit.c | 264 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 271 insertions(+)
index 000000000000..80a087a32884
--- /dev/null
+++ b/lib/math/math_kunit.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Simple KUnit suite for math helper funcs that are always enabled.
+ *
+ * Copyright (C) 2020, Google LLC.
+ * Author: Daniel Latypov <dlat...@google.com>
+ */
+
+#include <kunit/test.h>
+#include <linux/gcd.h>
+#include <linux/kernel.h>
+#include <linux/lcm.h>
+#include <linux/reciprocal_div.h>
+
+static void abs_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, abs('\0'), '\0');
+ KUNIT_EXPECT_EQ(test, abs('a'), 'a');
+ KUNIT_EXPECT_EQ(test, abs(-'a'), 'a');
+
+ /* The expression in the macro is actually promoted to an int. */
+ KUNIT_EXPECT_EQ(test, abs((short)0), 0);
+ KUNIT_EXPECT_EQ(test, abs((short)42), 42);
+ KUNIT_EXPECT_EQ(test, abs((short)-42), 42);
+
+ KUNIT_EXPECT_EQ(test, abs(0), 0);
+ KUNIT_EXPECT_EQ(test, abs(42), 42);
+ KUNIT_EXPECT_EQ(test, abs(-42), 42);
+
+ KUNIT_EXPECT_EQ(test, abs(0L), 0L);
+ KUNIT_EXPECT_EQ(test, abs(42L), 42L);
+ KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
+
+ KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
+ KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
+ KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
+
+ /* Unsigned types get casted to signed. */
+ KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
+ KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);
+}
+
+static void int_sqrt_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(1UL), 1UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(4UL), 2UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(5UL), 2UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(8UL), 2UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(1UL << 30), 1UL << 15);
+}
+
+static void round_up_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, round_up(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, round_up(1, 2), 2);
+ KUNIT_EXPECT_EQ(test, round_up(3, 2), 4);
+ KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 2), 1 << 30);
+ KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 1 << 29), 1 << 30);
+}
+
+static void round_down_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, round_down(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, round_down(1, 2), 0);
+ KUNIT_EXPECT_EQ(test, round_down(3, 2), 2);
+ KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 2), (1 << 30) - 2);
+ KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 1 << 29), 1 << 29);
+}
+
+static void div_round_up_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(20, 10), 2);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 10), 3);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 20), 2);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 99), 1);
+}
+
+static void div_round_closest_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(20, 10), 2);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(21, 10), 2);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(25, 10), 3);
+}
+struct u32_test_case {
+ u32 a, b;
+ u32 result;
+};
+
+static struct u32_test_case reciprocal_div_cases[] = {
+ {
+ .a = 0, .b = 1,
+ .result = 0,
+ },
+ {
+ .a = 42, .b = 20,
+ .result = 2,
+ },
+ {
+ .a = 42, .b = 9999,
+ .result = 0,
+ },
+ {
+ .a = (1 << 16), .b = (1 << 14),
+ .result = 1 << 2,
+ },
+};
+
+KUNIT_ARRAY_PARAM(reciprocal_div, reciprocal_div_cases, NULL);
+
+static void reciprocal_div_test(struct kunit *test)
+{
+ const struct u32_test_case *test_param = test->param_value;
+ struct reciprocal_value rv = reciprocal_value(test_param->b);
+
+ KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+ reciprocal_divide(test_param->a, rv),
+ "reciprocal_divide(%u, %u)",
+ test_param->a, test_param->b);
+}
+
+static void reciprocal_scale_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, reciprocal_scale(0u, 100), 0u);
+ KUNIT_EXPECT_EQ(test, reciprocal_scale(1u, 100), 0u);
+ KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 4, 1 << 28), 1u);
+ KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 16, 1 << 28), 1u << 12);
+ KUNIT_EXPECT_EQ(test, reciprocal_scale(~0u, 1 << 28), (1u << 28) - 1);
+}
+
+static struct kunit_case math_test_cases[] = {
+ KUNIT_CASE(abs_test),
+ KUNIT_CASE(int_sqrt_test),
+ KUNIT_CASE(round_up_test),
+ KUNIT_CASE(round_down_test),
+ KUNIT_CASE(div_round_up_test),
+ KUNIT_CASE(div_round_closest_test),
+ KUNIT_CASE_PARAM(gcd_test, gcd_gen_params),
+ KUNIT_CASE_PARAM(lcm_test, lcm_gen_params),
+ KUNIT_CASE_PARAM(reciprocal_div_test, reciprocal_div_gen_params),
+ KUNIT_CASE(reciprocal_scale_test),

David Gow

unread,
Apr 13, 2021, 2:41:27 AM4/13/21
to Daniel Latypov, Andy Shevchenko, Brendan Higgins, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Tue, Apr 13, 2021 at 3:07 AM Daniel Latypov <dlat...@google.com> wrote:
>
> Add basic test coverage for files that don't require any config options:
> * part of math.h (what seem to be the most commonly used macros)
> * gcd.c
> * lcm.c
> * int_sqrt.c
> * reciprocal_div.c
> (Ignored int_pow.c since it's a simple textbook algorithm.)
>
> These tests aren't particularly interesting, but they
> * provide short and simple examples of parameterized tests
> * provide a place to add tests for any new files in this dir
> * are written so adding new test cases to cover edge cases should be easy
> * looking at code coverage, we hit all the branches in the .c files
>
> Signed-off-by: Daniel Latypov <dlat...@google.com>

This looks good to me. A few comments/observations below, but nothing
that I think should actually block this.

Reviewed-by: David Gow <davi...@google.com>

-- David
This could have a description of the test and KUnit here, as mentioned
in the style guide doc:
https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-kconfig-entries

(I think it's sufficiently self explanatory that it's not essential,
but it could be nice to have a more detailed description of the things
being tested than just "lib/math".)
There's something weird about taking the absolute values of char
literals. I'm not sure if it's better to case integer literals (like
with 'short' below), or keep it as-is.
> + KUNIT_EXPECT_EQ(test, abs('\0'), '\0');
> + KUNIT_EXPECT_EQ(test, abs('a'), 'a');
> + KUNIT_EXPECT_EQ(test, abs(-'a'), 'a');
> +
> + /* The expression in the macro is actually promoted to an int. */
> + KUNIT_EXPECT_EQ(test, abs((short)0), 0);
> + KUNIT_EXPECT_EQ(test, abs((short)42), 42);
> + KUNIT_EXPECT_EQ(test, abs((short)-42), 42);
> +
> + KUNIT_EXPECT_EQ(test, abs(0), 0);
> + KUNIT_EXPECT_EQ(test, abs(42), 42);
> + KUNIT_EXPECT_EQ(test, abs(-42), 42);
> +
> + KUNIT_EXPECT_EQ(test, abs(0L), 0L);
> + KUNIT_EXPECT_EQ(test, abs(42L), 42L);
> + KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
> +
> + KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
> + KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
> + KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
> +
> + /* Unsigned types get casted to signed. */
> + KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
> + KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);

A part of me is curious what the result is for -0x80000000, but I
guess that's not defined, so shouldn't be tested. :-)
> +}
> +
> +static void int_sqrt_test(struct kunit *test)
> +{
> + KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL);
> + KUNIT_EXPECT_EQ(test, int_sqrt(1UL), 1UL);
> + KUNIT_EXPECT_EQ(test, int_sqrt(4UL), 2UL);
> + KUNIT_EXPECT_EQ(test, int_sqrt(5UL), 2UL);
> + KUNIT_EXPECT_EQ(test, int_sqrt(8UL), 2UL);
> + KUNIT_EXPECT_EQ(test, int_sqrt(1UL << 30), 1UL << 15);
> +}
> +

_Maybe_ it's worth a comment here that round_up (and round_down) only
support rounding to powers of two?
(Either that, or also test roundup/rounddown to provide the contrast.)
Is there a reason this test is using KUNIT_EXPECT_EQ_MSG() rather than
a get_desc function in KUNIT_ARRAY_PARAM()? I can sort-of see how the
former is a little simpler, so I'm not opposed to keeping it as-is,
but it's nice to have KUnit aware of a nicer name for the parameter if
all else is equal.
(I think there's a stronger case for keeping the gcd/lcm tests as is
because they actually have two checks per parameter, but even then,
it's not absurdly silly. And it'd be possible to have both a get_desc
function and use EXPECT_..._MSG() to get the best of both worlds,
albeit with twice as much work.)

Daniel Latypov

unread,
Apr 13, 2021, 8:33:15 PM4/13/21
to David Gow, Andy Shevchenko, Brendan Higgins, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
Done. I've left off the details about what the test tests so we have
less places to go and update if/when new tests are added.
I just thought it was amusing :)
Converting it to be like the short test below.
abs(-42ULL) == 42, but the compiler spits out a warning.

> > +}
> > +
> > +static void int_sqrt_test(struct kunit *test)
> > +{
> > + KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL);
> > + KUNIT_EXPECT_EQ(test, int_sqrt(1UL), 1UL);
> > + KUNIT_EXPECT_EQ(test, int_sqrt(4UL), 2UL);
> > + KUNIT_EXPECT_EQ(test, int_sqrt(5UL), 2UL);
> > + KUNIT_EXPECT_EQ(test, int_sqrt(8UL), 2UL);
> > + KUNIT_EXPECT_EQ(test, int_sqrt(1UL << 30), 1UL << 15);
> > +}
> > +
>
> _Maybe_ it's worth a comment here that round_up (and round_down) only
> support rounding to powers of two?
> (Either that, or also test roundup/rounddown to provide the contrast.)

Adding in those test cases for v6.
Andy had asked for those as well but I had forgotten them by the time
I sent out v5.
I can add in the get_desc for it if you want.

That's partly a relic of the previous versions of this patchset where
I reused the case arrays for the unary funcs as well.
But now the unary use case has disappeared and we only need to write
one get_desc.

But yeah, given it can test two calls of gcd, I've opted to keep it
using _MSG().
And I figured I'd keep the reciprocal_div test the same for
consistency (aka, I just copy-pasted it from gcd).

Daniel Latypov

unread,
Apr 16, 2021, 2:04:34 PM4/16/21
to andriy.s...@linux.intel.com, brendan...@google.com, davi...@google.com, linux-...@vger.kernel.org, kuni...@googlegroups.com, linux-k...@vger.kernel.org, sk...@linuxfoundation.org, Daniel Latypov
Add basic test coverage for files that don't require any config options:
* part of math.h (what seem to be the most commonly used macros)
* gcd.c
* lcm.c
* int_sqrt.c
* reciprocal_div.c
(Ignored int_pow.c since it's a simple textbook algorithm.)

These tests aren't particularly interesting, but they
* provide short and simple examples of parameterized tests
* provide a place to add tests for any new files in this dir
* are written so adding new test cases to cover edge cases should be easy
* looking at code coverage, we hit all the branches in the .c files

Signed-off-by: Daniel Latypov <dlat...@google.com>
Reviewed-by: David Gow <davi...@google.com>
---
Changes since v5:
* add in test cases for roundup/rounddown
* address misc comments from David

Changes since v4:
* add in test cases for some math.h macros (abs, round_up/round_down,
div_round_down/closest)
* use parameterized testing less to keep things terser

Changes since v3:
* fix `checkpatch.pl --strict` warnings
* add test cases for gcd(0,0) and lcm(0,0)
* minor: don't test both gcd(a,b) and gcd(b,a) when a == b

Changes since v2: mv math_test.c => math_kunit.c

Changes since v1:
* Rebase and rewrite to use the new parameterized testing support.
* misc: fix overflow in literal and inline int_sqrt format string.
* related: commit 1f0e943df68a ("Documentation: kunit: provide guidance
for testing many inputs") was merged explaining the patterns shown here.
* there's an in-flight patch to update it for parameterized testing.
---
lib/math/Kconfig | 12 ++
lib/math/Makefile | 2 +
lib/math/math_kunit.c | 291 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 305 insertions(+)
create mode 100644 lib/math/math_kunit.c

diff --git a/lib/math/Kconfig b/lib/math/Kconfig
index f19bc9734fa7..a974d4db0f9c 100644
--- a/lib/math/Kconfig
+++ b/lib/math/Kconfig
@@ -15,3 +15,15 @@ config PRIME_NUMBERS

config RATIONAL
bool
+
+config MATH_KUNIT_TEST
+ tristate "KUnit test for lib/math and math.h" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ This builds unit tests for lib/math and math.h.
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
diff --git a/lib/math/Makefile b/lib/math/Makefile
index be6909e943bd..30abb7a8d564 100644
--- a/lib/math/Makefile
+++ b/lib/math/Makefile
@@ -4,3 +4,5 @@ obj-y += div64.o gcd.o lcm.o int_pow.o int_sqrt.o reciprocal_div.o
obj-$(CONFIG_CORDIC) += cordic.o
obj-$(CONFIG_PRIME_NUMBERS) += prime_numbers.o
obj-$(CONFIG_RATIONAL) += rational.o
+
+obj-$(CONFIG_MATH_KUNIT_TEST) += math_kunit.o
diff --git a/lib/math/math_kunit.c b/lib/math/math_kunit.c
new file mode 100644
index 000000000000..556c23b17c3c
--- /dev/null
+++ b/lib/math/math_kunit.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Simple KUnit suite for math helper funcs that are always enabled.
+ *
+ * Copyright (C) 2020, Google LLC.
+ * Author: Daniel Latypov <dlat...@google.com>
+ */
+
+#include <kunit/test.h>
+#include <linux/gcd.h>
+#include <linux/kernel.h>
+#include <linux/lcm.h>
+#include <linux/reciprocal_div.h>
+
+static void abs_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, abs((char)0), (char)0);
+ KUNIT_EXPECT_EQ(test, abs((char)42), (char)42);
+ KUNIT_EXPECT_EQ(test, abs((char)-42), (char)42);
+
+ /* The expression in the macro is actually promoted to an int. */
+ KUNIT_EXPECT_EQ(test, abs((short)0), 0);
+ KUNIT_EXPECT_EQ(test, abs((short)42), 42);
+ KUNIT_EXPECT_EQ(test, abs((short)-42), 42);
+
+ KUNIT_EXPECT_EQ(test, abs(0), 0);
+ KUNIT_EXPECT_EQ(test, abs(42), 42);
+ KUNIT_EXPECT_EQ(test, abs(-42), 42);
+
+ KUNIT_EXPECT_EQ(test, abs(0L), 0L);
+ KUNIT_EXPECT_EQ(test, abs(42L), 42L);
+ KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
+
+ KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
+ KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
+ KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
+
+ /* Unsigned types get casted to signed. */
+ KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
+ KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);
+}
+
+static void int_sqrt_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(1UL), 1UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(4UL), 2UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(5UL), 2UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(8UL), 2UL);
+ KUNIT_EXPECT_EQ(test, int_sqrt(1UL << 30), 1UL << 15);
+}
+
+static void round_up_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, round_up(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, round_up(1, 2), 2);
+ KUNIT_EXPECT_EQ(test, round_up(3, 2), 4);
+ KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 2), 1 << 30);
+ KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 1 << 29), 1 << 30);
+}
+
+static void round_down_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, round_down(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, round_down(1, 2), 0);
+ KUNIT_EXPECT_EQ(test, round_down(3, 2), 2);
+ KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 2), (1 << 30) - 2);
+ KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 1 << 29), 1 << 29);
+}
+
+/* These versions can round to numbers that aren't a power of two */
+static void roundup_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, roundup(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, roundup(1, 2), 2);
+ KUNIT_EXPECT_EQ(test, roundup(3, 2), 4);
+ KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 2), 1 << 30);
+ KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 1 << 29), 1 << 30);
+
+ KUNIT_EXPECT_EQ(test, roundup(3, 2), 4);
+ KUNIT_EXPECT_EQ(test, roundup(4, 3), 6);
+}
+
+static void rounddown_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, rounddown(0, 1), 0);
+ KUNIT_EXPECT_EQ(test, rounddown(1, 2), 0);
+ KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2);
+ KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 2), (1 << 30) - 2);
+ KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 1 << 29), 1 << 29);
+
+ KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2);
+ KUNIT_EXPECT_EQ(test, rounddown(4, 3), 3);
+ KUNIT_CASE(roundup_test),
+ KUNIT_CASE(rounddown_test),
+ KUNIT_CASE(div_round_up_test),
+ KUNIT_CASE(div_round_closest_test),
+ KUNIT_CASE_PARAM(gcd_test, gcd_gen_params),
+ KUNIT_CASE_PARAM(lcm_test, lcm_gen_params),
+ KUNIT_CASE_PARAM(reciprocal_div_test, reciprocal_div_gen_params),
+ KUNIT_CASE(reciprocal_scale_test),
+ {}
+};
+
+static struct kunit_suite math_test_suite = {
+ .name = "lib-math",
+ .test_cases = math_test_cases,
+};
+
+kunit_test_suites(&math_test_suite);
+
+MODULE_LICENSE("GPL v2");

base-commit: 7e25f40eab52c57ff6772d27d2aef3640a3237d7
--
2.31.1.368.gbe11c130af-goog

Daniel Latypov

unread,
Apr 16, 2021, 2:10:39 PM4/16/21
to David Gow, Andy Shevchenko, Brendan Higgins, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
Btw another reason to use _MSG is that macro text gets expanded in the
EXPECT calls.
E.g.
[11:07:19] # round_down_test: EXPECTATION FAILED at lib/math/math_kunit.c:67
[11:07:19] Expected ((4) & ~((__typeof__(4))((2)-1))) == 2, but
[11:07:19] ((4) & ~((__typeof__(4))((2)-1))) == 4

(I didn't do so for the non-parameterized tests since they point to
the line number.)

So unless one makes a get_desc function for every test, we should use
_MSG variants to give a more readable "round_down(4, 2)." And the _MSG
variant is just more convenient (and amenable to copy-paste).

David Gow

unread,
Apr 17, 2021, 12:16:50 AM4/17/21
to Daniel Latypov, Andy Shevchenko, Brendan Higgins, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Sat, Apr 17, 2021 at 2:04 AM Daniel Latypov <dlat...@google.com> wrote:
>
> Add basic test coverage for files that don't require any config options:
> * part of math.h (what seem to be the most commonly used macros)
> * gcd.c
> * lcm.c
> * int_sqrt.c
> * reciprocal_div.c
> (Ignored int_pow.c since it's a simple textbook algorithm.)
>
> These tests aren't particularly interesting, but they
> * provide short and simple examples of parameterized tests
> * provide a place to add tests for any new files in this dir
> * are written so adding new test cases to cover edge cases should be easy
> * looking at code coverage, we hit all the branches in the .c files
>
> Signed-off-by: Daniel Latypov <dlat...@google.com>
> Reviewed-by: David Gow <davi...@google.com>
> ---

Thanks: I've tested this version, and am happy with it. A part of me
still kind-of would like there to be names for the parameters, but I
definitely understand that it doesn't really work well for the lcm and
gcd cases where we're doing both (a,b) and (b,a). So let's keep it
as-is.

Hopefully we can get these in for 5.13!

Cheers,
-- David

kernel test robot

unread,
Apr 17, 2021, 1:59:23 AM4/17/21
to Daniel Latypov, andriy.s...@linux.intel.com, kbuil...@lists.01.org, brendan...@google.com, davi...@google.com, linux-...@vger.kernel.org, kuni...@googlegroups.com, linux-k...@vger.kernel.org, sk...@linuxfoundation.org, Daniel Latypov
Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 7e25f40eab52c57ff6772d27d2aef3640a3237d7]

url: https://github.com/0day-ci/linux/commits/Daniel-Latypov/lib-add-basic-KUnit-test-for-lib-math/20210417-020619
base: 7e25f40eab52c57ff6772d27d2aef3640a3237d7
config: powerpc-randconfig-c004-20210416 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0f1888ffeaa6baa1bc2a99eac8ba7d1df29c8450
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Latypov/lib-add-basic-KUnit-test-for-lib-math/20210417-020619
git checkout 0f1888ffeaa6baa1bc2a99eac8ba7d1df29c8450
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>

All warnings (new ones prefixed by >>):

lib/math/math_kunit.c: In function 'abs_test':
>> lib/math/math_kunit.c:41:1: warning: the frame size of 1088 bytes is larger than 1024 bytes [-Wframe-larger-than=]
41 | }
| ^


vim +41 lib/math/math_kunit.c

14
15 static void abs_test(struct kunit *test)
16 {
17 KUNIT_EXPECT_EQ(test, abs((char)0), (char)0);
18 KUNIT_EXPECT_EQ(test, abs((char)42), (char)42);
19 KUNIT_EXPECT_EQ(test, abs((char)-42), (char)42);
20
21 /* The expression in the macro is actually promoted to an int. */
22 KUNIT_EXPECT_EQ(test, abs((short)0), 0);
23 KUNIT_EXPECT_EQ(test, abs((short)42), 42);
24 KUNIT_EXPECT_EQ(test, abs((short)-42), 42);
25
26 KUNIT_EXPECT_EQ(test, abs(0), 0);
27 KUNIT_EXPECT_EQ(test, abs(42), 42);
28 KUNIT_EXPECT_EQ(test, abs(-42), 42);
29
30 KUNIT_EXPECT_EQ(test, abs(0L), 0L);
31 KUNIT_EXPECT_EQ(test, abs(42L), 42L);
32 KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
33
34 KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
35 KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
36 KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
37
38 /* Unsigned types get casted to signed. */
39 KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
40 KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);
> 41 }
42

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuil...@lists.01.org
.config.gz

Daniel Latypov

unread,
Sep 2, 2021, 10:26:30 PM9/2/21
to andriy.s...@linux.intel.com, brendan...@google.com, davi...@google.com, linux-...@vger.kernel.org, kuni...@googlegroups.com, linux-k...@vger.kernel.org, sk...@linuxfoundation.org
On Mon, Apr 12, 2021 at 12:07 PM Daniel Latypov <dlat...@google.com> wrote:
>
> Add basic test coverage for files that don't require any config options:
> * part of math.h (what seem to be the most commonly used macros)
> * gcd.c
> * lcm.c
> * int_sqrt.c
> * reciprocal_div.c
> (Ignored int_pow.c since it's a simple textbook algorithm.)
>
> These tests aren't particularly interesting, but they
> * provide short and simple examples of parameterized tests
> * provide a place to add tests for any new files in this dir
> * are written so adding new test cases to cover edge cases should be easy
> * looking at code coverage, we hit all the branches in the .c files

Bumping this thread to see what the status is.

I think it's useful to have tests spread across the kernel so there's
"nearby" tests one can reference and/or copy-paste from.
Now there's the lib/math/rational-test.c, there's less need here.

But I think having tests for simpler functions is still nice to have.

E.g. there's a performance trade-off documented in gcd.c
If you'd want to run that test case to see if it still holds, you'd be
able to run gcd() in a loop fairly easily with:

$ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/math lib-math.gcd_test

(KUnit doesn't yet have support for timing tests or running tests
multiple times, so you'd have to tweak the test code for that).

Random fact:
I'd noticed that after running some 1000s of internal integration
tests , the gcd() function didn't actually get fully covered.
Specifically, they never got to line 34, the for loop
23 unsigned long gcd(unsigned long a, unsigned long b)
24 {
25 unsigned long r = a | b;
26
27 if (!a || !b)
28 return r;
29
30 b >>= __ffs(b);
31 if (b == 1)
32 return r & -r;
33
34 for (;;) {
Checking again now, a few months later, I see they now do hit that loop.
But I guess until then, we'd only been calling gcd() with b=2^n.
Reply all
Reply to author
Forward
0 new messages