diff mbox series

[3/6] lib: Move KUnit tests into tests/ subdirectory

Message ID 20241011072509.3068328-5-davidgow@google.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

David Gow Oct. 11, 2024, 7:25 a.m. UTC
From: Kees Cook <kees@kernel.org>

Following from the recent KUnit file naming discussion[1], move all
KUnit tests in lib/ into lib/tests/.

Link: https://lore.kernel.org/lkml/20240720165441.it.320-kees@kernel.org/ [1]
Signed-off-by: Kees Cook <kees@kernel.org>
Acked-by: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Reviewed-by: David Gow <davidgow@google.com>
[Rebased onto mm-nonmm-unstable, moved usercopy_kunit]
Signed-off-by: David Gow <davidgow@google.com>
---
 MAINTAINERS                            | 19 +++++++------
 lib/Makefile                           | 36 +-----------------------
 lib/tests/Makefile                     | 38 ++++++++++++++++++++++++++
 lib/{ => tests}/bitfield_kunit.c       |  0
 lib/{ => tests}/checksum_kunit.c       |  0
 lib/{ => tests}/cmdline_kunit.c        |  0
 lib/{ => tests}/cpumask_kunit.c        |  0
 lib/{ => tests}/fortify_kunit.c        |  0
 lib/{ => tests}/hashtable_test.c       |  0
 lib/{ => tests}/is_signed_type_kunit.c |  0
 lib/{ => tests}/kunit_iov_iter.c       |  0
 lib/{ => tests}/list-test.c            |  0
 lib/{ => tests}/memcpy_kunit.c         |  0
 lib/{ => tests}/overflow_kunit.c       |  0
 lib/{ => tests}/siphash_kunit.c        |  0
 lib/{ => tests}/slub_kunit.c           |  0
 lib/{ => tests}/stackinit_kunit.c      |  0
 lib/{ => tests}/string_helpers_kunit.c |  0
 lib/{ => tests}/string_kunit.c         |  0
 lib/{ => tests}/test_bits.c            |  0
 lib/{ => tests}/test_fprobe.c          |  0
 lib/{ => tests}/test_hash.c            |  0
 lib/{ => tests}/test_kprobes.c         |  0
 lib/{ => tests}/test_linear_ranges.c   |  0
 lib/{ => tests}/test_list_sort.c       |  0
 lib/{ => tests}/test_sort.c            |  0
 lib/{ => tests}/usercopy_kunit.c       |  0
 27 files changed, 49 insertions(+), 44 deletions(-)
 create mode 100644 lib/tests/Makefile
 rename lib/{ => tests}/bitfield_kunit.c (100%)
 rename lib/{ => tests}/checksum_kunit.c (100%)
 rename lib/{ => tests}/cmdline_kunit.c (100%)
 rename lib/{ => tests}/cpumask_kunit.c (100%)
 rename lib/{ => tests}/fortify_kunit.c (100%)
 rename lib/{ => tests}/hashtable_test.c (100%)
 rename lib/{ => tests}/is_signed_type_kunit.c (100%)
 rename lib/{ => tests}/kunit_iov_iter.c (100%)
 rename lib/{ => tests}/list-test.c (100%)
 rename lib/{ => tests}/memcpy_kunit.c (100%)
 rename lib/{ => tests}/overflow_kunit.c (100%)
 rename lib/{ => tests}/siphash_kunit.c (100%)
 rename lib/{ => tests}/slub_kunit.c (100%)
 rename lib/{ => tests}/stackinit_kunit.c (100%)
 rename lib/{ => tests}/string_helpers_kunit.c (100%)
 rename lib/{ => tests}/string_kunit.c (100%)
 rename lib/{ => tests}/test_bits.c (100%)
 rename lib/{ => tests}/test_fprobe.c (100%)
 rename lib/{ => tests}/test_hash.c (100%)
 rename lib/{ => tests}/test_kprobes.c (100%)
 rename lib/{ => tests}/test_linear_ranges.c (100%)
 rename lib/{ => tests}/test_list_sort.c (100%)
 rename lib/{ => tests}/test_sort.c (100%)
 rename lib/{ => tests}/usercopy_kunit.c (100%)

Comments

Vlastimil Babka Oct. 11, 2024, 9:17 a.m. UTC | #1
On 10/11/24 9:25 AM, David Gow wrote:
> From: Kees Cook <kees@kernel.org>
> 
> Following from the recent KUnit file naming discussion[1], move all
> KUnit tests in lib/ into lib/tests/.
> 
> Link: https://lore.kernel.org/lkml/20240720165441.it.320-kees@kernel.org/ [1]
> Signed-off-by: Kees Cook <kees@kernel.org>
> Acked-by: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> Acked-by: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>

Acked-by: Vlastimil Babka <vbabka@suse.cz> #slab

