diff mbox series

[v2,1/2] printf: convert self-test to KUnit

Message ID 20250207-printf-kunit-convert-v2-1-057b23860823@gmail.com (mailing list archive)
State New
Headers show
Series printf: convert self-test to KUnit | expand

Commit Message

Tamir Duberstein Feb. 7, 2025, 11:30 a.m. UTC 
Convert the printf() self-test to a KUnit test.

In the interest of keeping the patch reasonably-sized this doesn't
refactor the tests into proper parameterized tests - it's all one big
test case.

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> # m68k
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 Documentation/core-api/printk-formats.rst |   2 +-
 MAINTAINERS                               |   2 +-
 arch/m68k/configs/amiga_defconfig         |   1 -
 arch/m68k/configs/apollo_defconfig        |   1 -
 arch/m68k/configs/atari_defconfig         |   1 -
 arch/m68k/configs/bvme6000_defconfig      |   1 -
 arch/m68k/configs/hp300_defconfig         |   1 -
 arch/m68k/configs/mac_defconfig           |   1 -
 arch/m68k/configs/multi_defconfig         |   1 -
 arch/m68k/configs/mvme147_defconfig       |   1 -
 arch/m68k/configs/mvme16x_defconfig       |   1 -
 arch/m68k/configs/q40_defconfig           |   1 -
 arch/m68k/configs/sun3_defconfig          |   1 -
 arch/m68k/configs/sun3x_defconfig         |   1 -
 arch/powerpc/configs/ppc64_defconfig      |   1 -
 lib/Kconfig.debug                         |  20 +++-
 lib/Makefile                              |   2 +-
 lib/{test_printf.c => printf_kunit.c}     | 164 +++++++++++++++---------------
 tools/testing/selftests/lib/config        |   1 -
 tools/testing/selftests/lib/printf.sh     |   4 -
 20 files changed, 104 insertions(+), 104 deletions(-)

Comments

Tamir Duberstein Feb. 7, 2025, 2:37 p.m. UTC | #1 
On Fri, Feb 7, 2025 at 6:30 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Convert the printf() self-test to a KUnit test.
>
> In the interest of keeping the patch reasonably-sized this doesn't
> refactor the tests into proper parameterized tests - it's all one big
> test case.

Apologies for the version churn, I'll have to send one more which
removes spurious trailing newlines (not necessary in kunit) and
converts logging macros like pr_info to kunit_info.

I'll wait a few days in case folks have other comments.
Rasmus Villemoes Feb. 10, 2025, 1 p.m. UTC | #2 
On Fri, Feb 07 2025, Tamir Duberstein <tamird@gmail.com> wrote:

