Re: [PATCH v9 1/1] smb/client: introduce KUnit test to check search result of smb2_error_map_table

[Geert Uytterhoeven]

Hi ChenXiaoSong,

Thanks for your patch, which is now commit 480afcb19b61385d
("smb/client: introduce KUnit test to check search result of
smb2_error_map_table") in linus/master

> The KUnit test are executed when cifs.ko is loaded.

This means the tests are _always_ executed when cifs.ko is loaded,
which is different from how most other test modules work.
Please make it a separate test module, so it can be loaded independently
of the main cifs module. That way people can enable all tests in
production kernels, without affecting the system unless a test module
is actually loaded.

> Just like `fs/ext4/mballoc.c` includes `fs/ext4/mballoc-test.c`.
> `smb2maperror.c` also includes `smb2maperror_test.c`, allowing KUnit
> tests to access any functions and variables in `smb2maperror.c`.
>
> The maperror_test_check_search() checks whether all elements can be
> correctly found in the array.
>
> Suggested-by: David Howells <dhow...@redhat.com>
> Signed-off-by: ChenXiaoSong <chenxi...@kylinos.cn>
> ---
> fs/smb/Kconfig | 17 ++++++++++++
> fs/smb/client/smb2maperror.c | 8 ++++++
> fs/smb/client/smb2maperror_test.c | 45 +++++++++++++++++++++++++++++++
> 3 files changed, 70 insertions(+)
> create mode 100644 fs/smb/client/smb2maperror_test.c
>
> diff --git a/fs/smb/Kconfig b/fs/smb/Kconfig
> index ef425789fa6a..85f7ad5fbc5e 100644
> --- a/fs/smb/Kconfig
> +++ b/fs/smb/Kconfig
> @@ -9,3 +9,20 @@ config SMBFS
> tristate
> default y if CIFS=y || SMB_SERVER=y
> default m if CIFS=m || SMB_SERVER=m
> +
> +config SMB_KUNIT_TESTS
> + tristate "KUnit tests for SMB" if !KUNIT_ALL_TESTS
> + depends on SMBFS && KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This builds the SMB KUnit tests.
> +
> + KUnit tests run during boot and output the results to the debug log
> + in TAP format (https://testanything.org/). Only useful for kernel devs
> + running KUnit test harness and are not 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.
> diff --git a/fs/smb/client/smb2maperror.c b/fs/smb/client/smb2maperror.c
> index 090bbd10623d..cd036365201f 100644
> --- a/fs/smb/client/smb2maperror.c
> +++ b/fs/smb/client/smb2maperror.c
> @@ -114,3 +114,11 @@ int __init smb2_init_maperror(void)
>
> return 0;
> }
> +
> +#define SMB_CLIENT_KUNIT_AVAILABLE \
> + ((IS_MODULE(CONFIG_CIFS) && IS_ENABLED(CONFIG_KUNIT)) || \
> + (IS_BUILTIN(CONFIG_CIFS) && IS_BUILTIN(CONFIG_KUNIT)))
> +
> +#if SMB_CLIENT_KUNIT_AVAILABLE && IS_ENABLED(CONFIG_SMB_KUNIT_TESTS)
> +#include "smb2maperror_test.c"
> +#endif /* CONFIG_SMB_KUNIT_TESTS */
> diff --git a/fs/smb/client/smb2maperror_test.c b/fs/smb/client/smb2maperror_test.c
> new file mode 100644
> index 000000000000..38ea6b846a99
> --- /dev/null
> +++ b/fs/smb/client/smb2maperror_test.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +/*
> + *
> + * KUnit tests of SMB2 maperror
> + *
> + * Copyright (C) 2025 KylinSoft Co., Ltd. All rights reserved.
> + * Author(s): ChenXiaoSong <chenxi...@kylinos.cn>
> + *
> + */
> +
> +#include <kunit/test.h>
> +
> +static void
> +test_cmp_map(struct kunit *test, const struct status_to_posix_error *expect)
> +{
> + const struct status_to_posix_error *result;
> +
> + result = smb2_get_err_map(expect->smb2_status);
> + KUNIT_EXPECT_PTR_NE(test, NULL, result);
> + KUNIT_EXPECT_EQ(test, expect->smb2_status, result->smb2_status);
> + KUNIT_EXPECT_EQ(test, expect->posix_error, result->posix_error);
> + KUNIT_EXPECT_STREQ(test, expect->status_string, result->status_string);
> +}
> +
> +static void maperror_test_check_search(struct kunit *test)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(smb2_error_map_table); i++)
> + test_cmp_map(test, &smb2_error_map_table[i]);
> +}
> +
> +static struct kunit_case maperror_test_cases[] = {
> + KUNIT_CASE(maperror_test_check_search),
> + {}
> +};
> +
> +static struct kunit_suite maperror_suite = {
> + .name = "smb2_maperror",
> + .test_cases = maperror_test_cases,
> +};
> +
> +kunit_test_suite(maperror_suite);
> +
> +MODULE_LICENSE("GPL");

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