> Reviewed-by: David Gow <davidgow@google.com>
> [Rebased onto mm-nonmm-unstable, moved usercopy_kunit]
> Signed-off-by: David Gow <davidgow@google.com>
Andy Shevchenko Oct. 11, 2024, 10:44 a.m. UTC | #2
On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:
> From: Kees Cook <kees@kernel.org>
> 
> Following from the recent KUnit file naming discussion[1], move all
> KUnit tests in lib/ into lib/tests/.
> 
> Link: https://lore.kernel.org/lkml/20240720165441.it.320-kees@kernel.org/ [1]
> Signed-off-by: Kees Cook <kees@kernel.org>
> Acked-by: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> Acked-by: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
> Reviewed-by: David Gow <davidgow@google.com>
> [Rebased onto mm-nonmm-unstable, moved usercopy_kunit]
> Signed-off-by: David Gow <davidgow@google.com>

>  rename lib/{ => tests}/bitfield_kunit.c (100%)
>  rename lib/{ => tests}/checksum_kunit.c (100%)
>  rename lib/{ => tests}/cmdline_kunit.c (100%)
>  rename lib/{ => tests}/cpumask_kunit.c (100%)
>  rename lib/{ => tests}/fortify_kunit.c (100%)
>  rename lib/{ => tests}/hashtable_test.c (100%)
>  rename lib/{ => tests}/is_signed_type_kunit.c (100%)
>  rename lib/{ => tests}/kunit_iov_iter.c (100%)
>  rename lib/{ => tests}/list-test.c (100%)
>  rename lib/{ => tests}/memcpy_kunit.c (100%)
>  rename lib/{ => tests}/overflow_kunit.c (100%)
>  rename lib/{ => tests}/siphash_kunit.c (100%)
>  rename lib/{ => tests}/slub_kunit.c (100%)
>  rename lib/{ => tests}/stackinit_kunit.c (100%)
>  rename lib/{ => tests}/string_helpers_kunit.c (100%)
>  rename lib/{ => tests}/string_kunit.c (100%)
>  rename lib/{ => tests}/test_bits.c (100%)
>  rename lib/{ => tests}/test_fprobe.c (100%)
>  rename lib/{ => tests}/test_hash.c (100%)
>  rename lib/{ => tests}/test_kprobes.c (100%)
>  rename lib/{ => tests}/test_linear_ranges.c (100%)
>  rename lib/{ => tests}/test_list_sort.c (100%)
>  rename lib/{ => tests}/test_sort.c (100%)
>  rename lib/{ => tests}/usercopy_kunit.c (100%)

While I support the idea, I think this adds an additional churn in creating a
duplicate 'test' in the filenames. Why they all can't be cut while removing?
(at least this question is not answered in the commit message)
Geert Uytterhoeven Oct. 11, 2024, 1:38 p.m. UTC | #3
Hi Andy,

On Fri, Oct 11, 2024 at 12:44 PM Andy Shevchenko <andy@kernel.org> wrote:
> On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:
> > From: Kees Cook <kees@kernel.org>
> >
> > Following from the recent KUnit file naming discussion[1], move all
> > KUnit tests in lib/ into lib/tests/.
> >
> > Link: https://lore.kernel.org/lkml/20240720165441.it.320-kees@kernel.org/ [1]
> > Signed-off-by: Kees Cook <kees@kernel.org>
> > Acked-by: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > Acked-by: Jakub Kicinski <kuba@kernel.org>
> > Acked-by: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
> > Reviewed-by: David Gow <davidgow@google.com>
> > [Rebased onto mm-nonmm-unstable, moved usercopy_kunit]
> > Signed-off-by: David Gow <davidgow@google.com>
>
> >  rename lib/{ => tests}/bitfield_kunit.c (100%)
> >  rename lib/{ => tests}/checksum_kunit.c (100%)
> >  rename lib/{ => tests}/cmdline_kunit.c (100%)
> >  rename lib/{ => tests}/cpumask_kunit.c (100%)
> >  rename lib/{ => tests}/fortify_kunit.c (100%)
> >  rename lib/{ => tests}/hashtable_test.c (100%)
> >  rename lib/{ => tests}/is_signed_type_kunit.c (100%)
> >  rename lib/{ => tests}/kunit_iov_iter.c (100%)
> >  rename lib/{ => tests}/list-test.c (100%)
> >  rename lib/{ => tests}/memcpy_kunit.c (100%)
> >  rename lib/{ => tests}/overflow_kunit.c (100%)
> >  rename lib/{ => tests}/siphash_kunit.c (100%)
> >  rename lib/{ => tests}/slub_kunit.c (100%)
> >  rename lib/{ => tests}/stackinit_kunit.c (100%)
> >  rename lib/{ => tests}/string_helpers_kunit.c (100%)
> >  rename lib/{ => tests}/string_kunit.c (100%)
> >  rename lib/{ => tests}/test_bits.c (100%)
> >  rename lib/{ => tests}/test_fprobe.c (100%)
> >  rename lib/{ => tests}/test_hash.c (100%)
> >  rename lib/{ => tests}/test_kprobes.c (100%)
> >  rename lib/{ => tests}/test_linear_ranges.c (100%)
> >  rename lib/{ => tests}/test_list_sort.c (100%)
> >  rename lib/{ => tests}/test_sort.c (100%)
> >  rename lib/{ => tests}/usercopy_kunit.c (100%)
>
> While I support the idea, I think this adds an additional churn in creating a
> duplicate 'test' in the filenames. Why they all can't be cut while removing?
> (at least this question is not answered in the commit message)