> Convert the printf() self-test to a KUnit test.
>
> In the interest of keeping the patch reasonably-sized this doesn't
> refactor the tests into proper parameterized tests - it's all one big
> test case.
>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> # m68k
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  Documentation/core-api/printk-formats.rst |   2 +-
>  MAINTAINERS                               |   2 +-
>  arch/m68k/configs/amiga_defconfig         |   1 -
>  arch/m68k/configs/apollo_defconfig        |   1 -
>  arch/m68k/configs/atari_defconfig         |   1 -
>  arch/m68k/configs/bvme6000_defconfig      |   1 -
>  arch/m68k/configs/hp300_defconfig         |   1 -
>  arch/m68k/configs/mac_defconfig           |   1 -
>  arch/m68k/configs/multi_defconfig         |   1 -
>  arch/m68k/configs/mvme147_defconfig       |   1 -
>  arch/m68k/configs/mvme16x_defconfig       |   1 -
>  arch/m68k/configs/q40_defconfig           |   1 -
>  arch/m68k/configs/sun3_defconfig          |   1 -
>  arch/m68k/configs/sun3x_defconfig         |   1 -
>  arch/powerpc/configs/ppc64_defconfig      |   1 -
>  lib/Kconfig.debug                         |  20 +++-
>  lib/Makefile                              |   2 +-
>  lib/{test_printf.c => printf_kunit.c}     | 164 +++++++++++++++---------------
>  tools/testing/selftests/lib/config        |   1 -
>  tools/testing/selftests/lib/printf.sh     |   4 -
>  20 files changed, 104 insertions(+), 104 deletions(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index ecccc0473da9..0d9461bd6964 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -661,7 +661,7 @@ Do *not* use it from C.
>  Thanks
>  ======
>  
> -If you add other %p extensions, please extend <lib/test_printf.c> with
> +If you add other %p extensions, please extend <lib/printf_kunit.c> with
>  one or more test cases, if at all feasible.
>  
>  Thank you for your cooperation and attention.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 896a307fa065..57e366d356bd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25368,7 +25368,7 @@ R:	Sergey Senozhatsky <senozhatsky@chromium.org>
>  S:	Maintained
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git
>  F:	Documentation/core-api/printk-formats.rst
> -F:	lib/test_printf.c
> +F:	lib/printf_kunit.c
>  F:	lib/test_scanf.c
>  F:	lib/vsprintf.c
>  
> diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig
> index dbf2ea561c85..e8c5e08fb6ec 100644
> --- a/arch/m68k/configs/amiga_defconfig
> +++ b/arch/m68k/configs/amiga_defconfig
> @@ -622,7 +622,6 @@ CONFIG_ATOMIC64_SELFTEST=m
>  CONFIG_ASYNC_RAID6_TEST=m
>  CONFIG_TEST_HEXDUMP=m
>  CONFIG_TEST_KSTRTOX=m
> -CONFIG_TEST_PRINTF=m
>  CONFIG_TEST_SCANF=m
>  CONFIG_TEST_BITMAP=m
>  CONFIG_TEST_UUID=m

If/when you do re-roll a v3, can you split the defconfig changes off to
a separate patch? It's a little annoying to scroll through all those
mechanical one-liner diffs to get to the actual changes.

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 1af972a92d06..4834cac1eb8f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST
>  config TEST_HEXDUMP
>  	tristate "Test functions located in the hexdump module at runtime"
>  
> +config PRINTF_KUNIT_TEST
> +	tristate "KUnit test printf() family of functions at runtime" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  Enable this option to test the printf functions at boot.
> +

Well, not necessarily at boot unless built-in? 

> +	  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.
> +

I see that this is copy-pasted from other kunit config items, but not
all of them have all this boilerplate, and I don't think it's useful
(and, the mentions of "at boot" and "during boot" are actively
misleading). Other kunit config items are shorter and more to the point,
e.g.

          Enable this to turn on 'list_sort()' function test. This test is
          executed only once during system boot (so affects only boot time),
          or at module load time.

          If unsure, say N.

>  
>  obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
> diff --git a/lib/test_printf.c b/lib/printf_kunit.c
> similarity index 89%
> rename from lib/test_printf.c
> rename to lib/printf_kunit.c
> index 59dbe4f9a4cb..fe6f4df92dd5 100644
> --- a/lib/test_printf.c
> +++ b/lib/printf_kunit.c
> @@ -5,7 +5,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/init.h>
> +#include <kunit/test.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/printk.h>
> @@ -25,8 +25,6 @@
>  
>  #include <linux/property.h>
>  
> -#include "../tools/testing/selftests/kselftest_module.h"
> -
>  #define BUF_SIZE 256
>  #define PAD_SIZE 16
>  #define FILL_CHAR '$'
> @@ -37,72 +35,71 @@
>  	block \
>  	__diag_pop();
>  
> -KSTM_MODULE_GLOBALS();
> +static char *test_buffer;
> +static char *alloced_buffer;
> +
> +static struct kunit *kunittest;
>  
> -static char *test_buffer __initdata;
> -static char *alloced_buffer __initdata;
> +#define tc_fail(fmt, ...) \
> +	KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
>  
> -static int __printf(4, 0) __init
> +static void __printf(4, 0)
>  do_test(int bufsize, const char *expect, int elen,
>  	const char *fmt, va_list ap)
>  {
>  	va_list aq;
>  	int ret, written;
>  
> -	total_tests++;
> -
>  	memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE);
>  	va_copy(aq, ap);
>  	ret = vsnprintf(test_buffer, bufsize, fmt, aq);
>  	va_end(aq);
>  
>  	if (ret != elen) {
> -		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
> +		tc_fail("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
>  			bufsize, fmt, ret, elen);
> -		return 1;
> +		return;
>  	}
>  
>  	if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
> -		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
> -		return 1;
> +		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
> +		return;
>  	}
>  
>  	if (!bufsize) {
>  		if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
> -			pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
> +			tc_fail("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
>  				fmt);
> -			return 1;
>  		}
> -		return 0;
> +		return;
>  	}
>  
>  	written = min(bufsize-1, elen);
>  	if (test_buffer[written]) {
> -		pr_warn("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
> +		tc_fail("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
>  			bufsize, fmt);
> -		return 1;
> +		return;
>  	}
>  
>  	if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
> -		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
> +		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
>  			bufsize, fmt);
> -		return 1;
> +		return;
>  	}
>  
>  	if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) {
> -		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);
> -		return 1;
> +		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);
> +		return;
>  	}
>  
>  	if (memcmp(test_buffer, expect, written)) {
> -		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
> +		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
>  			bufsize, fmt, test_buffer, written, expect);
> -		return 1;
> +		return;
>  	}
> -	return 0;
>  }
>  
> -static void __printf(3, 4) __init
> +static void __printf(3, 4)
>  __test(const char *expect, int elen, const char *fmt, ...)
>  {
>  	va_list ap;
> @@ -110,9 +107,8 @@ __test(const char *expect, int elen, const char *fmt, ...)
>  	char *p;
>  
>  	if (elen >= BUF_SIZE) {
> -		pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",
> -		       elen, fmt);
> -		failed_tests++;
> +		tc_fail("error in test suite: expected output length %d too long. Format was '%s'.\n",
> +			elen, fmt);
>  		return;
>  	}
>  
> @@ -124,19 +120,17 @@ __test(const char *expect, int elen, const char *fmt, ...)
>  	 * enough and 0), and then we also test that kvasprintf would
>  	 * be able to print it as expected.
>  	 */
> -	failed_tests += do_test(BUF_SIZE, expect, elen, fmt, ap);
> +	do_test(BUF_SIZE, expect, elen, fmt, ap);
>  	rand = get_random_u32_inclusive(1, elen + 1);
>  	/* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */
> -	failed_tests += do_test(rand, expect, elen, fmt, ap);
> -	failed_tests += do_test(0, expect, elen, fmt, ap);
> +	do_test(rand, expect, elen, fmt, ap);
> +	do_test(0, expect, elen, fmt, ap);
>  
>  	p = kvasprintf(GFP_KERNEL, fmt, ap);
>  	if (p) {
> -		total_tests++;
>  		if (memcmp(p, expect, elen+1)) {
> -			pr_warn("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
> +			tc_fail("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
>  				fmt, p, expect);
> -			failed_tests++;
>  		}
>  		kfree(p);
>  	}

So reading through this, is there really any reason to drop those
existing total_tests and failed_tests tallies? Since you do need to keep
the control flow the same, leaving the return types and "return
1"/"return 0" and their uses in updating failed_tests would also reduce
the patch size quite significantly.

So they no longer come from some KSTM_MODULE_GLOBALS(), and thus need to
be explicitly declared in this module, but that's exactly how it was
originally until someone decided to lift that from the the printf suite
to kstm.

Yes, they'd need to be explicitly initialized to 0 during kunit_init
since they are no longer use-once __initdata variable, and some
kunit_exit logic would need to "manually" report them.

Rasmus
Tamir Duberstein Feb. 10, 2025, 2:36 p.m. UTC | #3 
On Mon, Feb 10, 2025 at 8:01 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On Fri, Feb 07 2025, Tamir Duberstein <tamird@gmail.com> wrote:
>
> [...]
>
> If/when you do re-roll a v3, can you split the defconfig changes off to
> a separate patch? It's a little annoying to scroll through all those
> mechanical one-liner diffs to get to the actual changes.

Yep. I'll split them into one for m68k and another for powerpc. Geert,
I'll move your Acked-by to the m68k patch.

> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 1af972a92d06..4834cac1eb8f 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2427,6 +2427,23 @@ config ASYNC_RAID6_TEST
> >  config TEST_HEXDUMP
> >       tristate "Test functions located in the hexdump module at runtime"
> >
> > +config PRINTF_KUNIT_TEST
> > +     tristate "KUnit test printf() family of functions at runtime" if !KUNIT_ALL_TESTS
> > +     depends on KUNIT
> > +     default KUNIT_ALL_TESTS
> > +     help
> > +       Enable this option to test the printf functions at boot.
> > +
>
> Well, not necessarily at boot unless built-in?
>
> > +       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.
> > +
>
> I see that this is copy-pasted from other kunit config items, but not
> all of them have all this boilerplate, and I don't think it's useful
> (and, the mentions of "at boot" and "during boot" are actively
> misleading). Other kunit config items are shorter and more to the point,
> e.g.
>
>           Enable this to turn on 'list_sort()' function test. This test is
>           executed only once during system boot (so affects only boot time),
>           or at module load time.
>
>           If unsure, say N.
>

Very good point. Truthfully this boilerplate is a result of
checkpatch.pl nagging about this paragraph being a certain length.
I'll drop it and just write "Enable this option to test the printf
functions at runtime.".

> >
> >  obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
> > diff --git a/lib/test_printf.c b/lib/printf_kunit.c
> > similarity index 89%
> > rename from lib/test_printf.c
> > rename to lib/printf_kunit.c
> > index 59dbe4f9a4cb..fe6f4df92dd5 100644
> > --- a/lib/test_printf.c
> > +++ b/lib/printf_kunit.c
> > @@ -5,7 +5,7 @@
> >
> >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >
> > -#include <linux/init.h>
> > +#include <kunit/test.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/printk.h>
> > @@ -25,8 +25,6 @@
> >
> >  #include <linux/property.h>
> >
> > -#include "../tools/testing/selftests/kselftest_module.h"
> > -
> >  #define BUF_SIZE 256
> >  #define PAD_SIZE 16
> >  #define FILL_CHAR '$'
> > @@ -37,72 +35,71 @@
> >       block \
> >       __diag_pop();
> >
> > -KSTM_MODULE_GLOBALS();
> > +static char *test_buffer;
> > +static char *alloced_buffer;
> > +
> > +static struct kunit *kunittest;
> >
> > -static char *test_buffer __initdata;
> > -static char *alloced_buffer __initdata;
> > +#define tc_fail(fmt, ...) \
> > +     KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
> >
> > -static int __printf(4, 0) __init
> > +static void __printf(4, 0)
> >  do_test(int bufsize, const char *expect, int elen,
> >       const char *fmt, va_list ap)
> >  {
> >       va_list aq;
> >       int ret, written;
> >
> > -     total_tests++;
> > -
> >       memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE);
> >       va_copy(aq, ap);
> >       ret = vsnprintf(test_buffer, bufsize, fmt, aq);
> >       va_end(aq);
> >
> >       if (ret != elen) {
> > -             pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
> > +             tc_fail("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
> >                       bufsize, fmt, ret, elen);
> > -             return 1;
> > +             return;
> >       }
> >
> >       if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
> > -             pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
> > -             return 1;
> > +             tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
> > +             return;
> >       }
> >
> >       if (!bufsize) {
> >               if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
> > -                     pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
> > +                     tc_fail("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
> >                               fmt);
> > -                     return 1;
> >               }
> > -             return 0;
> > +             return;
> >       }
> >
> >       written = min(bufsize-1, elen);
> >       if (test_buffer[written]) {
> > -             pr_warn("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
> > +             tc_fail("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
> >                       bufsize, fmt);
> > -             return 1;
> > +             return;
> >       }
> >
> >       if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
> > -             pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
> > +             tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
> >                       bufsize, fmt);
> > -             return 1;
> > +             return;
> >       }
> >
> >       if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) {
> > -             pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);
> > -             return 1;
> > +             tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);
> > +             return;
> >       }
> >
> >       if (memcmp(test_buffer, expect, written)) {
> > -             pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
> > +             tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
> >                       bufsize, fmt, test_buffer, written, expect);
> > -             return 1;
> > +             return;
> >       }
> > -     return 0;
> >  }
> >
> > -static void __printf(3, 4) __init
> > +static void __printf(3, 4)
> >  __test(const char *expect, int elen, const char *fmt, ...)
> >  {
> >       va_list ap;
> > @@ -110,9 +107,8 @@ __test(const char *expect, int elen, const char *fmt, ...)
> >       char *p;
> >
> >       if (elen >= BUF_SIZE) {
> > -             pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",
> > -                    elen, fmt);
> > -             failed_tests++;
> > +             tc_fail("error in test suite: expected output length %d too long. Format was '%s'.\n",
> > +                     elen, fmt);
> >               return;
> >       }
> >
> > @@ -124,19 +120,17 @@ __test(const char *expect, int elen, const char *fmt, ...)
> >        * enough and 0), and then we also test that kvasprintf would
> >        * be able to print it as expected.
> >        */
> > -     failed_tests += do_test(BUF_SIZE, expect, elen, fmt, ap);
> > +     do_test(BUF_SIZE, expect, elen, fmt, ap);
> >       rand = get_random_u32_inclusive(1, elen + 1);
> >       /* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */
> > -     failed_tests += do_test(rand, expect, elen, fmt, ap);
> > -     failed_tests += do_test(0, expect, elen, fmt, ap);
> > +     do_test(rand, expect, elen, fmt, ap);
> > +     do_test(0, expect, elen, fmt, ap);
> >
> >       p = kvasprintf(GFP_KERNEL, fmt, ap);
> >       if (p) {
> > -             total_tests++;
> >               if (memcmp(p, expect, elen+1)) {
> > -                     pr_warn("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
> > +                     tc_fail("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
> >                               fmt, p, expect);
> > -                     failed_tests++;
> >               }
> >               kfree(p);
> >       }
>
> So reading through this, is there really any reason to drop those
> existing total_tests and failed_tests tallies? Since you do need to keep
> the control flow the same, leaving the return types and "return
> 1"/"return 0" and their uses in updating failed_tests would also reduce
> the patch size quite significantly.
>
> So they no longer come from some KSTM_MODULE_GLOBALS(), and thus need to
> be explicitly declared in this module, but that's exactly how it was
> originally until someone decided to lift that from the the printf suite
> to kstm.
>
> Yes, they'd need to be explicitly initialized to 0 during kunit_init
> since they are no longer use-once __initdata variable, and some
> kunit_exit logic would need to "manually" report them.

If I understand your concern correctly, you only really care about
`total_tests`, right? I think it's slightly unidiomatic to count tests
outside the framework this way but it's not a hill I'll die on.

Just to elaborate on why I think this unidiomatic: the KUnit test
runner script that produces human-readable output doesn't emit logs
unless something fails. In the success case, you'd get (before the
next patch that breaks this into many tests):

