[12/18] mtd: tests: fix object shared between several modules
Commit Message
mtd_test.o is linked to 8(!) different test modules:
> scripts/Makefile.build:252: ./drivers/mtd/tests/Makefile: mtd_test.o
> is added to multiple modules: mtd_nandbiterrs mtd_oobtest mtd_pagetest
> mtd_readtest mtd_speedtest mtd_stresstest mtd_subpagetest mtd_torturetest
Although all of them share one Kconfig option
(CONFIG_MTD_TESTS), it's better to not link one object file into
several modules (and/or vmlinux).
Under certain circumstances, such can lead to the situation fixed by
commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
In this particular case, there's also no need to duplicate the very
same object code 8 times.
Convert mtd_test.o to a standalone module which will export its
functions to the rest.
Fixes: a995c792280d ("mtd: tests: rename sources in order to link a helper object")
Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
drivers/mtd/tests/Makefile | 17 +++++++++--------
drivers/mtd/tests/mtd_test.c | 9 +++++++++
drivers/mtd/tests/nandbiterrs.c | 2 ++
drivers/mtd/tests/oobtest.c | 2 ++
drivers/mtd/tests/pagetest.c | 2 ++
drivers/mtd/tests/readtest.c | 2 ++
drivers/mtd/tests/speedtest.c | 2 ++
drivers/mtd/tests/stresstest.c | 2 ++
drivers/mtd/tests/subpagetest.c | 2 ++
drivers/mtd/tests/torturetest.c | 2 ++
10 files changed, 34 insertions(+), 8 deletions(-)
--
2.38.1
Comments
On Sun, Nov 20, 2022 at 8:08 AM Alexander Lobakin <alobakin@pm.me> wrote:
>
> mtd_test.o is linked to 8(!) different test modules:
>
> > scripts/Makefile.build:252: ./drivers/mtd/tests/Makefile: mtd_test.o
> > is added to multiple modules: mtd_nandbiterrs mtd_oobtest mtd_pagetest
> > mtd_readtest mtd_speedtest mtd_stresstest mtd_subpagetest mtd_torturetest
>
> Although all of them share one Kconfig option
> (CONFIG_MTD_TESTS), it's better to not link one object file into
> several modules (and/or vmlinux).
> Under certain circumstances, such can lead to the situation fixed by
> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> In this particular case, there's also no need to duplicate the very
> same object code 8 times.
>
> Convert mtd_test.o to a standalone module which will export its
> functions to the rest.
>
> Fixes: a995c792280d ("mtd: tests: rename sources in order to link a helper object")
> Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
IMHO, Reported-by might be a better fit.
I think they can become static inline functions in mtd_test.h
(at least, mtdtest_relax() is a static inline there), but I am not sure.
Please send this to the MTD list, and consult the maintainer(s).
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
> drivers/mtd/tests/Makefile | 17 +++++++++--------
> drivers/mtd/tests/mtd_test.c | 9 +++++++++
> drivers/mtd/tests/nandbiterrs.c | 2 ++
> drivers/mtd/tests/oobtest.c | 2 ++
> drivers/mtd/tests/pagetest.c | 2 ++
> drivers/mtd/tests/readtest.c | 2 ++
> drivers/mtd/tests/speedtest.c | 2 ++
> drivers/mtd/tests/stresstest.c | 2 ++
> drivers/mtd/tests/subpagetest.c | 2 ++
> drivers/mtd/tests/torturetest.c | 2 ++
> 10 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/tests/Makefile b/drivers/mtd/tests/Makefile
> index 5de0378f90db..e3f86ed123ca 100644
> --- a/drivers/mtd/tests/Makefile
> +++ b/drivers/mtd/tests/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_MTD_TESTS) += mtd_test.o
> obj-$(CONFIG_MTD_TESTS) += mtd_oobtest.o
> obj-$(CONFIG_MTD_TESTS) += mtd_pagetest.o
> obj-$(CONFIG_MTD_TESTS) += mtd_readtest.o
> @@ -9,11 +10,11 @@ obj-$(CONFIG_MTD_TESTS) += mtd_torturetest.o
> obj-$(CONFIG_MTD_TESTS) += mtd_nandecctest.o
> obj-$(CONFIG_MTD_TESTS) += mtd_nandbiterrs.o
>
> -mtd_oobtest-objs := oobtest.o mtd_test.o
> -mtd_pagetest-objs := pagetest.o mtd_test.o
> -mtd_readtest-objs := readtest.o mtd_test.o
> -mtd_speedtest-objs := speedtest.o mtd_test.o
> -mtd_stresstest-objs := stresstest.o mtd_test.o
> -mtd_subpagetest-objs := subpagetest.o mtd_test.o
> -mtd_torturetest-objs := torturetest.o mtd_test.o
> -mtd_nandbiterrs-objs := nandbiterrs.o mtd_test.o
> +mtd_oobtest-objs := oobtest.o
> +mtd_pagetest-objs := pagetest.o
> +mtd_readtest-objs := readtest.o
> +mtd_speedtest-objs := speedtest.o
> +mtd_stresstest-objs := stresstest.o
> +mtd_subpagetest-objs := subpagetest.o
> +mtd_torturetest-objs := torturetest.o
> +mtd_nandbiterrs-objs := nandbiterrs.o
> diff --git a/drivers/mtd/tests/mtd_test.c b/drivers/mtd/tests/mtd_test.c
> index c84250beffdc..93920a714315 100644
> --- a/drivers/mtd/tests/mtd_test.c
> +++ b/drivers/mtd/tests/mtd_test.c
> @@ -25,6 +25,7 @@ int mtdtest_erase_eraseblock(struct mtd_info *mtd, unsigned int ebnum)
>
> return 0;
> }
> +EXPORT_SYMBOL_NS_GPL(mtdtest_erase_eraseblock, MTD_TESTS);
>
> static int is_block_bad(struct mtd_info *mtd, unsigned int ebnum)
> {
> @@ -57,6 +58,7 @@ int mtdtest_scan_for_bad_eraseblocks(struct mtd_info *mtd, unsigned char *bbt,
>
> return 0;
> }
> +EXPORT_SYMBOL_NS_GPL(mtdtest_scan_for_bad_eraseblocks, MTD_TESTS);
>
> int mtdtest_erase_good_eraseblocks(struct mtd_info *mtd, unsigned char *bbt,
> unsigned int eb, int ebcnt)
> @@ -75,6 +77,7 @@ int mtdtest_erase_good_eraseblocks(struct mtd_info *mtd, unsigned char *bbt,
>
> return 0;
> }
> +EXPORT_SYMBOL_NS_GPL(mtdtest_erase_good_eraseblocks, MTD_TESTS);
>
> int mtdtest_read(struct mtd_info *mtd, loff_t addr, size_t size, void *buf)
> {
> @@ -92,6 +95,7 @@ int mtdtest_read(struct mtd_info *mtd, loff_t addr, size_t size, void *buf)
>
> return err;
> }
> +EXPORT_SYMBOL_NS_GPL(mtdtest_read, MTD_TESTS);
>
> int mtdtest_write(struct mtd_info *mtd, loff_t addr, size_t size,
> const void *buf)
> @@ -107,3 +111,8 @@ int mtdtest_write(struct mtd_info *mtd, loff_t addr, size_t size,
>
> return err;
> }
> +EXPORT_SYMBOL_NS_GPL(mtdtest_write, MTD_TESTS);
> +
> +MODULE_DESCRIPTION("MTD test common module");
> +MODULE_AUTHOR("Adrian Hunter");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/nandbiterrs.c b/drivers/mtd/tests/nandbiterrs.c
> index 98d7508f95b1..acf44edfca53 100644
> --- a/drivers/mtd/tests/nandbiterrs.c
> +++ b/drivers/mtd/tests/nandbiterrs.c
> @@ -414,6 +414,8 @@ static void __exit mtd_nandbiterrs_exit(void)
> module_init(mtd_nandbiterrs_init);
> module_exit(mtd_nandbiterrs_exit);
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("NAND bit error recovery test");
> MODULE_AUTHOR("Iwo Mergler");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
> index 13fed398937e..da4efcdd59b2 100644
> --- a/drivers/mtd/tests/oobtest.c
> +++ b/drivers/mtd/tests/oobtest.c
> @@ -728,6 +728,8 @@ static void __exit mtd_oobtest_exit(void)
> }
> module_exit(mtd_oobtest_exit);
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("Out-of-band test module");
> MODULE_AUTHOR("Adrian Hunter");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/pagetest.c b/drivers/mtd/tests/pagetest.c
> index 8eb40b6e6dfa..ac2bcc76b402 100644
> --- a/drivers/mtd/tests/pagetest.c
> +++ b/drivers/mtd/tests/pagetest.c
> @@ -456,6 +456,8 @@ static void __exit mtd_pagetest_exit(void)
> }
> module_exit(mtd_pagetest_exit);
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("NAND page test");
> MODULE_AUTHOR("Adrian Hunter");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/readtest.c b/drivers/mtd/tests/readtest.c
> index 99670ef91f2b..7e01dbc1e8ca 100644
> --- a/drivers/mtd/tests/readtest.c
> +++ b/drivers/mtd/tests/readtest.c
> @@ -210,6 +210,8 @@ static void __exit mtd_readtest_exit(void)
> }
> module_exit(mtd_readtest_exit);
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("Read test module");
> MODULE_AUTHOR("Adrian Hunter");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/speedtest.c b/drivers/mtd/tests/speedtest.c
> index 075bce32caa5..58f3701d65f2 100644
> --- a/drivers/mtd/tests/speedtest.c
> +++ b/drivers/mtd/tests/speedtest.c
> @@ -413,6 +413,8 @@ static void __exit mtd_speedtest_exit(void)
> }
> module_exit(mtd_speedtest_exit);
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("Speed test module");
> MODULE_AUTHOR("Adrian Hunter");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/stresstest.c b/drivers/mtd/tests/stresstest.c
> index 75b6ddc5dc4d..341d7cc86d89 100644
> --- a/drivers/mtd/tests/stresstest.c
> +++ b/drivers/mtd/tests/stresstest.c
> @@ -227,6 +227,8 @@ static void __exit mtd_stresstest_exit(void)
> }
> module_exit(mtd_stresstest_exit);
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("Stress test module");
> MODULE_AUTHOR("Adrian Hunter");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/subpagetest.c b/drivers/mtd/tests/subpagetest.c
> index 05250a080139..87ee2a5c518a 100644
> --- a/drivers/mtd/tests/subpagetest.c
> +++ b/drivers/mtd/tests/subpagetest.c
> @@ -432,6 +432,8 @@ static void __exit mtd_subpagetest_exit(void)
> }
> module_exit(mtd_subpagetest_exit);
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("Subpage test module");
> MODULE_AUTHOR("Adrian Hunter");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/mtd/tests/torturetest.c b/drivers/mtd/tests/torturetest.c
> index 841689b4d86d..2de770f18724 100644
> --- a/drivers/mtd/tests/torturetest.c
> +++ b/drivers/mtd/tests/torturetest.c
> @@ -475,6 +475,8 @@ static int countdiffs(unsigned char *buf, unsigned char *check_buf,
> return first;
> }
>
> +MODULE_IMPORT_NS(MTD_TESTS);
> +
> MODULE_DESCRIPTION("Eraseblock torturing module");
> MODULE_AUTHOR("Artem Bityutskiy, Jarkko Lavinen, Adrian Hunter");
> MODULE_LICENSE("GPL");
> --
> 2.38.1
>
>
Hi,
masahiroy@kernel.org wrote on Wed, 23 Nov 2022 22:11:49 +0900:
> On Sun, Nov 20, 2022 at 8:08 AM Alexander Lobakin <alobakin@pm.me> wrote:
> >
> > mtd_test.o is linked to 8(!) different test modules:
> >
> > > scripts/Makefile.build:252: ./drivers/mtd/tests/Makefile: mtd_test.o
> > > is added to multiple modules: mtd_nandbiterrs mtd_oobtest mtd_pagetest
> > > mtd_readtest mtd_speedtest mtd_stresstest mtd_subpagetest mtd_torturetest
> >
> > Although all of them share one Kconfig option
> > (CONFIG_MTD_TESTS), it's better to not link one object file into
> > several modules (and/or vmlinux).
> > Under certain circumstances, such can lead to the situation fixed by
> > commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> > In this particular case, there's also no need to duplicate the very
> > same object code 8 times.
> >
> > Convert mtd_test.o to a standalone module which will export its
> > functions to the rest.
> >
> > Fixes: a995c792280d ("mtd: tests: rename sources in order to link a helper object")
> > Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
>
> IMHO, Reported-by might be a better fit.
>
>
> I think they can become static inline functions in mtd_test.h
> (at least, mtdtest_relax() is a static inline there), but I am not sure.
>
> Please send this to the MTD list, and consult the maintainer(s).
TBH I don't really mind. These are test modules that you insert to
harden and profile the stack, so whatever makes the robots happy is
fine. Anyway, they are being slowly replaced by userspace tools so we
might eventually get rid of them.
Thanks,
Miquèl
Hello,
Reducing the Cc list.
miquel.raynal@bootlin.com wrote on Wed, 23 Nov 2022 17:59:00 +0100:
> Hi,
>
> masahiroy@kernel.org wrote on Wed, 23 Nov 2022 22:11:49 +0900:
>
> > On Sun, Nov 20, 2022 at 8:08 AM Alexander Lobakin <alobakin@pm.me> wrote:
> > >
> > > mtd_test.o is linked to 8(!) different test modules:
> > >
> > > > scripts/Makefile.build:252: ./drivers/mtd/tests/Makefile: mtd_test.o
> > > > is added to multiple modules: mtd_nandbiterrs mtd_oobtest mtd_pagetest
> > > > mtd_readtest mtd_speedtest mtd_stresstest mtd_subpagetest mtd_torturetest
> > >
> > > Although all of them share one Kconfig option
> > > (CONFIG_MTD_TESTS), it's better to not link one object file into
> > > several modules (and/or vmlinux).
> > > Under certain circumstances, such can lead to the situation fixed by
> > > commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> > > In this particular case, there's also no need to duplicate the very
> > > same object code 8 times.
> > >
> > > Convert mtd_test.o to a standalone module which will export its
> > > functions to the rest.
> > >
> > > Fixes: a995c792280d ("mtd: tests: rename sources in order to link a helper object")
> > > Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> >
> > IMHO, Reported-by might be a better fit.
> >
> >
> > I think they can become static inline functions in mtd_test.h
> > (at least, mtdtest_relax() is a static inline there), but I am not sure.
> >
> > Please send this to the MTD list, and consult the maintainer(s).
>
> TBH I don't really mind. These are test modules that you insert to
> harden and profile the stack, so whatever makes the robots happy is
> fine. Anyway, they are being slowly replaced by userspace tools so we
> might eventually get rid of them.
Once you feel ready, if you want this to be merged through the mtd
tree, you can resend with the linux-mtd ML in Cc, otherwise if falls out
of the scope of the tools and it's a pain to collect.
Thanks,
Miquèl
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_MTD_TESTS) += mtd_test.o
obj-$(CONFIG_MTD_TESTS) += mtd_oobtest.o
obj-$(CONFIG_MTD_TESTS) += mtd_pagetest.o
obj-$(CONFIG_MTD_TESTS) += mtd_readtest.o
@@ -9,11 +10,11 @@ obj-$(CONFIG_MTD_TESTS) += mtd_torturetest.o
obj-$(CONFIG_MTD_TESTS) += mtd_nandecctest.o
obj-$(CONFIG_MTD_TESTS) += mtd_nandbiterrs.o
-mtd_oobtest-objs := oobtest.o mtd_test.o
-mtd_pagetest-objs := pagetest.o mtd_test.o
-mtd_readtest-objs := readtest.o mtd_test.o
-mtd_speedtest-objs := speedtest.o mtd_test.o
-mtd_stresstest-objs := stresstest.o mtd_test.o
-mtd_subpagetest-objs := subpagetest.o mtd_test.o
-mtd_torturetest-objs := torturetest.o mtd_test.o
-mtd_nandbiterrs-objs := nandbiterrs.o mtd_test.o
+mtd_oobtest-objs := oobtest.o
+mtd_pagetest-objs := pagetest.o
+mtd_readtest-objs := readtest.o
+mtd_speedtest-objs := speedtest.o
+mtd_stresstest-objs := stresstest.o
+mtd_subpagetest-objs := subpagetest.o
+mtd_torturetest-objs := torturetest.o
+mtd_nandbiterrs-objs := nandbiterrs.o
@@ -25,6 +25,7 @@ int mtdtest_erase_eraseblock(struct mtd_info *mtd, unsigned int ebnum)
return 0;
}
+EXPORT_SYMBOL_NS_GPL(mtdtest_erase_eraseblock, MTD_TESTS);
static int is_block_bad(struct mtd_info *mtd, unsigned int ebnum)
{
@@ -57,6 +58,7 @@ int mtdtest_scan_for_bad_eraseblocks(struct mtd_info *mtd, unsigned char *bbt,
return 0;
}
+EXPORT_SYMBOL_NS_GPL(mtdtest_scan_for_bad_eraseblocks, MTD_TESTS);
int mtdtest_erase_good_eraseblocks(struct mtd_info *mtd, unsigned char *bbt,
unsigned int eb, int ebcnt)
@@ -75,6 +77,7 @@ int mtdtest_erase_good_eraseblocks(struct mtd_info *mtd, unsigned char *bbt,
return 0;
}
+EXPORT_SYMBOL_NS_GPL(mtdtest_erase_good_eraseblocks, MTD_TESTS);
int mtdtest_read(struct mtd_info *mtd, loff_t addr, size_t size, void *buf)
{
@@ -92,6 +95,7 @@ int mtdtest_read(struct mtd_info *mtd, loff_t addr, size_t size, void *buf)
return err;
}
+EXPORT_SYMBOL_NS_GPL(mtdtest_read, MTD_TESTS);
int mtdtest_write(struct mtd_info *mtd, loff_t addr, size_t size,
const void *buf)
@@ -107,3 +111,8 @@ int mtdtest_write(struct mtd_info *mtd, loff_t addr, size_t size,
return err;
}
+EXPORT_SYMBOL_NS_GPL(mtdtest_write, MTD_TESTS);
+
+MODULE_DESCRIPTION("MTD test common module");
+MODULE_AUTHOR("Adrian Hunter");
+MODULE_LICENSE("GPL");
@@ -414,6 +414,8 @@ static void __exit mtd_nandbiterrs_exit(void)
module_init(mtd_nandbiterrs_init);
module_exit(mtd_nandbiterrs_exit);
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("NAND bit error recovery test");
MODULE_AUTHOR("Iwo Mergler");
MODULE_LICENSE("GPL");
@@ -728,6 +728,8 @@ static void __exit mtd_oobtest_exit(void)
}
module_exit(mtd_oobtest_exit);
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("Out-of-band test module");
MODULE_AUTHOR("Adrian Hunter");
MODULE_LICENSE("GPL");
@@ -456,6 +456,8 @@ static void __exit mtd_pagetest_exit(void)
}
module_exit(mtd_pagetest_exit);
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("NAND page test");
MODULE_AUTHOR("Adrian Hunter");
MODULE_LICENSE("GPL");
@@ -210,6 +210,8 @@ static void __exit mtd_readtest_exit(void)
}
module_exit(mtd_readtest_exit);
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("Read test module");
MODULE_AUTHOR("Adrian Hunter");
MODULE_LICENSE("GPL");
@@ -413,6 +413,8 @@ static void __exit mtd_speedtest_exit(void)
}
module_exit(mtd_speedtest_exit);
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("Speed test module");
MODULE_AUTHOR("Adrian Hunter");
MODULE_LICENSE("GPL");
@@ -227,6 +227,8 @@ static void __exit mtd_stresstest_exit(void)
}
module_exit(mtd_stresstest_exit);
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("Stress test module");
MODULE_AUTHOR("Adrian Hunter");
MODULE_LICENSE("GPL");
@@ -432,6 +432,8 @@ static void __exit mtd_subpagetest_exit(void)
}
module_exit(mtd_subpagetest_exit);
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("Subpage test module");
MODULE_AUTHOR("Adrian Hunter");
MODULE_LICENSE("GPL");
@@ -475,6 +475,8 @@ static int countdiffs(unsigned char *buf, unsigned char *check_buf,
return first;
}
+MODULE_IMPORT_NS(MTD_TESTS);
+
MODULE_DESCRIPTION("Eraseblock torturing module");
MODULE_AUTHOR("Artem Bityutskiy, Jarkko Lavinen, Adrian Hunter");
MODULE_LICENSE("GPL");