To avoid duplicate *.ko file names?

Gr{oetje,eeting}s,

                        Geert
Andy Shevchenko Oct. 11, 2024, 1:57 p.m. UTC | #4
On Fri, Oct 11, 2024 at 03:38:00PM +0200, Geert Uytterhoeven wrote:
> Hi Andy,
> 
> On Fri, Oct 11, 2024 at 12:44 PM Andy Shevchenko <andy@kernel.org> wrote:
> > On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:
> > > From: Kees Cook <kees@kernel.org>
> > >
> > > Following from the recent KUnit file naming discussion[1], move all
> > > KUnit tests in lib/ into lib/tests/.
> > >
> > > Link: https://lore.kernel.org/lkml/20240720165441.it.320-kees@kernel.org/ [1]
> > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > Acked-by: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > > Acked-by: Jakub Kicinski <kuba@kernel.org>
> > > Acked-by: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
> > > Reviewed-by: David Gow <davidgow@google.com>
> > > [Rebased onto mm-nonmm-unstable, moved usercopy_kunit]
> > > Signed-off-by: David Gow <davidgow@google.com>
> >
> > >  rename lib/{ => tests}/bitfield_kunit.c (100%)
> > >  rename lib/{ => tests}/checksum_kunit.c (100%)
> > >  rename lib/{ => tests}/cmdline_kunit.c (100%)
> > >  rename lib/{ => tests}/cpumask_kunit.c (100%)
> > >  rename lib/{ => tests}/fortify_kunit.c (100%)
> > >  rename lib/{ => tests}/hashtable_test.c (100%)
> > >  rename lib/{ => tests}/is_signed_type_kunit.c (100%)
> > >  rename lib/{ => tests}/kunit_iov_iter.c (100%)
> > >  rename lib/{ => tests}/list-test.c (100%)
> > >  rename lib/{ => tests}/memcpy_kunit.c (100%)
> > >  rename lib/{ => tests}/overflow_kunit.c (100%)
> > >  rename lib/{ => tests}/siphash_kunit.c (100%)
> > >  rename lib/{ => tests}/slub_kunit.c (100%)
> > >  rename lib/{ => tests}/stackinit_kunit.c (100%)
> > >  rename lib/{ => tests}/string_helpers_kunit.c (100%)
> > >  rename lib/{ => tests}/string_kunit.c (100%)
> > >  rename lib/{ => tests}/test_bits.c (100%)
> > >  rename lib/{ => tests}/test_fprobe.c (100%)
> > >  rename lib/{ => tests}/test_hash.c (100%)
> > >  rename lib/{ => tests}/test_kprobes.c (100%)
> > >  rename lib/{ => tests}/test_linear_ranges.c (100%)
> > >  rename lib/{ => tests}/test_list_sort.c (100%)
> > >  rename lib/{ => tests}/test_sort.c (100%)
> > >  rename lib/{ => tests}/usercopy_kunit.c (100%)
> >
> > While I support the idea, I think this adds an additional churn in creating a
> > duplicate 'test' in the filenames. Why they all can't be cut while removing?
> > (at least this question is not answered in the commit message)
> 
> To avoid duplicate *.ko file names?

With what? Sorry, but I don't see how it's a problem. These are test cases.
Do they use kernel command line parameters? If so, shouldn't KUnit take care
about it in a more proper way?
Geert Uytterhoeven Oct. 11, 2024, 2:14 p.m. UTC | #5
Hi Andy,