[09:34:15] ==================== printf (1 subtest)
====================
[09:34:15] [PASSED] printf_test
[09:34:15] ===================== [PASSED] printf
======================
[09:34:15] ============================================================
[09:34:15] Testing complete. Ran 1 tests: passed: 1

but the raw output does contain the tally:

KTAP version 1
1..1
    KTAP version 1
    # Subtest: printf
    # module: printf_kunit
    1..1
    ok 1 printf_test
    # printf: ran 448 tests
ok 1 printf

So assuming you're OK with this compromise, I'll go ahead and spin v3.

Thanks for the review!
Geert Uytterhoeven Feb. 10, 2025, 2:52 p.m. UTC | #4 
Hi Tamir,

On Mon, 10 Feb 2025 at 15:37, Tamir Duberstein <tamird@gmail.com> wrote:
> On Mon, Feb 10, 2025 at 8:01 AM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
> >
> > On Fri, Feb 07 2025, Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > [...]
> >
> > If/when you do re-roll a v3, can you split the defconfig changes off to
> > a separate patch? It's a little annoying to scroll through all those
> > mechanical one-liner diffs to get to the actual changes.
>
> Yep. I'll split them into one for m68k and another for powerpc. Geert,
> I'll move your Acked-by to the m68k patch.

Fine for me!