[ChenXiaoSong]

Hi Geert,

Thank you for reviewing this patch.

The KUnit test cases are only executed when the CONFIG_SMB_KUNIT_TESTS
is enabled.

Making it a separate test module would require exporting local variables
and functions so that the test code can access them. However, exporting
local variables and functions would likely make the code much uglier, as
it would require adding "#if" conditionals into the production code to
isolate the test code.

Geert, please let me know if you have a better idea.

I am also discussing this with the ext4 community, and we all hope to
find a way to make the tests a separate module.

Thanks,
ChenXiaoSong <chenxi...@kylinos.cn>

On 2/10/26 4:10 PM, Geert Uytterhoeven wrote:
> Hi ChenXiaoSong,
>
> Thanks for your patch, which is now commit 480afcb19b61385d
> ("smb/client: introduce KUnit test to check search result of
> smb2_error_map_table") in linus/master
>
>> The KUnit test are executed when cifs.ko is loaded.

> This means the tests are_always_ executed when cifs.ko is loaded,

[Geert Uytterhoeven]

Hi ChenXiaoSong,

On Tue, 10 Feb 2026 at 10:53, ChenXiaoSong
<chenxiaosong...@linux.dev> wrote:
> The KUnit test cases are only executed when the CONFIG_SMB_KUNIT_TESTS
> is enabled.

... which defaults to KUNIT_ALL_TESTS, so if KUNIT_ALL_TESTS is enabled,
the test is enabled by default, too.

> Making it a separate test module would require exporting local variables
> and functions so that the test code can access them. However, exporting
> local variables and functions would likely make the code much uglier, as
> it would require adding "#if" conditionals into the production code to
> isolate the test code.
>
> Geert, please let me know if you have a better idea.
>
> I am also discussing this with the ext4 community, and we all hope to
> find a way to make the tests a separate module.

There are ways to restrict exported symbols to test modules only,
see EXPORT_SYMBOL_FOR_TESTS_ONLY(), EXPORT_SYMBOL_FOR_MODULES(),
and EXPORT_SYMBOL_NS().

If it is really hard to convert the tests into a separate module,
you can add a new kernel/module parameter, which needs to be specified
explicitly to run the test. That would avoid running the tests when just
(auto)loading cifs.ko.

> On 2/10/26 4:10 PM, Geert Uytterhoeven wrote:
> > Thanks for your patch, which is now commit 480afcb19b61385d
> > ("smb/client: introduce KUnit test to check search result of
> > smb2_error_map_table") in linus/master
> >
> >> The KUnit test are executed when cifs.ko is loaded.
> > This means the tests are_always_ executed when cifs.ko is loaded,
> > which is different from how most other test modules work.
> > Please make it a separate test module, so it can be loaded independently
> > of the main cifs module. That way people can enable all tests in
> > production kernels, without affecting the system unless a test module
> > is actually loaded.

[ChenXiaoSong]

Thanks for your suggestions, these are really helpful. I will make the
changes as soon as possible.

Thanks,
ChenXiaoSong <chenxi...@kylinos.cn>