On Fri, Oct 11, 2024 at 3:57 PM Andy Shevchenko <andy@kernel.org> wrote:
> On Fri, Oct 11, 2024 at 03:38:00PM +0200, Geert Uytterhoeven wrote:
> > On Fri, Oct 11, 2024 at 12:44 PM Andy Shevchenko <andy@kernel.org> wrote:
> > > On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:
> > > > From: Kees Cook <kees@kernel.org>
> > > >
> > > > Following from the recent KUnit file naming discussion[1], move all
> > > > KUnit tests in lib/ into lib/tests/.
> > > >
> > > > Link: https://lore.kernel.org/lkml/20240720165441.it.320-kees@kernel.org/ [1]
> > > > Signed-off-by: Kees Cook <kees@kernel.org>
> > > > Acked-by: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > > > Acked-by: Jakub Kicinski <kuba@kernel.org>
> > > > Acked-by: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
> > > > Reviewed-by: David Gow <davidgow@google.com>
> > > > [Rebased onto mm-nonmm-unstable, moved usercopy_kunit]
> > > > Signed-off-by: David Gow <davidgow@google.com>
> > >
> > > >  rename lib/{ => tests}/bitfield_kunit.c (100%)
> > > >  rename lib/{ => tests}/checksum_kunit.c (100%)
> > > >  rename lib/{ => tests}/cmdline_kunit.c (100%)
> > > >  rename lib/{ => tests}/cpumask_kunit.c (100%)
> > > >  rename lib/{ => tests}/fortify_kunit.c (100%)
> > > >  rename lib/{ => tests}/hashtable_test.c (100%)
> > > >  rename lib/{ => tests}/is_signed_type_kunit.c (100%)
> > > >  rename lib/{ => tests}/kunit_iov_iter.c (100%)
> > > >  rename lib/{ => tests}/list-test.c (100%)
> > > >  rename lib/{ => tests}/memcpy_kunit.c (100%)
> > > >  rename lib/{ => tests}/overflow_kunit.c (100%)
> > > >  rename lib/{ => tests}/siphash_kunit.c (100%)
> > > >  rename lib/{ => tests}/slub_kunit.c (100%)
> > > >  rename lib/{ => tests}/stackinit_kunit.c (100%)
> > > >  rename lib/{ => tests}/string_helpers_kunit.c (100%)
> > > >  rename lib/{ => tests}/string_kunit.c (100%)
> > > >  rename lib/{ => tests}/test_bits.c (100%)
> > > >  rename lib/{ => tests}/test_fprobe.c (100%)
> > > >  rename lib/{ => tests}/test_hash.c (100%)
> > > >  rename lib/{ => tests}/test_kprobes.c (100%)
> > > >  rename lib/{ => tests}/test_linear_ranges.c (100%)
> > > >  rename lib/{ => tests}/test_list_sort.c (100%)
> > > >  rename lib/{ => tests}/test_sort.c (100%)
> > > >  rename lib/{ => tests}/usercopy_kunit.c (100%)
> > >
> > > While I support the idea, I think this adds an additional churn in creating a
> > > duplicate 'test' in the filenames. Why they all can't be cut while removing?
> > > (at least this question is not answered in the commit message)
> >
> > To avoid duplicate *.ko file names?
>
> With what? Sorry, but I don't see how it's a problem. These are test cases.
> Do they use kernel command line parameters? If so, shouldn't KUnit take care
> about it in a more proper way?

If .e.g. lib/list_sort.o could be modular, its module would be called
"list_sort.ko", conflicting with the "list_sort.ko" test module.

Gr{oetje,eeting}s,

                        Geert
Andy Shevchenko Oct. 11, 2024, 3:21 p.m. UTC | #6
On Fri, Oct 11, 2024 at 5:20 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Oct 11, 2024 at 3:57 PM Andy Shevchenko <andy@kernel.org> wrote:
> > On Fri, Oct 11, 2024 at 03:38:00PM +0200, Geert Uytterhoeven wrote:
> > > On Fri, Oct 11, 2024 at 12:44 PM Andy Shevchenko <andy@kernel.org> wrote:
> > > > On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:

...

> > > > >  rename lib/{ => tests}/bitfield_kunit.c (100%)
> > > > >  rename lib/{ => tests}/checksum_kunit.c (100%)
> > > > >  rename lib/{ => tests}/cmdline_kunit.c (100%)
> > > > >  rename lib/{ => tests}/cpumask_kunit.c (100%)
> > > > >  rename lib/{ => tests}/fortify_kunit.c (100%)
> > > > >  rename lib/{ => tests}/hashtable_test.c (100%)
> > > > >  rename lib/{ => tests}/is_signed_type_kunit.c (100%)
> > > > >  rename lib/{ => tests}/kunit_iov_iter.c (100%)
> > > > >  rename lib/{ => tests}/list-test.c (100%)
> > > > >  rename lib/{ => tests}/memcpy_kunit.c (100%)
> > > > >  rename lib/{ => tests}/overflow_kunit.c (100%)
> > > > >  rename lib/{ => tests}/siphash_kunit.c (100%)
> > > > >  rename lib/{ => tests}/slub_kunit.c (100%)
> > > > >  rename lib/{ => tests}/stackinit_kunit.c (100%)
> > > > >  rename lib/{ => tests}/string_helpers_kunit.c (100%)
> > > > >  rename lib/{ => tests}/string_kunit.c (100%)
> > > > >  rename lib/{ => tests}/test_bits.c (100%)
> > > > >  rename lib/{ => tests}/test_fprobe.c (100%)
> > > > >  rename lib/{ => tests}/test_hash.c (100%)
> > > > >  rename lib/{ => tests}/test_kprobes.c (100%)
> > > > >  rename lib/{ => tests}/test_linear_ranges.c (100%)
> > > > >  rename lib/{ => tests}/test_list_sort.c (100%)
> > > > >  rename lib/{ => tests}/test_sort.c (100%)
> > > > >  rename lib/{ => tests}/usercopy_kunit.c (100%)
> > > >
> > > > While I support the idea, I think this adds an additional churn in creating a
> > > > duplicate 'test' in the filenames. Why they all can't be cut while removing?
> > > > (at least this question is not answered in the commit message)
> > >
> > > To avoid duplicate *.ko file names?
> >
> > With what? Sorry, but I don't see how it's a problem. These are test cases.
> > Do they use kernel command line parameters? If so, shouldn't KUnit take care
> > about it in a more proper way?
>
> If .e.g. lib/list_sort.o could be modular, its module would be called
> "list_sort.ko", conflicting with the "list_sort.ko" test module.

