char: misc: make miscdevice unit test built-in only

[PATCH] char: misc: make miscdevice unit test built-in only

Posted by Thadeu Lima de Souza Cascardo 9 months, 1 week ago

Since it uses __init symbols, it cannot be a module. Builds with
CONFIG_TEST_MISC_MINOR=m will fail with:

ERROR: modpost: "init_mknod" [drivers/misc/misc_minor_kunit.ko] undefined!
ERROR: modpost: "init_unlink" [drivers/misc/misc_minor_kunit.ko] undefined!

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/linux-next/20250429155404.2b6fe5b1@canb.auug.org.au/
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202504160338.BjUL3Owb-lkp@intel.com/
Fixes: 45f0de4f8dc3 ("char: misc: add test cases")
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 lib/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f9051ab610d54358b21d61c141b737bb345b4cee..0117b852bd131b8a585dc02d8225e2e8c0740077 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2512,7 +2512,7 @@ config TEST_IDA
 	tristate "Perform selftest on IDA functions"
 
 config TEST_MISC_MINOR
-	tristate "miscdevice KUnit test" if !KUNIT_ALL_TESTS
+	bool "miscdevice KUnit test" if !KUNIT_ALL_TESTS
 	depends on KUNIT
 	default KUNIT_ALL_TESTS
 	help

---
base-commit: 4f822ad5ee944ffafc21937a32dd055f1df5c28d
change-id: 20250430-misc-test-fixup-90c5afce1d14

Best regards,
-- 
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Re: [PATCH] char: misc: make miscdevice unit test built-in only

Posted by Geert Uytterhoeven 9 months, 1 week ago

Hi Thadeu,

On Wed, 30 Apr 2025 at 18:53, Thadeu Lima de Souza Cascardo
<cascardo@igalia.com> wrote:
> Since it uses __init symbols, it cannot be a module. Builds with
> CONFIG_TEST_MISC_MINOR=m will fail with:
>
> ERROR: modpost: "init_mknod" [drivers/misc/misc_minor_kunit.ko] undefined!
> ERROR: modpost: "init_unlink" [drivers/misc/misc_minor_kunit.ko] undefined!
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Closes: https://lore.kernel.org/linux-next/20250429155404.2b6fe5b1@canb.auug.org.au/
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202504160338.BjUL3Owb-lkp@intel.com/
> Fixes: 45f0de4f8dc3 ("char: misc: add test cases")
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Thanks for your patch, which is now commit 20acf4dd46e4c090 ("char:
misc: make miscdevice unit test built-in only") in char-misc-next.

> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2512,7 +2512,7 @@ config TEST_IDA
>         tristate "Perform selftest on IDA functions"
>
>  config TEST_MISC_MINOR
> -       tristate "miscdevice KUnit test" if !KUNIT_ALL_TESTS
> +       bool "miscdevice KUnit test" if !KUNIT_ALL_TESTS
>         depends on KUNIT
>         default KUNIT_ALL_TESTS

This means "default y" if KUNIT_ALL_TESTS=m, which is IMHO not
what we want.

Perhaps

    default KUNIT_ALL_TESTS=y

?

>         help


Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] char: misc: make miscdevice unit test built-in only

Posted by Thadeu Lima de Souza Cascardo 9 months, 1 week ago