Alternatively, you could just drop them. I do refresh the m68k
defconfigs after every kernel release, but these updates only go
upstream one cycle later.  In this case that doesn't matter at all.

Gr{oetje,eeting}s,

                        Geert
Tamir Duberstein Feb. 10, 2025, 3:22 p.m. UTC | #5 
On Mon, Feb 10, 2025 at 9:52 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Tamir,
>
> On Mon, 10 Feb 2025 at 15:37, Tamir Duberstein <tamird@gmail.com> wrote:
> > On Mon, Feb 10, 2025 at 8:01 AM Rasmus Villemoes
> > <linux@rasmusvillemoes.dk> wrote:
> > >
> > > On Fri, Feb 07 2025, Tamir Duberstein <tamird@gmail.com> wrote:
> > >
> > > [...]
> > >
> > > If/when you do re-roll a v3, can you split the defconfig changes off to
> > > a separate patch? It's a little annoying to scroll through all those
> > > mechanical one-liner diffs to get to the actual changes.
> >
> > Yep. I'll split them into one for m68k and another for powerpc. Geert,
> > I'll move your Acked-by to the m68k patch.
>
> Fine for me!
>
> Alternatively, you could just drop them. I do refresh the m68k
> defconfigs after every kernel release, but these updates only go
> upstream one cycle later.  In this case that doesn't matter at all.