Can't this be solved by automatically adding a prefix in Makefile for
kunit tests, for example?
Christophe Leroy Oct. 11, 2024, 4:29 p.m. UTC | #7
Le 11/10/2024 à 17:21, Andy Shevchenko a écrit :
> [Vous ne recevez pas souvent de courriers de andy.shevchenko@gmail.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Fri, Oct 11, 2024 at 5:20 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Fri, Oct 11, 2024 at 3:57 PM Andy Shevchenko <andy@kernel.org> wrote:
>>> On Fri, Oct 11, 2024 at 03:38:00PM +0200, Geert Uytterhoeven wrote:
>>>> On Fri, Oct 11, 2024 at 12:44 PM Andy Shevchenko <andy@kernel.org> wrote:
>>>>> On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:
> 
> ...
> 
>>>>>>   rename lib/{ => tests}/bitfield_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/checksum_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/cmdline_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/cpumask_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/fortify_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/hashtable_test.c (100%)
>>>>>>   rename lib/{ => tests}/is_signed_type_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/kunit_iov_iter.c (100%)
>>>>>>   rename lib/{ => tests}/list-test.c (100%)
>>>>>>   rename lib/{ => tests}/memcpy_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/overflow_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/siphash_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/slub_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/stackinit_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/string_helpers_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/string_kunit.c (100%)
>>>>>>   rename lib/{ => tests}/test_bits.c (100%)
>>>>>>   rename lib/{ => tests}/test_fprobe.c (100%)
>>>>>>   rename lib/{ => tests}/test_hash.c (100%)
>>>>>>   rename lib/{ => tests}/test_kprobes.c (100%)
>>>>>>   rename lib/{ => tests}/test_linear_ranges.c (100%)
>>>>>>   rename lib/{ => tests}/test_list_sort.c (100%)
>>>>>>   rename lib/{ => tests}/test_sort.c (100%)
>>>>>>   rename lib/{ => tests}/usercopy_kunit.c (100%)
>>>>>
>>>>> While I support the idea, I think this adds an additional churn in creating a
>>>>> duplicate 'test' in the filenames. Why they all can't be cut while removing?
>>>>> (at least this question is not answered in the commit message)
>>>>
>>>> To avoid duplicate *.ko file names?
>>>
>>> With what? Sorry, but I don't see how it's a problem. These are test cases.
>>> Do they use kernel command line parameters? If so, shouldn't KUnit take care
>>> about it in a more proper way?
>>
>> If .e.g. lib/list_sort.o could be modular, its module would be called
>> "list_sort.ko", conflicting with the "list_sort.ko" test module.

But as it is now in lib/tests/, the module will also be installed in 
tests/ subdir, so it shouldn't clash anymore ?

You'd have:

/lib/modules/lib/list_sort.ko and
/lib/modules/lib/tests/list_sort.ko

Or did I miss something ?
> 
> Can't this be solved by automatically adding a prefix in Makefile for
> kunit tests, for example?
> 
> --
> With Best Regards,
> Andy Shevchenko
Kees Cook Oct. 11, 2024, 5:20 p.m. UTC | #8
On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:
> From: Kees Cook <kees@kernel.org>
> 
> Following from the recent KUnit file naming discussion[1], move all
> KUnit tests in lib/ into lib/tests/.

Thanks for picking this up! I hadn't had time to resend it. :)
Andy Shevchenko Oct. 11, 2024, 5:22 p.m. UTC | #9
On Fri, Oct 11, 2024 at 06:29:49PM +0200, Christophe Leroy wrote:
> Le 11/10/2024 à 17:21, Andy Shevchenko a écrit :
> > [Vous ne recevez pas souvent de courriers de andy.shevchenko@gmail.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > On Fri, Oct 11, 2024 at 5:20 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Fri, Oct 11, 2024 at 3:57 PM Andy Shevchenko <andy@kernel.org> wrote:
> > > > On Fri, Oct 11, 2024 at 03:38:00PM +0200, Geert Uytterhoeven wrote:
> > > > > On Fri, Oct 11, 2024 at 12:44 PM Andy Shevchenko <andy@kernel.org> wrote:
> > > > > > On Fri, Oct 11, 2024 at 03:25:07PM +0800, David Gow wrote:

...

> > > > > > >   rename lib/{ => tests}/bitfield_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/checksum_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/cmdline_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/cpumask_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/fortify_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/hashtable_test.c (100%)
> > > > > > >   rename lib/{ => tests}/is_signed_type_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/kunit_iov_iter.c (100%)
> > > > > > >   rename lib/{ => tests}/list-test.c (100%)
> > > > > > >   rename lib/{ => tests}/memcpy_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/overflow_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/siphash_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/slub_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/stackinit_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/string_helpers_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/string_kunit.c (100%)
> > > > > > >   rename lib/{ => tests}/test_bits.c (100%)
> > > > > > >   rename lib/{ => tests}/test_fprobe.c (100%)
> > > > > > >   rename lib/{ => tests}/test_hash.c (100%)
> > > > > > >   rename lib/{ => tests}/test_kprobes.c (100%)
> > > > > > >   rename lib/{ => tests}/test_linear_ranges.c (100%)
> > > > > > >   rename lib/{ => tests}/test_list_sort.c (100%)
> > > > > > >   rename lib/{ => tests}/test_sort.c (100%)
> > > > > > >   rename lib/{ => tests}/usercopy_kunit.c (100%)
> > > > > > 
> > > > > > While I support the idea, I think this adds an additional churn in creating a
> > > > > > duplicate 'test' in the filenames. Why they all can't be cut while removing?
> > > > > > (at least this question is not answered in the commit message)
> > > > > 
> > > > > To avoid duplicate *.ko file names?
> > > > 
> > > > With what? Sorry, but I don't see how it's a problem. These are test cases.
> > > > Do they use kernel command line parameters? If so, shouldn't KUnit take care
> > > > about it in a more proper way?
> > > 
> > > If .e.g. lib/list_sort.o could be modular, its module would be called
> > > "list_sort.ko", conflicting with the "list_sort.ko" test module.
> 
> But as it is now in lib/tests/, the module will also be installed in tests/
> subdir, so it shouldn't clash anymore ?
> 
> You'd have:
> 
> /lib/modules/lib/list_sort.ko and
> /lib/modules/lib/tests/list_sort.ko
> 
> Or did I miss something ?

I believe Geert is talking about `modprobe list-sort` in the userspace.
Which one will be loaded?

> > Can't this be solved by automatically adding a prefix in Makefile for
> > kunit tests, for example?
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index f1e1cd652699..bbba5c760533 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3929,10 +3929,10 @@  F:	include/vdso/bits.h
 F:	lib/bitmap-str.c
 F:	lib/bitmap.c
 F:	lib/cpumask.c
-F:	lib/cpumask_kunit.c
 F:	lib/find_bit.c
 F:	lib/find_bit_benchmark.c
 F:	lib/test_bitmap.c
+F:	lib/tests/cpumask_kunit.c
 F:	tools/include/linux/bitfield.h
 F:	tools/include/linux/bitmap.h
 F:	tools/include/linux/bits.h
@@ -8911,9 +8911,10 @@  L:	linux-hardening@vger.kernel.org
 S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
 F:	include/linux/fortify-string.h
-F:	lib/fortify_kunit.c
-F:	lib/memcpy_kunit.c
 F:	lib/test_fortify/*
+F:	lib/tests/fortify_kunit.c
+F:	lib/tests/memcpy_kunit.c
+F:	scripts/test_fortify.sh
 K:	\bunsafe_memcpy\b
 K:	\b__NO_FORTIFY\b
 
@@ -9578,9 +9579,9 @@  F:	include/linux/string.h
 F:	include/linux/string_choices.h
 F:	include/linux/string_helpers.h
 F:	lib/string.c
-F:	lib/string_kunit.c
 F:	lib/string_helpers.c
-F:	lib/string_helpers_kunit.c
+F:	lib/tests/string_helpers_kunit.c
+F:	lib/tests/string_kunit.c
 F:	scripts/coccinelle/api/string_choices.cocci
 
 GENERIC UIO DRIVER FOR PCI DEVICES
@@ -12741,7 +12742,7 @@  F:	Documentation/trace/kprobes.rst
 F:	include/asm-generic/kprobes.h
 F:	include/linux/kprobes.h
 F:	kernel/kprobes.c
-F:	lib/test_kprobes.c
+F:	lib/tests/test_kprobes.c
 F:	samples/kprobes
 
 KS0108 LCD CONTROLLER DRIVER
@@ -13091,7 +13092,7 @@  M:	Mark Brown <broonie@kernel.org>
 R:	Matti Vaittinen <mazziesaccount@gmail.com>
 F:	include/linux/linear_range.h
 F:	lib/linear_ranges.c
-F:	lib/test_linear_ranges.c
+F:	lib/tests/test_linear_ranges.c
 
 LINUX FOR POWER MACINTOSH
 L:	linuxppc-dev@lists.ozlabs.org
@@ -13219,7 +13220,7 @@  M:	David Gow <davidgow@google.com>
 L:	linux-kselftest@vger.kernel.org
 L:	kunit-dev@googlegroups.com
 S:	Maintained
-F:	lib/list-test.c
+F:	lib/tests/list-test.c
 
 LITEX PLATFORM
 M:	Karol Gugala <kgugala@antmicro.com>
@@ -21199,7 +21200,7 @@  M:	Jason A. Donenfeld <Jason@zx2c4.com>
 S:	Maintained
 F:	include/linux/siphash.h
 F:	lib/siphash.c
-F:	lib/siphash_kunit.c
+F:	lib/tests/siphash_kunit.c
 
 SIS 190 ETHERNET DRIVER
 M:	Francois Romieu <romieu@fr.zoreil.com>
diff --git a/lib/Makefile b/lib/Makefile
index 53f82de7cbe2..7f2ddbf03fe9 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -51,9 +51,7 @@  obj-y += bcd.o sort.o parser.o debug_locks.o random32.o \
 	 percpu-refcount.o rhashtable.o base64.o \
 	 once.o refcount.o rcuref.o usercopy.o errseq.o bucket_locks.o \
 	 generic-radix-tree.o bitmap-str.o
-obj-$(CONFIG_STRING_KUNIT_TEST) += string_kunit.o
 obj-y += string_helpers.o
-obj-$(CONFIG_STRING_HELPERS_KUNIT_TEST) += string_helpers_kunit.o
 obj-y += hexdump.o
 obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o
 obj-y += kstrtox.o
@@ -64,22 +62,17 @@  obj-$(CONFIG_TEST_DHRY) += test_dhry.o
 obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
 obj-$(CONFIG_TEST_BITOPS) += test_bitops.o
 CFLAGS_test_bitops.o += -Werror
-obj-$(CONFIG_CPUMASK_KUNIT_TEST) += cpumask_kunit.o
 obj-$(CONFIG_TEST_SYSCTL) += test_sysctl.o
-obj-$(CONFIG_TEST_IOV_ITER) += kunit_iov_iter.o
-obj-$(CONFIG_HASH_KUNIT_TEST) += test_hash.o
 obj-$(CONFIG_TEST_IDA) += test_ida.o
 obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o
 CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
 CFLAGS_test_ubsan.o += $(call cc-disable-warning, unused-but-set-variable)
 UBSAN_SANITIZE_test_ubsan.o := y
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
-obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
 obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
 obj-$(CONFIG_TEST_LKM) += test_module.o
 obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
 obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
-obj-$(CONFIG_TEST_SORT) += test_sort.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
 obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
@@ -105,10 +98,7 @@  obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
 obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
 obj-$(CONFIG_TEST_HMM) += test_hmm.o
 obj-$(CONFIG_TEST_FREE_PAGES) += test_free_pages.o
-obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_TEST_REF_TRACKER) += test_ref_tracker.o
-CFLAGS_test_fprobe.o += $(CC_FLAGS_FTRACE)
-obj-$(CONFIG_FPROBE_SANITY_TEST) += test_fprobe.o
 obj-$(CONFIG_TEST_OBJPOOL) += test_objpool.o
 
 obj-$(CONFIG_TEST_FPU) += test_fpu.o
@@ -130,7 +120,7 @@  endif
 obj-$(CONFIG_DEBUG_INFO_REDUCED) += debug_info.o
 CFLAGS_debug_info.o += $(call cc-option, -femit-struct-debug-detailed=any)
 
-obj-y += math/ crypto/
+obj-y += math/ crypto/ tests/
 
 obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
 obj-$(CONFIG_HAS_IOMEM) += iomap_copy.o devres.o
@@ -367,30 +357,6 @@  obj-$(CONFIG_OBJAGG) += objagg.o
 # pldmfw library
 obj-$(CONFIG_PLDMFW) += pldmfw/
 
-# KUnit tests
-CFLAGS_bitfield_kunit.o := $(DISABLE_STRUCTLEAK_PLUGIN)
-obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
-obj-$(CONFIG_CHECKSUM_KUNIT) += checksum_kunit.o
-obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
-obj-$(CONFIG_HASHTABLE_KUNIT_TEST) += hashtable_test.o
-obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
-obj-$(CONFIG_BITS_TEST) += test_bits.o
-obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
-obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
-obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
-obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
-CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare)
-obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
-CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
-obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
-CFLAGS_fortify_kunit.o += $(call cc-disable-warning, unsequenced)
-CFLAGS_fortify_kunit.o += $(call cc-disable-warning, stringop-overread)
-CFLAGS_fortify_kunit.o += $(call cc-disable-warning, stringop-truncation)
-CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
-obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
-obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
-obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
-
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
 
 obj-$(CONFIG_FIRMWARE_TABLE) += fw_table.o
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
new file mode 100644
index 000000000000..41689f0b7c97
--- /dev/null
+++ b/lib/tests/Makefile
@@ -0,0 +1,38 @@ 
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for tests of kernel library functions.
+
+# KUnit tests
+CFLAGS_bitfield_kunit.o := $(DISABLE_STRUCTLEAK_PLUGIN)
+obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
+obj-$(CONFIG_BITS_TEST) += test_bits.o
+obj-$(CONFIG_CHECKSUM_KUNIT) += checksum_kunit.o
+obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
+obj-$(CONFIG_CPUMASK_KUNIT_TEST) += cpumask_kunit.o
+CFLAGS_fortify_kunit.o += $(call cc-disable-warning, unsequenced)
+CFLAGS_fortify_kunit.o += $(call cc-disable-warning, stringop-overread)
+CFLAGS_fortify_kunit.o += $(call cc-disable-warning, stringop-truncation)
+CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
+obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
+CFLAGS_test_fprobe.o += $(CC_FLAGS_FTRACE)
+obj-$(CONFIG_FPROBE_SANITY_TEST) += test_fprobe.o
+obj-$(CONFIG_HASHTABLE_KUNIT_TEST) += hashtable_test.o
+obj-$(CONFIG_HASH_KUNIT_TEST) += test_hash.o
+obj-$(CONFIG_TEST_IOV_ITER) += kunit_iov_iter.o
+obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
+obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
+obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
+obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
+obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
+obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
+CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare)
+obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
+obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
+obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
+obj-$(CONFIG_TEST_SORT) += test_sort.o
+CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
+obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
+obj-$(CONFIG_STRING_KUNIT_TEST) += string_kunit.o
+obj-$(CONFIG_STRING_HELPERS_KUNIT_TEST) += string_helpers_kunit.o
+obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
+
diff --git a/lib/bitfield_kunit.c b/lib/tests/bitfield_kunit.c
similarity index 100%
rename from lib/bitfield_kunit.c
rename to lib/tests/bitfield_kunit.c
diff --git a/lib/checksum_kunit.c b/lib/tests/checksum_kunit.c
similarity index 100%
rename from lib/checksum_kunit.c
rename to lib/tests/checksum_kunit.c
diff --git a/lib/cmdline_kunit.c b/lib/tests/cmdline_kunit.c
similarity index 100%
rename from lib/cmdline_kunit.c
rename to lib/tests/cmdline_kunit.c
diff --git a/lib/cpumask_kunit.c b/lib/tests/cpumask_kunit.c
similarity index 100%
rename from lib/cpumask_kunit.c
rename to lib/tests/cpumask_kunit.c
diff --git a/lib/fortify_kunit.c b/lib/tests/fortify_kunit.c
similarity index 100%
rename from lib/fortify_kunit.c
rename to lib/tests/fortify_kunit.c
diff --git a/lib/hashtable_test.c b/lib/tests/hashtable_test.c
similarity index 100%
rename from lib/hashtable_test.c
rename to lib/tests/hashtable_test.c
diff --git a/lib/is_signed_type_kunit.c b/lib/tests/is_signed_type_kunit.c
similarity index 100%
rename from lib/is_signed_type_kunit.c
rename to lib/tests/is_signed_type_kunit.c
diff --git a/lib/kunit_iov_iter.c b/lib/tests/kunit_iov_iter.c
similarity index 100%
rename from lib/kunit_iov_iter.c
rename to lib/tests/kunit_iov_iter.c
diff --git a/lib/list-test.c b/lib/tests/list-test.c
similarity index 100%
rename from lib/list-test.c
rename to lib/tests/list-test.c
diff --git a/lib/memcpy_kunit.c b/lib/tests/memcpy_kunit.c
similarity index 100%
rename from lib/memcpy_kunit.c
rename to lib/tests/memcpy_kunit.c
diff --git a/lib/overflow_kunit.c b/lib/tests/overflow_kunit.c
similarity index 100%
rename from lib/overflow_kunit.c
rename to lib/tests/overflow_kunit.c
diff --git a/lib/siphash_kunit.c b/lib/tests/siphash_kunit.c
similarity index 100%
rename from lib/siphash_kunit.c
rename to lib/tests/siphash_kunit.c
diff --git a/lib/slub_kunit.c b/lib/tests/slub_kunit.c
similarity index 100%
rename from lib/slub_kunit.c
rename to lib/tests/slub_kunit.c
diff --git a/lib/stackinit_kunit.c b/lib/tests/stackinit_kunit.c
similarity index 100%
rename from lib/stackinit_kunit.c
rename to lib/tests/stackinit_kunit.c
diff --git a/lib/string_helpers_kunit.c b/lib/tests/string_helpers_kunit.c
similarity index 100%
rename from lib/string_helpers_kunit.c
rename to lib/tests/string_helpers_kunit.c
diff --git a/lib/string_kunit.c b/lib/tests/string_kunit.c
similarity index 100%
rename from lib/string_kunit.c
rename to lib/tests/string_kunit.c
diff --git a/lib/test_bits.c b/lib/tests/test_bits.c
similarity index 100%
rename from lib/test_bits.c
rename to lib/tests/test_bits.c
diff --git a/lib/test_fprobe.c b/lib/tests/test_fprobe.c
similarity index 100%
rename from lib/test_fprobe.c
rename to lib/tests/test_fprobe.c
diff --git a/lib/test_hash.c b/lib/tests/test_hash.c
similarity index 100%
rename from lib/test_hash.c
rename to lib/tests/test_hash.c
diff --git a/lib/test_kprobes.c b/lib/tests/test_kprobes.c
similarity index 100%
rename from lib/test_kprobes.c
rename to lib/tests/test_kprobes.c
diff --git a/lib/test_linear_ranges.c b/lib/tests/test_linear_ranges.c
similarity index 100%
rename from lib/test_linear_ranges.c
rename to lib/tests/test_linear_ranges.c
diff --git a/lib/test_list_sort.c b/lib/tests/test_list_sort.c
similarity index 100%
rename from lib/test_list_sort.c
rename to lib/tests/test_list_sort.c
diff --git a/lib/test_sort.c b/lib/tests/test_sort.c
similarity index 100%
rename from lib/test_sort.c
rename to lib/tests/test_sort.c
diff --git a/lib/usercopy_kunit.c b/lib/tests/usercopy_kunit.c
similarity index 100%
rename from lib/usercopy_kunit.c
rename to lib/tests/usercopy_kunit.c