This adds the conversion of the test_sort.c to KUnit test.
Please apply this commit first (linux-kselftest/kunit-fixes):
3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
Code Style Documentation: [0]
Fix these warnings Reported-by [email protected]
WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test
The variable sort_test_cases references
the variable __init sort_test
If the reference is valid then annotate the
variable with or __refdata (see linux/init.h) or name the variable
WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test()
The variable sort_test_cases references
the function __init sort_test()
Signed-off-by: Vitor Massaru Iha <[email protected]>
Reported-by: kernel test robot <[email protected]>
Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
---
v2:
* Add Kunit Code Style reference in commit message;
* Fix [email protected] warning report;
---
lib/Kconfig.debug | 26 +++++++++++++++++---------
lib/Makefile | 2 +-
lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
3 files changed, 33 insertions(+), 26 deletions(-)
rename lib/{test_sort.c => sort_kunit.c} (55%)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210d70a1..1fe19e78d7ca 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1874,15 +1874,6 @@ config TEST_MIN_HEAP
If unsure, say N.
-config TEST_SORT
- tristate "Array-based sort test"
- depends on DEBUG_KERNEL || m
- help
- This option enables the self-test function of 'sort()' at boot,
- or at module load time.
-
- If unsure, say N.
-
config KPROBES_SANITY_TEST
bool "Kprobes sanity tests"
depends on DEBUG_KERNEL
@@ -2185,6 +2176,23 @@ config LINEAR_RANGES_TEST
If unsure, say N.
+config SORT_KUNIT
+ tristate "KUnit test for Array-based sort"
+ depends on DEBUG_KERNEL || m
+ help
+ This option enables the KUnit function of 'sort()' at boot,
+ or at module load time.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (http://testanything.org/). Only useful for kernel devs
+ running the KUnit test harness, and not intended for inclusion into a
+ production build.
+
+ 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.
+
config TEST_UDELAY
tristate "udelay test driver"
help
diff --git a/lib/Makefile b/lib/Makefile
index b1c42c10073b..c22bb13b0a08 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -77,7 +77,6 @@ obj-$(CONFIG_TEST_LKM) += test_module.o
obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
-obj-$(CONFIG_TEST_SORT) += test_sort.o
obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
@@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
# KUnit tests
obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
+obj-$(CONFIG_SORT_KUNIT) += sort_kunit.o
diff --git a/lib/test_sort.c b/lib/sort_kunit.c
similarity index 55%
rename from lib/test_sort.c
rename to lib/sort_kunit.c
index 52edbe10f2e5..602a234f1e7d 100644
--- a/lib/test_sort.c
+++ b/lib/sort_kunit.c
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <linux/sort.h>
-#include <linux/slab.h>
-#include <linux/module.h>
+#include <kunit/test.h>
/* a simple boot-time regression test */
@@ -12,13 +11,12 @@ static int __init cmpint(const void *a, const void *b)
return *(int *)a - *(int *)b;
}
-static int __init test_sort_init(void)
+static void __init sort_test(struct kunit *test)
{
- int *a, i, r = 1, err = -ENOMEM;
+ int *a, i, r = 1;
a = kmalloc_array(TEST_LEN, sizeof(*a), GFP_KERNEL);
- if (!a)
- return err;
+ KUNIT_ASSERT_FALSE_MSG(test, a == NULL, "kmalloc_array failed");
for (i = 0; i < TEST_LEN; i++) {
r = (r * 725861) % 6599;
@@ -27,24 +25,25 @@ static int __init test_sort_init(void)
sort(a, TEST_LEN, sizeof(*a), cmpint, NULL);
- err = -EINVAL;
for (i = 0; i < TEST_LEN-1; i++)
if (a[i] > a[i+1]) {
- pr_err("test has failed\n");
+ KUNIT_FAIL(test, "test has failed");
goto exit;
}
- err = 0;
- pr_info("test passed\n");
exit:
kfree(a);
- return err;
}
-static void __exit test_sort_exit(void)
-{
-}
+static struct kunit_case __refdata sort_test_cases[] = {
+ KUNIT_CASE(sort_test),
+ {}
+};
+
+static struct kunit_suite sort_test_suite = {
+ .name = "sort",
+ .test_cases = sort_test_cases,
+};
-module_init(test_sort_init);
-module_exit(test_sort_exit);
+kunit_test_suites(&sort_test_suite);
MODULE_LICENSE("GPL");
base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
--
2.26.2
On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
> This adds the conversion of the test_sort.c to KUnit test.
>
> Please apply this commit first (linux-kselftest/kunit-fixes):
> 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
Looks like you mixed up commit message and changelog / comments.
> Code Style Documentation: [0]
>
> Fix these warnings Reported-by [email protected]
>
> WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test
> The variable sort_test_cases references
> the variable __init sort_test
> If the reference is valid then annotate the
> variable with or __refdata (see linux/init.h) or name the variable
>
> WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test()
> The variable sort_test_cases references
> the function __init sort_test()
> Signed-off-by: Vitor Massaru Iha <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
This should be in different order: Link, Reported-by, SoB.
Also [0] is unnecessary
> lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
Still opened question why kunit is a suffix? Can't we leave same name? Can't we
do it rather prefix?
--
With Best Regards,
Andy Shevchenko
On Wed, Jul 29, 2020 at 12:19 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
> > This adds the conversion of the test_sort.c to KUnit test.
> >
> > Please apply this commit first (linux-kselftest/kunit-fixes):
> > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
>
> Looks like you mixed up commit message and changelog / comments.
>
> > Code Style Documentation: [0]
> >
> > Fix these warnings Reported-by [email protected]
> >
> > WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test
> > The variable sort_test_cases references
> > the variable __init sort_test
> > If the reference is valid then annotate the
> > variable with or __refdata (see linux/init.h) or name the variable
> >
> > WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test()
> > The variable sort_test_cases references
> > the function __init sort_test()
>
> > Signed-off-by: Vitor Massaru Iha <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
> > Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
>
> This should be in different order: Link, Reported-by, SoB.
> Also [0] is unnecessary
>
> > lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
>
> Still opened question why kunit is a suffix? Can't we leave same name? Can't we
> do it rather prefix?
Sorry to jump in now; I thought Vitor's reply with a link to the new
proposed documentation[1] addressed this. The naming convention issue
came up about a month ago[2]. The people in the discussion (including
myself) came to an agreement on _kunit.c. That being said, the
documentation hasn't been accepted yet, so if you really feel strongly
about it, now is the time to change it.
All that being said, I would rather not discuss that issue here for
the benefit of the participants in the preceding discussions.
I posted lore links for the relevant threads, which should be easy
enough to In-Reply-To your way into. Nevertheless, if it makes it
easier, let me know and I can CC you into the discussions.
[1] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
[2] https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
Cheers
On Wed, Jul 29, 2020 at 4:19 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
> > This adds the conversion of the test_sort.c to KUnit test.
> >
> > Please apply this commit first (linux-kselftest/kunit-fixes):
> > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
>
> Looks like you mixed up commit message and changelog / comments.
>
> > Code Style Documentation: [0]
> >
> > Fix these warnings Reported-by [email protected]
> >
> > WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test
> > The variable sort_test_cases references
> > the variable __init sort_test
> > If the reference is valid then annotate the
> > variable with or __refdata (see linux/init.h) or name the variable
> >
> > WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test()
> > The variable sort_test_cases references
> > the function __init sort_test()
>
> > Signed-off-by: Vitor Massaru Iha <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
> > Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
>
> This should be in different order: Link, Reported-by, SoB.
> Also [0] is unnecessary
Sure, thanks!
>
> > lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
>
> Still opened question why kunit is a suffix? Can't we leave same name? Can't we
> do it rather prefix?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Hi Vitor,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on d43c7fb05765152d4d4a39a8ef957c4ea14d8847]
url: https://github.com/0day-ci/linux/commits/Vitor-Massaru-Iha/lib-kunit-Convert-test_sort-to-KUnit-test/20200730-031404
base: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
config: x86_64-randconfig-s022-20200803 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-117-g8c7aee71-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
ld: lib/sort_kunit.o: in function `kunit_test_suites_init':
>> lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_init'
ld: lib/sort_kunit.o: in function `sort_test':
>> lib/sort_kunit.c:19: undefined reference to `kunit_unary_assert_format'
>> ld: lib/sort_kunit.c:19: undefined reference to `kunit_do_assertion'
>> ld: lib/sort_kunit.c:30: undefined reference to `kunit_fail_assert_format'
ld: lib/sort_kunit.c:30: undefined reference to `kunit_do_assertion'
ld: lib/sort_kunit.o: in function `kunit_test_suites_exit':
>> lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_exit'
vim +47 lib/sort_kunit.c
13
14 static void __init sort_test(struct kunit *test)
15 {
16 int *a, i, r = 1;
17
18 a = kmalloc_array(TEST_LEN, sizeof(*a), GFP_KERNEL);
> 19 KUNIT_ASSERT_FALSE_MSG(test, a == NULL, "kmalloc_array failed");
20
21 for (i = 0; i < TEST_LEN; i++) {
22 r = (r * 725861) % 6599;
23 a[i] = r;
24 }
25
26 sort(a, TEST_LEN, sizeof(*a), cmpint, NULL);
27
28 for (i = 0; i < TEST_LEN-1; i++)
29 if (a[i] > a[i+1]) {
> 30 KUNIT_FAIL(test, "test has failed");
31 goto exit;
32 }
33 exit:
34 kfree(a);
35 }
36
37 static struct kunit_case __refdata sort_test_cases[] = {
38 KUNIT_CASE(sort_test),
39 {}
40 };
41
42 static struct kunit_suite sort_test_suite = {
43 .name = "sort",
44 .test_cases = sort_test_cases,
45 };
46
> 47 kunit_test_suites(&sort_test_suite);
48
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Vitor,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on d43c7fb05765152d4d4a39a8ef957c4ea14d8847]
url: https://github.com/0day-ci/linux/commits/Vitor-Massaru-Iha/lib-kunit-Convert-test_sort-to-KUnit-test/20200730-031404
base: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
config: microblaze-randconfig-s031-20200803 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-117-g8c7aee71-dirty
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=microblaze
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
microblaze-linux-ld: lib/sort_kunit.o: in function `kunit_test_suites_init':
lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_init'
microblaze-linux-ld: lib/sort_kunit.o: in function `sort_test':
lib/sort_kunit.c:19: undefined reference to `kunit_unary_assert_format'
>> microblaze-linux-ld: lib/sort_kunit.c:19: undefined reference to `kunit_do_assertion'
>> microblaze-linux-ld: lib/sort_kunit.c:30: undefined reference to `kunit_fail_assert_format'
microblaze-linux-ld: lib/sort_kunit.c:30: undefined reference to `kunit_do_assertion'
microblaze-linux-ld: lib/sort_kunit.o: in function `kunit_test_suites_exit':
lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_exit'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Wed, Jul 29, 2020 at 12:59:28PM -0700, Brendan Higgins wrote:
> On Wed, Jul 29, 2020 at 12:19 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
...
> > > lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
> >
> > Still opened question why kunit is a suffix? Can't we leave same name? Can't we
> > do it rather prefix?
>
> Sorry to jump in now; I thought Vitor's reply with a link to the new
> proposed documentation[1] addressed this. The naming convention issue
> came up about a month ago[2]. The people in the discussion (including
> myself) came to an agreement on _kunit.c. That being said, the
> documentation hasn't been accepted yet, so if you really feel strongly
> about it, now is the time to change it.
My argument is to do something like
- ls .../test* vs. ls .../*_kunit.c
- use shell completion vs. no completion when looking if there is a test
module for something
But I agree that this is matter of style.
> All that being said, I would rather not discuss that issue here for
> the benefit of the participants in the preceding discussions.
>
> I posted lore links for the relevant threads, which should be easy
> enough to In-Reply-To your way into. Nevertheless, if it makes it
> easier, let me know and I can CC you into the discussions.
No need. I think you have enough clever folks and good ideas behind this. Just
put a reference to all these conversion patches to the summary of pros and cons
of renaming.
> [1] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
> [2] https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
--
With Best Regards,
Andy Shevchenko