Will do, and in scanf and bitmap as well.

Cheers.
Tamir
diff mbox series

Patch

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index ecccc0473da9..0d9461bd6964 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -661,7 +661,7 @@  Do *not* use it from C.
 Thanks
 ======
 
-If you add other %p extensions, please extend <lib/test_printf.c> with
+If you add other %p extensions, please extend <lib/printf_kunit.c> with
 one or more test cases, if at all feasible.
 
 Thank you for your cooperation and attention.
diff --git a/MAINTAINERS b/MAINTAINERS
index 896a307fa065..57e366d356bd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25368,7 +25368,7 @@  R:	Sergey Senozhatsky <senozhatsky@chromium.org>
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git
 F:	Documentation/core-api/printk-formats.rst
-F:	lib/test_printf.c
+F:	lib/printf_kunit.c
 F:	lib/test_scanf.c
 F:	lib/vsprintf.c
 
diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig
index dbf2ea561c85..e8c5e08fb6ec 100644
--- a/arch/m68k/configs/amiga_defconfig
+++ b/arch/m68k/configs/amiga_defconfig
@@ -622,7 +622,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/apollo_defconfig b/arch/m68k/configs/apollo_defconfig
index b0fd199cc0a4..dcaddfc675de 100644
--- a/arch/m68k/configs/apollo_defconfig
+++ b/arch/m68k/configs/apollo_defconfig
@@ -579,7 +579,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/atari_defconfig b/arch/m68k/configs/atari_defconfig
index bb5b2d3b6c10..53a91c62d415 100644
--- a/arch/m68k/configs/atari_defconfig
+++ b/arch/m68k/configs/atari_defconfig
@@ -599,7 +599,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/bvme6000_defconfig b/arch/m68k/configs/bvme6000_defconfig
index 8315a13bab73..ea2f84907c4e 100644
--- a/arch/m68k/configs/bvme6000_defconfig
+++ b/arch/m68k/configs/bvme6000_defconfig
@@ -571,7 +571,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/hp300_defconfig b/arch/m68k/configs/hp300_defconfig
index 350370657e5f..7b1cc9f597bf 100644
--- a/arch/m68k/configs/hp300_defconfig
+++ b/arch/m68k/configs/hp300_defconfig
@@ -581,7 +581,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/mac_defconfig b/arch/m68k/configs/mac_defconfig
index f942b4755702..831695e766a8 100644
--- a/arch/m68k/configs/mac_defconfig
+++ b/arch/m68k/configs/mac_defconfig
@@ -598,7 +598,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig
index b1eaad02efab..901d8d357218 100644
--- a/arch/m68k/configs/multi_defconfig
+++ b/arch/m68k/configs/multi_defconfig
@@ -685,7 +685,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/mvme147_defconfig b/arch/m68k/configs/mvme147_defconfig
index 6309a4442bb3..2212bb7877e2 100644
--- a/arch/m68k/configs/mvme147_defconfig
+++ b/arch/m68k/configs/mvme147_defconfig
@@ -571,7 +571,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/mvme16x_defconfig b/arch/m68k/configs/mvme16x_defconfig
index 3feb0731f814..a87b455049e8 100644
--- a/arch/m68k/configs/mvme16x_defconfig
+++ b/arch/m68k/configs/mvme16x_defconfig
@@ -572,7 +572,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/q40_defconfig b/arch/m68k/configs/q40_defconfig
index ea04b1b0da7d..411e555c7e07 100644
--- a/arch/m68k/configs/q40_defconfig
+++ b/arch/m68k/configs/q40_defconfig
@@ -588,7 +588,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/sun3_defconfig b/arch/m68k/configs/sun3_defconfig
index f52d9af92153..e095fa159d05 100644
--- a/arch/m68k/configs/sun3_defconfig
+++ b/arch/m68k/configs/sun3_defconfig
@@ -568,7 +568,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/m68k/configs/sun3x_defconfig b/arch/m68k/configs/sun3x_defconfig
index f348447824da..11eb37864ac6 100644
--- a/arch/m68k/configs/sun3x_defconfig
+++ b/arch/m68k/configs/sun3x_defconfig
@@ -569,7 +569,6 @@  CONFIG_ATOMIC64_SELFTEST=m
 CONFIG_ASYNC_RAID6_TEST=m
 CONFIG_TEST_HEXDUMP=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 465eb96c755e..05018898d282 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -444,7 +444,6 @@  CONFIG_TEST_HEXDUMP=m
 CONFIG_STRING_SELFTEST=m
 CONFIG_TEST_STRING_HELPERS=m
 CONFIG_TEST_KSTRTOX=m
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_TEST_UUID=m
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af972a92d06..4834cac1eb8f 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2427,6 +2427,23 @@  config ASYNC_RAID6_TEST
 config TEST_HEXDUMP
 	tristate "Test functions located in the hexdump module at runtime"
 
+config PRINTF_KUNIT_TEST
+	tristate "KUnit test printf() family of functions at runtime" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Enable this option to test the printf functions at boot.
+
+	  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 STRING_KUNIT_TEST
 	tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
 	depends on KUNIT
@@ -2440,9 +2457,6 @@  config STRING_HELPERS_KUNIT_TEST
 config TEST_KSTRTOX
 	tristate "Test kstrto*() family of functions at runtime"
 
-config TEST_PRINTF
-	tristate "Test printf() family of functions at runtime"
-
 config TEST_SCANF
 	tristate "Test scanf() family of functions at runtime"
 
diff --git a/lib/Makefile b/lib/Makefile
index d5cfc7afbbb8..844665b1f0e7 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -84,7 +84,7 @@  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
-obj-$(CONFIG_TEST_PRINTF) += test_printf.o
+obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o
 obj-$(CONFIG_TEST_SCANF) += test_scanf.o
 
 obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
diff --git a/lib/test_printf.c b/lib/printf_kunit.c
similarity index 89%
rename from lib/test_printf.c
rename to lib/printf_kunit.c
index 59dbe4f9a4cb..fe6f4df92dd5 100644
--- a/lib/test_printf.c
+++ b/lib/printf_kunit.c
@@ -5,7 +5,7 @@ 
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/init.h>
+#include <kunit/test.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/printk.h>
@@ -25,8 +25,6 @@ 
 
 #include <linux/property.h>
 
-#include "../tools/testing/selftests/kselftest_module.h"
-
 #define BUF_SIZE 256
 #define PAD_SIZE 16
 #define FILL_CHAR '$'
@@ -37,72 +35,71 @@ 
 	block \
 	__diag_pop();
 
-KSTM_MODULE_GLOBALS();
+static char *test_buffer;
+static char *alloced_buffer;
+
+static struct kunit *kunittest;
 
-static char *test_buffer __initdata;
-static char *alloced_buffer __initdata;
+#define tc_fail(fmt, ...) \
+	KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__)
 
-static int __printf(4, 0) __init
+static void __printf(4, 0)
 do_test(int bufsize, const char *expect, int elen,
 	const char *fmt, va_list ap)
 {
 	va_list aq;
 	int ret, written;
 
-	total_tests++;
-
 	memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE);
 	va_copy(aq, ap);
 	ret = vsnprintf(test_buffer, bufsize, fmt, aq);
 	va_end(aq);
 
 	if (ret != elen) {
-		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
+		tc_fail("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
 			bufsize, fmt, ret, elen);
-		return 1;
+		return;
 	}
 
 	if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
-		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
-		return 1;
+		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
+		return;
 	}
 
 	if (!bufsize) {
 		if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
-			pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
+			tc_fail("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
 				fmt);
-			return 1;
 		}
-		return 0;
+		return;
 	}
 
 	written = min(bufsize-1, elen);
 	if (test_buffer[written]) {
-		pr_warn("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
+		tc_fail("vsnprintf(buf, %d, \"%s\", ...) did not nul-terminate buffer\n",
 			bufsize, fmt);
-		return 1;
+		return;
 	}
 
 	if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
-		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
+		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
 			bufsize, fmt);
-		return 1;
+		return;
 	}
 
 	if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) {
-		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);
-		return 1;
+		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);
+		return;
 	}
 
 	if (memcmp(test_buffer, expect, written)) {
-		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
+		tc_fail("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
 			bufsize, fmt, test_buffer, written, expect);
-		return 1;
+		return;
 	}
-	return 0;
 }
 
-static void __printf(3, 4) __init
+static void __printf(3, 4)
 __test(const char *expect, int elen, const char *fmt, ...)
 {
 	va_list ap;
@@ -110,9 +107,8 @@  __test(const char *expect, int elen, const char *fmt, ...)
 	char *p;
 
 	if (elen >= BUF_SIZE) {
-		pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",
-		       elen, fmt);
-		failed_tests++;
+		tc_fail("error in test suite: expected output length %d too long. Format was '%s'.\n",
+			elen, fmt);
 		return;
 	}
 
@@ -124,19 +120,17 @@  __test(const char *expect, int elen, const char *fmt, ...)
 	 * enough and 0), and then we also test that kvasprintf would
 	 * be able to print it as expected.
 	 */
-	failed_tests += do_test(BUF_SIZE, expect, elen, fmt, ap);
+	do_test(BUF_SIZE, expect, elen, fmt, ap);
 	rand = get_random_u32_inclusive(1, elen + 1);
 	/* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */
-	failed_tests += do_test(rand, expect, elen, fmt, ap);
-	failed_tests += do_test(0, expect, elen, fmt, ap);
+	do_test(rand, expect, elen, fmt, ap);
+	do_test(0, expect, elen, fmt, ap);
 
 	p = kvasprintf(GFP_KERNEL, fmt, ap);
 	if (p) {
-		total_tests++;
 		if (memcmp(p, expect, elen+1)) {
-			pr_warn("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
+			tc_fail("kvasprintf(..., \"%s\", ...) returned '%s', expected '%s'\n",
 				fmt, p, expect);
-			failed_tests++;
 		}
 		kfree(p);
 	}
@@ -146,7 +140,7 @@  __test(const char *expect, int elen, const char *fmt, ...)
 #define test(expect, fmt, ...)					\
 	__test(expect, strlen(expect), fmt, ##__VA_ARGS__)
 
-static void __init
+static void
 test_basic(void)
 {
 	/* Work around annoying "warning: zero-length gnu_printf format string". */
@@ -158,7 +152,7 @@  test_basic(void)
 	__test("xxx\0yyy", 7, "xxx%cyyy", '\0');
 }
 
-static void __init
+static void
 test_number(void)
 {
 	test("0x1234abcd  ", "%#-12x", 0x1234abcd);
@@ -180,7 +174,7 @@  test_number(void)
 	test("00|0|0|0|0", "%.2d|%.1d|%.0d|%.*d|%1.0d", 0, 0, 0, 0, 0, 0);
 }
 
-static void __init
+static void
 test_string(void)
 {
 	test("", "%s%.0s", "", "123");
@@ -218,7 +212,7 @@  test_string(void)
 #define ZEROS "00000000"	/* hex 32 zero bits */
 #define ONES "ffffffff"		/* hex 32 one bits */
 
-static int __init
+static int
 plain_format(void)
 {
 	char buf[PLAIN_BUF_SIZE];
@@ -250,7 +244,7 @@  plain_format(void)
 #define ZEROS ""
 #define ONES ""
 
-static int __init
+static int
 plain_format(void)
 {
 	/* Format is implicitly tested for 32 bit machines by plain_hash() */
@@ -259,7 +253,7 @@  plain_format(void)
 
 #endif	/* BITS_PER_LONG == 64 */
 
-static int __init
+static int
 plain_hash_to_buffer(const void *p, char *buf, size_t len)
 {
 	int nchars;
@@ -278,7 +272,7 @@  plain_hash_to_buffer(const void *p, char *buf, size_t len)
 	return 0;
 }
 
-static int __init
+static int
 plain_hash(void)
 {
 	char buf[PLAIN_BUF_SIZE];
@@ -298,32 +292,29 @@  plain_hash(void)
  * We can't use test() to test %p because we don't know what output to expect
  * after an address is hashed.
  */
-static void __init
+static void
 plain(void)
 {
 	int err;
 
 	if (no_hash_pointers) {
 		pr_warn("skipping plain 'p' tests");
-		skipped_tests += 2;
 		return;
 	}
 
 	err = plain_hash();
 	if (err) {
-		pr_warn("plain 'p' does not appear to be hashed\n");
-		failed_tests++;
+		tc_fail("plain 'p' does not appear to be hashed\n");
 		return;
 	}
 
 	err = plain_format();
 	if (err) {
-		pr_warn("hashing plain 'p' has unexpected format\n");
-		failed_tests++;
+		tc_fail("hashing plain 'p' has unexpected format\n");
 	}
 }
 
-static void __init
+static void
 test_hashed(const char *fmt, const void *p)
 {
 	char buf[PLAIN_BUF_SIZE];
@@ -343,7 +334,7 @@  test_hashed(const char *fmt, const void *p)
 /*
  * NULL pointers aren't hashed.
  */
-static void __init
+static void
 null_pointer(void)
 {
 	test(ZEROS "00000000", "%p", NULL);
@@ -354,7 +345,7 @@  null_pointer(void)
 /*
  * Error pointers aren't hashed.
  */
-static void __init
+static void
 error_pointer(void)
 {
 	test(ONES "fffffff5", "%p", ERR_PTR(-11));
@@ -364,7 +355,7 @@  error_pointer(void)
 
 #define PTR_INVALID ((void *)0x000000ab)
 
-static void __init
+static void
 invalid_pointer(void)
 {
 	test_hashed("%p", PTR_INVALID);
@@ -372,18 +363,18 @@  invalid_pointer(void)
 	test("(efault)", "%pE", PTR_INVALID);
 }
 
-static void __init
+static void
 symbol_ptr(void)
 {
 }
 
-static void __init
+static void
 kernel_ptr(void)
 {
 	/* We can't test this without access to kptr_restrict. */
 }
 
-static void __init
+static void
 struct_resource(void)
 {
 	struct resource test_resource = {
@@ -432,7 +423,7 @@  struct_resource(void)
 	     "%pR", &test_resource);
 }
 
-static void __init
+static void
 struct_range(void)
 {
 	struct range test_range = DEFINE_RANGE(0xc0ffee00ba5eba11,
@@ -448,17 +439,17 @@  struct_range(void)
 	     "%pra", &test_range);
 }
 
-static void __init
+static void
 addr(void)
 {
 }
 
-static void __init
+static void
 escaped_str(void)
 {
 }
 
-static void __init
+static void
 hex_string(void)
 {
 	const char buf[3] = {0xc0, 0xff, 0xee};
@@ -469,7 +460,7 @@  hex_string(void)
 	     "%*ph|%*phC|%*phD|%*phN", 3, buf, 3, buf, 3, buf, 3, buf);
 }
 
-static void __init
+static void
 mac(void)
 {
 	const u8 addr[6] = {0x2d, 0x48, 0xd6, 0xfc, 0x7a, 0x05};
@@ -481,7 +472,7 @@  mac(void)
 	test("057afcd6482d", "%pmR", addr);
 }
 
-static void __init
+static void
 ip4(void)
 {
 	struct sockaddr_in sa;
@@ -496,19 +487,19 @@  ip4(void)
 	test("001.002.003.004:12345|1.2.3.4:12345", "%piSp|%pISp", &sa, &sa);
 }
 
-static void __init
+static void
 ip6(void)
 {
 }
 
-static void __init
+static void
 ip(void)
 {
 	ip4();
 	ip6();
 }
 
-static void __init
+static void
 uuid(void)
 {
 	const char uuid[16] = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
@@ -520,7 +511,7 @@  uuid(void)
 	test("03020100-0504-0706-0809-0A0B0C0D0E0F", "%pUL", uuid);
 }
 
-static struct dentry test_dentry[4] __initdata = {
+static struct dentry test_dentry[4] = {
 	{ .d_parent = &test_dentry[0],
 	  .d_name = QSTR_INIT(test_dentry[0].d_iname, 3),
 	  .d_iname = "foo" },
@@ -535,7 +526,7 @@  static struct dentry test_dentry[4] __initdata = {
 	  .d_iname = "romeo" },
 };
 
-static void __init
+static void
 dentry(void)
 {
 	test("foo", "%pd", &test_dentry[0]);
@@ -556,12 +547,12 @@  dentry(void)
 	test("  bravo/alfa|  bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]);
 }
 
-static void __init
+static void
 struct_va_format(void)
 {
 }
 
-static void __init
+static void
 time_and_date(void)
 {
 	/* 1543210543 */
@@ -595,12 +586,12 @@  time_and_date(void)
 	test("15:32:23|0119-00-04", "%ptTtrs|%ptTdrs", &t, &t);
 }
 
-static void __init
+static void
 struct_clk(void)
 {
 }
 
-static void __init
+static void
 large_bitmap(void)
 {
 	const int nbits = 1 << 16;
@@ -614,7 +605,7 @@  large_bitmap(void)
 	bitmap_free(bits);
 }
 
-static void __init
+static void
 bitmap(void)
 {
 	DECLARE_BITMAP(bits, 20);
@@ -637,7 +628,7 @@  bitmap(void)
 	large_bitmap();
 }
 
-static void __init
+static void
 netdev_features(void)
 {
 }
@@ -663,7 +654,7 @@  static const struct page_flags_test pft[] = {
 	 "%#x", "kasantag"},
 };
 
-static void __init
+static void
 page_flags_test(int section, int node, int zone, int last_cpupid,
 		int kasan_tag, unsigned long flags, const char *name,
 		char *cmp_buf)
@@ -701,7 +692,7 @@  page_flags_test(int section, int node, int zone, int last_cpupid,
 	test(cmp_buf, "%pGp", &flags);
 }
 
-static void __init
+static void
 flags(void)
 {
 	unsigned long flags;
@@ -749,7 +740,7 @@  flags(void)
 	kfree(cmp_buffer);
 }
 
-static void __init fwnode_pointer(void)
+static void fwnode_pointer(void)
 {
 	const struct software_node first = { .name = "first" };
 	const struct software_node second = { .name = "second", .parent = &first };
@@ -776,7 +767,7 @@  static void __init fwnode_pointer(void)
 	software_node_unregister_node_group(group);
 }
 
-static void __init fourcc_pointer(void)
+static void fourcc_pointer(void)
 {
 	struct {
 		u32 code;
@@ -793,7 +784,7 @@  static void __init fourcc_pointer(void)
 		test(try[i].str, "%p4cc", &try[i].code);
 }
 
-static void __init
+static void
 errptr(void)
 {
 	test("-1234", "%pe", ERR_PTR(-1234));
@@ -813,7 +804,7 @@  errptr(void)
 #endif
 }
 
-static void __init
+static void
 test_pointer(void)
 {
 	plain();
@@ -842,13 +833,15 @@  test_pointer(void)
 	fourcc_pointer();
 }
 
-static void __init selftest(void)
+static void printf_test(struct kunit *test)
 {
 	alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL);
 	if (!alloced_buffer)
 		return;
 	test_buffer = alloced_buffer + PAD_SIZE;
 
+	kunittest = test;
+
 	test_basic();
 	test_number();
 	test_string();
@@ -857,7 +850,18 @@  static void __init selftest(void)
 	kfree(alloced_buffer);
 }
 
-KSTM_MODULE_LOADERS(test_printf);
+static struct kunit_case printf_test_cases[] = {
+	KUNIT_CASE(printf_test),
+	{}
+};
+
+static struct kunit_suite printf_test_suite = {
+	.name = "printf",
+	.test_cases = printf_test_cases,
+};
+
+kunit_test_suite(printf_test_suite);
+
 MODULE_AUTHOR("Rasmus Villemoes <linux@rasmusvillemoes.dk>");
 MODULE_DESCRIPTION("Test cases for printf facility");
 MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
index dc15aba8d0a3..0a63594177c2 100644
--- a/tools/testing/selftests/lib/config
+++ b/tools/testing/selftests/lib/config
@@ -1,4 +1,3 @@ 
-CONFIG_TEST_PRINTF=m
 CONFIG_TEST_SCANF=m
 CONFIG_TEST_BITMAP=m
 CONFIG_PRIME_NUMBERS=m
diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh
deleted file mode 100755
index 05f4544e87f9..000000000000
--- a/tools/testing/selftests/lib/printf.sh
+++ /dev/null
@@ -1,4 +0,0 @@ 
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-# Tests the printf infrastructure using test_printf kernel module.
-$(dirname $0)/../kselftest/module.sh "printf" test_printf