On Mon, May 05, 2025 at 09:21:15AM +0200, Geert Uytterhoeven wrote:
> Hi Thadeu,
> 
> On Wed, 30 Apr 2025 at 18:53, Thadeu Lima de Souza Cascardo
> <cascardo@igalia.com> wrote:
> > Since it uses __init symbols, it cannot be a module. Builds with
> > CONFIG_TEST_MISC_MINOR=m will fail with:
> >
> > ERROR: modpost: "init_mknod" [drivers/misc/misc_minor_kunit.ko] undefined!
> > ERROR: modpost: "init_unlink" [drivers/misc/misc_minor_kunit.ko] undefined!
> >
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Closes: https://lore.kernel.org/linux-next/20250429155404.2b6fe5b1@canb.auug.org.au/
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202504160338.BjUL3Owb-lkp@intel.com/
> > Fixes: 45f0de4f8dc3 ("char: misc: add test cases")
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
> 
> Thanks for your patch, which is now commit 20acf4dd46e4c090 ("char:
> misc: make miscdevice unit test built-in only") in char-misc-next.
> 
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2512,7 +2512,7 @@ config TEST_IDA
> >         tristate "Perform selftest on IDA functions"
> >
> >  config TEST_MISC_MINOR
> > -       tristate "miscdevice KUnit test" if !KUNIT_ALL_TESTS
> > +       bool "miscdevice KUnit test" if !KUNIT_ALL_TESTS
> >         depends on KUNIT
> >         default KUNIT_ALL_TESTS
> 
> This means "default y" if KUNIT_ALL_TESTS=m, which is IMHO not
> what we want.
> 

The precedent for other kunit config options that are bool is that they use
"default KUNIT_ALL_TESTS".

It makes sense that if you choose to build all tests, you would not skip
the ones that cannot be built as a module.

Thanks.
Cascardo.

> Perhaps
> 
>     default KUNIT_ALL_TESTS=y
> 
> ?
> 
> >         help
> 
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH] char: misc: make miscdevice unit test built-in only

Posted by Geert Uytterhoeven 9 months, 1 week ago

Hi Thadeu,

CC kunit

On Mon, 5 May 2025 at 14:13, Thadeu Lima de Souza Cascardo
<cascardo@igalia.com> wrote:
> On Mon, May 05, 2025 at 09:21:15AM +0200, Geert Uytterhoeven wrote:
> > On Wed, 30 Apr 2025 at 18:53, Thadeu Lima de Souza Cascardo
> > <cascardo@igalia.com> wrote:
> > > Since it uses __init symbols, it cannot be a module. Builds with
> > > CONFIG_TEST_MISC_MINOR=m will fail with:
> > >
> > > ERROR: modpost: "init_mknod" [drivers/misc/misc_minor_kunit.ko] undefined!
> > > ERROR: modpost: "init_unlink" [drivers/misc/misc_minor_kunit.ko] undefined!
> > >
> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Closes: https://lore.kernel.org/linux-next/20250429155404.2b6fe5b1@canb.auug.org.au/
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202504160338.BjUL3Owb-lkp@intel.com/
> > > Fixes: 45f0de4f8dc3 ("char: misc: add test cases")
> > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
> >
> > Thanks for your patch, which is now commit 20acf4dd46e4c090 ("char:
> > misc: make miscdevice unit test built-in only") in char-misc-next.
> >
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -2512,7 +2512,7 @@ config TEST_IDA
> > >         tristate "Perform selftest on IDA functions"
> > >
> > >  config TEST_MISC_MINOR
> > > -       tristate "miscdevice KUnit test" if !KUNIT_ALL_TESTS
> > > +       bool "miscdevice KUnit test" if !KUNIT_ALL_TESTS
> > >         depends on KUNIT
> > >         default KUNIT_ALL_TESTS
> >
> > This means "default y" if KUNIT_ALL_TESTS=m, which is IMHO not
> > what we want.
>
> The precedent for other kunit config options that are bool is that they use
> "default KUNIT_ALL_TESTS".

Seems like you are right.  Looks like none of the boolean ones can
be enabled on m68k, which is where I run most of the tests, so I never
noticed before :-(

> It makes sense that if you choose to build all tests, you would not skip
> the ones that cannot be built as a module.

You can still enable the test manually if you want.
But I think it should not be enabled by default when all other tests
that can be modular are built as modules.  Unlike for modular tests,
enabling builtin tests by default does impact the base kernel.

> > Perhaps
> >
> >     default KUNIT_ALL_TESTS=y
> >
> > ?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds