| Message ID | Z0ZxiLw9hauUynTS@bombadil.infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] Modules changes for v6.13-rc1 | expand |
| Context | Check | Description |
|---|---|---|
| mcgrof/vmtest-modules-next-PR | fail | merge-conflict |
The pull request you sent on Tue, 26 Nov 2024 17:10:32 -0800:
> git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git/ tags/modules-6.13-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b5361254c9027c2b3730be1bebcdb37eed42e9a5
Thank you!
On Tue, 26 Nov 2024 at 17:10, Luis Chamberlain <mcgrof@kernel.org> wrote: > > Highlights for this merge window: Lowlights: > * Adds a new modules selftests: kallsyms which helps us tests find_symbol() > and the limits of kallsyms on Linux today. This is completely broken. Try doing "make allmodconfig" and then do "make" twice in a row. It re-generates the tests, so you see this: GEN lib/tests/module/test_kallsyms_a.c GEN lib/tests/module/test_kallsyms_b.c GEN lib/tests/module/test_kallsyms_c.c GEN lib/tests/module/test_kallsyms_d.c CC [M] lib/tests/module/test_kallsyms_a.o CC [M] lib/tests/module/test_kallsyms_b.o CC [M] lib/tests/module/test_kallsyms_c.o CC [M] lib/tests/module/test_kallsyms_d.o LD [M] lib/tests/module/test_kallsyms_a.ko LD [M] lib/tests/module/test_kallsyms_b.ko LD [M] lib/tests/module/test_kallsyms_c.ko LD [M] lib/tests/module/test_kallsyms_d.ko both times, and this has made the empty build slow down by about a factor of two. Which is a big deal, because the "empty build" is actually fairly important for me. It's the baseline build test performance, and with small pulls it actually dominates. So this isn't ok. Please fix asap, because otherwise I will just revert it - it really does affect my workflow that much. Linus
On Wed, Nov 27, 2024 at 01:22:37PM -0800, Linus Torvalds wrote: > On Tue, 26 Nov 2024 at 17:10, Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > Highlights for this merge window: > > Lowlights: > > > * Adds a new modules selftests: kallsyms which helps us tests find_symbol() > > and the limits of kallsyms on Linux today. > > This is completely broken. > > Try doing "make allmodconfig" and then do "make" twice in a row. > > It re-generates the tests, so you see this: > > GEN lib/tests/module/test_kallsyms_a.c > GEN lib/tests/module/test_kallsyms_b.c > GEN lib/tests/module/test_kallsyms_c.c > GEN lib/tests/module/test_kallsyms_d.c > CC [M] lib/tests/module/test_kallsyms_a.o > CC [M] lib/tests/module/test_kallsyms_b.o > CC [M] lib/tests/module/test_kallsyms_c.o > CC [M] lib/tests/module/test_kallsyms_d.o > LD [M] lib/tests/module/test_kallsyms_a.ko > LD [M] lib/tests/module/test_kallsyms_b.ko > LD [M] lib/tests/module/test_kallsyms_c.ko > LD [M] lib/tests/module/test_kallsyms_d.ko > > both times, and this has made the empty build slow down by about a > factor of two. > > Which is a big deal, because the "empty build" is actually fairly > important for me. It's the baseline build test performance, and with > small pulls it actually dominates. > > So this isn't ok. Please fix asap, because otherwise I will just > revert it - it really does affect my workflow that much. Sorry about that, I'm on it. Luis
On Wed, Nov 27, 2024 at 02:35:36PM -0800, Luis Chamberlain wrote: > Sorry about that, I'm on it. OK here is a fix, goes double build tested and then run time tested. From 6c9ef19d7676c3f650f1de3e2619c958f21c0b75 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain <mcgrof@kernel.org> Date: Wed, 27 Nov 2024 14:10:57 -0800 Subject: [PATCH] selftests: kallsyms: fix with sensible defaults Folks use 'make allmodconfig' to test building Linux, and although the test is intended to stress test that and later load insane modules, we don't need to make 'make allmodconfig' suffer. So scale down to the bare bones by default. To do this be explicit about the sensible ranges and bound the test so that folks who the impact if they enable more broader range test options. This also helps us clarify where we run into existing limits today. Also, avoid the stupid FORCE so that re-builds will only trigger when really needed. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- lib/Kconfig.debug | 38 ++++++++++++++++++++++++++- lib/tests/module/Makefile | 2 +- lib/tests/module/gen_test_kallsyms.sh | 9 +++++-- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index b5929721fc63..31b843690a00 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2986,9 +2986,43 @@ config TEST_KALLSYMS_D tristate depends on m +choice + prompt "Kallsym test range" + default TEST_KALLSYMS_FAST + help + Selecting something other than "Fast" will enable tests which slow + down the build and may crash your build. + +config TEST_KALLSYMS_FAST + bool "Fast builds" + help + You won't really be testing kallsysms, so this just helps fast builds + when allmodconfig is used.. + +config TEST_KALLSYMS_LARGE + bool "Enable testing kallsyms with large exports" + help + This will enable larger number of symbols. Only enable this if you + are a modules developer. This will slow down your build considerbly + by at least 2x. + +config TEST_KALLSYMS_MAX + bool "Known kallsysms limits" + help + This will enable exports to the point we know we'll start crashing + builds. + +endchoice + + config TEST_KALLSYMS_NUMSYMS int "test kallsyms number of symbols" - default 100 + range 2 8 if TEST_KALLSYMS_FAST + range 8 100 if TEST_KALLSYMS_LARGE + range 100 1000 if TEST_KALLSYMS_MAX + default 2 if TEST_KALLSYMS_FAST + default 8 if TEST_KALLSYMS_LARGE + default 100 if TEST_KALLSYMS_MAX help The number of symbols to create on TEST_KALLSYMS_A, only one of which module TEST_KALLSYMS_B will use. This also will be used @@ -2997,6 +3031,8 @@ config TEST_KALLSYMS_NUMSYMS trigger a segfault today, don't use anything close to it unless you are aware that this should not be used for automated build tests. + We use sensible default to not slow down default allmodconfig builds. + config TEST_KALLSYMS_SCALE_FACTOR int "test kallsyms scale factor" default 8 diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile index af5c27b996cb..5436386d7aa0 100644 --- a/lib/tests/module/Makefile +++ b/lib/tests/module/Makefile @@ -3,7 +3,7 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o -$(obj)/%.c: FORCE +$(obj)/%.c: $(srctree)/lib/tests/module/gen_test_kallsyms.sh $(KCONFIG_CONFIG) @$(kecho) " GEN $@" $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ diff --git a/lib/tests/module/gen_test_kallsyms.sh b/lib/tests/module/gen_test_kallsyms.sh index 3f2c626350ad..561dcac0f359 100755 --- a/lib/tests/module/gen_test_kallsyms.sh +++ b/lib/tests/module/gen_test_kallsyms.sh @@ -7,6 +7,11 @@ NUM_SYMS=$2 SCALE_FACTOR=$3 TEST_TYPE=$(echo $TARGET | sed -e 's|lib/tests/module/test_kallsyms_||g') TEST_TYPE=$(echo $TEST_TYPE | sed -e 's|.c||g') +FIRST_B_LOOKUP=1 + +if [[ $NUM_SYMS -gt 2 ]]; then + FIRST_B_LOOKUP=$((NUM_SYMS/2)) +fi gen_template_module_header() { @@ -52,10 +57,10 @@ ____END_MODULE gen_template_module_data_b() { - printf "\nextern int auto_test_a_%010d;\n\n" 28 + printf "\nextern int auto_test_a_%010d;\n\n" $FIRST_B_LOOKUP echo "static int auto_runtime_test(void)" echo "{" - printf "\nreturn auto_test_a_%010d;\n" 28 + printf "\nreturn auto_test_a_%010d;\n" $FIRST_B_LOOKUP echo "}" }
On Wed, 27 Nov 2024 at 15:26, Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Wed, Nov 27, 2024 at 02:35:36PM -0800, Luis Chamberlain wrote: > > Sorry about that, I'm on it. > > OK here is a fix, goes double build tested and then run time tested. No, you misunderstand. I don't mind the tests being built. That's *good*. I mind them being built *twice*. That means that there's some seriously broken lack of dependency logic. Linus
Hi Linus and Luis, On Thu, Nov 28, 2024 at 8:57 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Wed, 27 Nov 2024 at 15:26, Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Wed, Nov 27, 2024 at 02:35:36PM -0800, Luis Chamberlain wrote: > > > Sorry about that, I'm on it. > > > > OK here is a fix, goes double build tested and then run time tested. > > No, you misunderstand. > > I don't mind the tests being built. That's *good*. > > I mind them being built *twice*. That means that there's some > seriously broken lack of dependency logic. > > Linus Right. The lib/tests/module/test_kallsyms_*.c files are always regenerated due to the 'FORCE'. lib/tests/module/Makefile: $(obj)/%.c: FORCE The following diff will fix the issue. (I used 'foreach' to factor out similar lines, but it is just a bonus clean-up). diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile index af5c27b996cb..8cfc4ae600a9 100644 --- a/lib/tests/module/Makefile +++ b/lib/tests/module/Makefile @@ -3,13 +3,12 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o -$(obj)/%.c: FORCE - @$(kecho) " GEN $@" - $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ - $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ - $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) +quiet_cmd_gen_test_kallsyms = GEN $@ + cmd_gen_test_kallsyms = $< $@ \ + $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ + $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) -clean-files += test_kallsyms_a.c -clean-files += test_kallsyms_b.c -clean-files += test_kallsyms_c.c -clean-files += test_kallsyms_d.c +$(obj)/%.c: $(src)/gen_test_kallsyms.sh FORCE + $(call if_changed,gen_test_kallsyms) + +targets += $(foreach x, a b c d, test_kallsyms_$(x).c)
On Wed, Nov 27, 2024 at 03:56:48PM -0800, Linus Torvalds wrote: > On Wed, 27 Nov 2024 at 15:26, Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Wed, Nov 27, 2024 at 02:35:36PM -0800, Luis Chamberlain wrote: > > > Sorry about that, I'm on it. > > > > OK here is a fix, goes double build tested and then run time tested. > > No, you misunderstand. > > I don't mind the tests being built. That's *good*. > > I mind them being built *twice*. That means that there's some > seriously broken lack of dependency logic. Ah, gobble gobble, got it. That was also fixed in the patch but it I also changed the default build to go fast, ok we'll revert back to the older defaults (TEST_KALLSYMS_LARGE now) now and just make it clear the double build was the issue being fixed. From f7da80262bd89a0d2c2c1a9e59f5a14b84e34f3f Mon Sep 17 00:00:00 2001 From: Luis Chamberlain <mcgrof@kernel.org> Date: Wed, 27 Nov 2024 14:10:57 -0800 Subject: [PATCH v2] selftests: kallsyms: fix double build stupidity Fix the stupid FORCE so that re-builds will only trigger when really needed. While at it, document the sensible ranges supported and fix the script to accept these alternatives. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- lib/Kconfig.debug | 32 ++++++++++++++++++++++++++- lib/tests/module/Makefile | 2 +- lib/tests/module/gen_test_kallsyms.sh | 9 ++++++-- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index b5929721fc63..da8c35bfaeaf 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2986,9 +2986,39 @@ config TEST_KALLSYMS_D tristate depends on m +choice + prompt "Kallsym test range" + default TEST_KALLSYMS_LARGE + help + Selecting something other than "Fast" will enable tests which slow + down the build and may crash your build. + +config TEST_KALLSYMS_FAST + bool "Fast builds" + help + You won't really be testing kallsysms, so this just helps fast builds + when allmodconfig is used.. + +config TEST_KALLSYMS_LARGE + bool "Enable testing kallsyms with large exports" + help + This will enable larger number of symbols. Only enable this if you + are a modules developer. This will slow down your build considerbly. + +config TEST_KALLSYMS_MAX + bool "Known kallsysms limits" + help + This will enable exports to the point we know we'll start crashing + builds. + +endchoice + config TEST_KALLSYMS_NUMSYMS int "test kallsyms number of symbols" - default 100 + range 2 10000 + default 2 if TEST_KALLSYMS_FAST + default 100 if TEST_KALLSYMS_LARGE + default 10000 if TEST_KALLSYMS_MAX help The number of symbols to create on TEST_KALLSYMS_A, only one of which module TEST_KALLSYMS_B will use. This also will be used diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile index af5c27b996cb..5436386d7aa0 100644 --- a/lib/tests/module/Makefile +++ b/lib/tests/module/Makefile @@ -3,7 +3,7 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o -$(obj)/%.c: FORCE +$(obj)/%.c: $(srctree)/lib/tests/module/gen_test_kallsyms.sh $(KCONFIG_CONFIG) @$(kecho) " GEN $@" $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ diff --git a/lib/tests/module/gen_test_kallsyms.sh b/lib/tests/module/gen_test_kallsyms.sh index 3f2c626350ad..561dcac0f359 100755 --- a/lib/tests/module/gen_test_kallsyms.sh +++ b/lib/tests/module/gen_test_kallsyms.sh @@ -7,6 +7,11 @@ NUM_SYMS=$2 SCALE_FACTOR=$3 TEST_TYPE=$(echo $TARGET | sed -e 's|lib/tests/module/test_kallsyms_||g') TEST_TYPE=$(echo $TEST_TYPE | sed -e 's|.c||g') +FIRST_B_LOOKUP=1 + +if [[ $NUM_SYMS -gt 2 ]]; then + FIRST_B_LOOKUP=$((NUM_SYMS/2)) +fi gen_template_module_header() { @@ -52,10 +57,10 @@ ____END_MODULE gen_template_module_data_b() { - printf "\nextern int auto_test_a_%010d;\n\n" 28 + printf "\nextern int auto_test_a_%010d;\n\n" $FIRST_B_LOOKUP echo "static int auto_runtime_test(void)" echo "{" - printf "\nreturn auto_test_a_%010d;\n" 28 + printf "\nreturn auto_test_a_%010d;\n" $FIRST_B_LOOKUP echo "}" }
On Thu, Nov 28, 2024 at 11:09:43AM +0900, Masahiro Yamada wrote: > diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile > index af5c27b996cb..8cfc4ae600a9 100644 > --- a/lib/tests/module/Makefile > +++ b/lib/tests/module/Makefile > @@ -3,13 +3,12 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o > obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o > obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o > > -$(obj)/%.c: FORCE > - @$(kecho) " GEN $@" > - $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ > - $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ > - $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) > +quiet_cmd_gen_test_kallsyms = GEN $@ > + cmd_gen_test_kallsyms = $< $@ \ > + $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ > + $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) > > -clean-files += test_kallsyms_a.c > -clean-files += test_kallsyms_b.c > -clean-files += test_kallsyms_c.c > -clean-files += test_kallsyms_d.c > +$(obj)/%.c: $(src)/gen_test_kallsyms.sh FORCE Thanks! We can also just replace the FORCE with $(KCONFIG_CONFIG), no? Luis
Now with Masahiro's cleanups, in my testing we don't need the FORCE anymore. From 83497e0e83d81950df54d82461b1dae629842147 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain <mcgrof@kernel.org> Date: Wed, 27 Nov 2024 14:10:57 -0800 Subject: [PATCH v3] selftests: kallsyms: fix double build stupidity Fix the stupid FORCE so that re-builds will only trigger when really needed. While at it, document the sensible ranges supported and fix the script to accept these alternatives. Also adopt Masahiro Yamada's suggested cleanups on the Makefile. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- lib/Kconfig.debug | 32 ++++++++++++++++++++++++++- lib/tests/module/Makefile | 17 +++++++------- lib/tests/module/gen_test_kallsyms.sh | 9 ++++++-- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index b5929721fc63..da8c35bfaeaf 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2986,9 +2986,39 @@ config TEST_KALLSYMS_D tristate depends on m +choice + prompt "Kallsym test range" + default TEST_KALLSYMS_LARGE + help + Selecting something other than "Fast" will enable tests which slow + down the build and may crash your build. + +config TEST_KALLSYMS_FAST + bool "Fast builds" + help + You won't really be testing kallsysms, so this just helps fast builds + when allmodconfig is used.. + +config TEST_KALLSYMS_LARGE + bool "Enable testing kallsyms with large exports" + help + This will enable larger number of symbols. Only enable this if you + are a modules developer. This will slow down your build considerbly. + +config TEST_KALLSYMS_MAX + bool "Known kallsysms limits" + help + This will enable exports to the point we know we'll start crashing + builds. + +endchoice + config TEST_KALLSYMS_NUMSYMS int "test kallsyms number of symbols" - default 100 + range 2 10000 + default 2 if TEST_KALLSYMS_FAST + default 100 if TEST_KALLSYMS_LARGE + default 10000 if TEST_KALLSYMS_MAX help The number of symbols to create on TEST_KALLSYMS_A, only one of which module TEST_KALLSYMS_B will use. This also will be used diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile index af5c27b996cb..86cc3e42e44a 100644 --- a/lib/tests/module/Makefile +++ b/lib/tests/module/Makefile @@ -3,13 +3,12 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o -$(obj)/%.c: FORCE - @$(kecho) " GEN $@" - $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ - $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ - $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) +quiet_cmd_gen_test_kallsyms = GEN $@ + cmd_gen_test_kallsyms = $< $@ \ + $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ + $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) -clean-files += test_kallsyms_a.c -clean-files += test_kallsyms_b.c -clean-files += test_kallsyms_c.c -clean-files += test_kallsyms_d.c +$(obj)/%.c: $(src)/gen_test_kallsyms.sh $(KCONFIG_CONFIG) + $(call if_changed,gen_test_kallsyms) + +targets += $(foreach x, a b c d, test_kallsyms_$(x).c) diff --git a/lib/tests/module/gen_test_kallsyms.sh b/lib/tests/module/gen_test_kallsyms.sh index 3f2c626350ad..561dcac0f359 100755 --- a/lib/tests/module/gen_test_kallsyms.sh +++ b/lib/tests/module/gen_test_kallsyms.sh @@ -7,6 +7,11 @@ NUM_SYMS=$2 SCALE_FACTOR=$3 TEST_TYPE=$(echo $TARGET | sed -e 's|lib/tests/module/test_kallsyms_||g') TEST_TYPE=$(echo $TEST_TYPE | sed -e 's|.c||g') +FIRST_B_LOOKUP=1 + +if [[ $NUM_SYMS -gt 2 ]]; then + FIRST_B_LOOKUP=$((NUM_SYMS/2)) +fi gen_template_module_header() { @@ -52,10 +57,10 @@ ____END_MODULE gen_template_module_data_b() { - printf "\nextern int auto_test_a_%010d;\n\n" 28 + printf "\nextern int auto_test_a_%010d;\n\n" $FIRST_B_LOOKUP echo "static int auto_runtime_test(void)" echo "{" - printf "\nreturn auto_test_a_%010d;\n" 28 + printf "\nreturn auto_test_a_%010d;\n" $FIRST_B_LOOKUP echo "}" }
On Thu, Nov 28, 2024 at 11:22 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Wed, Nov 27, 2024 at 03:56:48PM -0800, Linus Torvalds wrote: > > On Wed, 27 Nov 2024 at 15:26, Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > > > On Wed, Nov 27, 2024 at 02:35:36PM -0800, Luis Chamberlain wrote: > > > > Sorry about that, I'm on it. > > > > > > OK here is a fix, goes double build tested and then run time tested. > > > > No, you misunderstand. > > > > I don't mind the tests being built. That's *good*. > > > > I mind them being built *twice*. That means that there's some > > seriously broken lack of dependency logic. > > Ah, gobble gobble, got it. No. You still misunderstood what Linus said. Linus did not suggest changing Kconfig or shell script or whatever. He just pointed out "your Makefile was wrong". I guess you should take some time to study Documentation/kbuild/makefiles.rst > That was also fixed in the patch but it I > also changed the default build to go fast, ok we'll revert back to the > older defaults (TEST_KALLSYMS_LARGE now) now and just make it clear the > double build was the issue being fixed. > > From f7da80262bd89a0d2c2c1a9e59f5a14b84e34f3f Mon Sep 17 00:00:00 2001 > From: Luis Chamberlain <mcgrof@kernel.org> > Date: Wed, 27 Nov 2024 14:10:57 -0800 > Subject: [PATCH v2] selftests: kallsyms: fix double build stupidity > > Fix the stupid FORCE so that re-builds will only trigger > when really needed. While at it, document the sensible ranges > supported and fix the script to accept these alternatives. > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > lib/Kconfig.debug | 32 ++++++++++++++++++++++++++- > lib/tests/module/Makefile | 2 +- > lib/tests/module/gen_test_kallsyms.sh | 9 ++++++-- > 3 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index b5929721fc63..da8c35bfaeaf 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2986,9 +2986,39 @@ config TEST_KALLSYMS_D > tristate > depends on m > > +choice > + prompt "Kallsym test range" > + default TEST_KALLSYMS_LARGE > + help > + Selecting something other than "Fast" will enable tests which slow > + down the build and may crash your build. > + > +config TEST_KALLSYMS_FAST > + bool "Fast builds" > + help > + You won't really be testing kallsysms, so this just helps fast builds > + when allmodconfig is used.. > + > +config TEST_KALLSYMS_LARGE > + bool "Enable testing kallsyms with large exports" > + help > + This will enable larger number of symbols. Only enable this if you > + are a modules developer. This will slow down your build considerbly. > + > +config TEST_KALLSYMS_MAX > + bool "Known kallsysms limits" > + help > + This will enable exports to the point we know we'll start crashing > + builds. > + > +endchoice > + > config TEST_KALLSYMS_NUMSYMS > int "test kallsyms number of symbols" > - default 100 > + range 2 10000 > + default 2 if TEST_KALLSYMS_FAST > + default 100 if TEST_KALLSYMS_LARGE > + default 10000 if TEST_KALLSYMS_MAX > help > The number of symbols to create on TEST_KALLSYMS_A, only one of which > module TEST_KALLSYMS_B will use. This also will be used > diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile > index af5c27b996cb..5436386d7aa0 100644 > --- a/lib/tests/module/Makefile > +++ b/lib/tests/module/Makefile > @@ -3,7 +3,7 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o > obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o > obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o > > -$(obj)/%.c: FORCE > +$(obj)/%.c: $(srctree)/lib/tests/module/gen_test_kallsyms.sh $(KCONFIG_CONFIG) > @$(kecho) " GEN $@" > $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ > $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ > diff --git a/lib/tests/module/gen_test_kallsyms.sh b/lib/tests/module/gen_test_kallsyms.sh > index 3f2c626350ad..561dcac0f359 100755 > --- a/lib/tests/module/gen_test_kallsyms.sh > +++ b/lib/tests/module/gen_test_kallsyms.sh > @@ -7,6 +7,11 @@ NUM_SYMS=$2 > SCALE_FACTOR=$3 > TEST_TYPE=$(echo $TARGET | sed -e 's|lib/tests/module/test_kallsyms_||g') > TEST_TYPE=$(echo $TEST_TYPE | sed -e 's|.c||g') > +FIRST_B_LOOKUP=1 > + > +if [[ $NUM_SYMS -gt 2 ]]; then > + FIRST_B_LOOKUP=$((NUM_SYMS/2)) > +fi > > gen_template_module_header() > { > @@ -52,10 +57,10 @@ ____END_MODULE > > gen_template_module_data_b() > { > - printf "\nextern int auto_test_a_%010d;\n\n" 28 > + printf "\nextern int auto_test_a_%010d;\n\n" $FIRST_B_LOOKUP > echo "static int auto_runtime_test(void)" > echo "{" > - printf "\nreturn auto_test_a_%010d;\n" 28 > + printf "\nreturn auto_test_a_%010d;\n" $FIRST_B_LOOKUP > echo "}" > } > > -- > 2.45.2 > -- Best Regards Masahiro Yamada
On Thu, Nov 28, 2024 at 11:38 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Thu, Nov 28, 2024 at 11:09:43AM +0900, Masahiro Yamada wrote: > > diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile > > index af5c27b996cb..8cfc4ae600a9 100644 > > --- a/lib/tests/module/Makefile > > +++ b/lib/tests/module/Makefile > > @@ -3,13 +3,12 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o > > obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o > > obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o > > > > -$(obj)/%.c: FORCE > > - @$(kecho) " GEN $@" > > - $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ > > - $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ > > - $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) > > +quiet_cmd_gen_test_kallsyms = GEN $@ > > + cmd_gen_test_kallsyms = $< $@ \ > > + $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ > > + $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) > > > > -clean-files += test_kallsyms_a.c > > -clean-files += test_kallsyms_b.c > > -clean-files += test_kallsyms_c.c > > -clean-files += test_kallsyms_d.c > > +$(obj)/%.c: $(src)/gen_test_kallsyms.sh FORCE > > Thanks! We can also just replace the FORCE with $(KCONFIG_CONFIG), no? Nope. Absolutely. If you change any unrelated CONFIG option, (e.g. CONFIG_DRIVER_FOO), the timestamp of .config is updated, then lib/tests/module/test_kallsyms_*.c are rebuilt for non good reason. -- Best Regards Masahiro Yamada
On Thu, Nov 28, 2024 at 11:42 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > Now with Masahiro's cleanups, in my testing we don't need the FORCE anymore. > > > From 83497e0e83d81950df54d82461b1dae629842147 Mon Sep 17 00:00:00 2001 > From: Luis Chamberlain <mcgrof@kernel.org> > Date: Wed, 27 Nov 2024 14:10:57 -0800 > Subject: [PATCH v3] selftests: kallsyms: fix double build stupidity > > Fix the stupid FORCE so that re-builds will only trigger > when really needed. While at it, document the sensible ranges > supported and fix the script to accept these alternatives. > > Also adopt Masahiro Yamada's suggested cleanups on the Makefile. > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> Did you run any compile-test before submitting this? if_changed requires FORCE. Your patch introduces new warnings. mkdir -p /home/masahiro/ref/linux/tools/objtool && make O=/home/masahiro/ref/linux subdir=tools/objtool --no-print-directory -C objtool INSTALL libsubcmd_headers CALL scripts/checksyscalls.sh lib/tests/module/Makefile:12: FORCE prerequisite is missing GEN lib/tests/module/test_kallsyms_a.c lib/tests/module/Makefile:12: FORCE prerequisite is missing lib/tests/module/Makefile:12: FORCE prerequisite is missing GEN lib/tests/module/test_kallsyms_b.c lib/tests/module/Makefile:12: FORCE prerequisite is missing GEN lib/tests/module/test_kallsyms_c.c GEN lib/tests/module/test_kallsyms_d.c CC [M] lib/tests/module/test_kallsyms_a.o CC [M] lib/tests/module/test_kallsyms_b.o CC [M] lib/tests/module/test_kallsyms_c.o CC [M] lib/tests/module/test_kallsyms_d.o -- Best Regards Masahiro Yamada
On Thu, Nov 28, 2024 at 11:56:44AM +0900, Masahiro Yamada wrote: > On Thu, Nov 28, 2024 at 11:42 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > Now with Masahiro's cleanups, in my testing we don't need the FORCE anymore. > > > > > > From 83497e0e83d81950df54d82461b1dae629842147 Mon Sep 17 00:00:00 2001 > > From: Luis Chamberlain <mcgrof@kernel.org> > > Date: Wed, 27 Nov 2024 14:10:57 -0800 > > Subject: [PATCH v3] selftests: kallsyms: fix double build stupidity > > > > Fix the stupid FORCE so that re-builds will only trigger > > when really needed. While at it, document the sensible ranges > > supported and fix the script to accept these alternatives. > > > > Also adopt Masahiro Yamada's suggested cleanups on the Makefile. > > > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > > > Did you run any compile-test before submitting this? I did. Multiple times. > if_changed requires FORCE. > Your patch introduces new warnings. > mkdir -p /home/masahiro/ref/linux/tools/objtool && make > O=/home/masahiro/ref/linux subdir=tools/objtool --no-print-directory > -C objtool > INSTALL libsubcmd_headers > CALL scripts/checksyscalls.sh > lib/tests/module/Makefile:12: FORCE prerequisite is missing > GEN lib/tests/module/test_kallsyms_a.c > lib/tests/module/Makefile:12: FORCE prerequisite is missing > lib/tests/module/Makefile:12: FORCE prerequisite is missing > GEN lib/tests/module/test_kallsyms_b.c > lib/tests/module/Makefile:12: FORCE prerequisite is missing > GEN lib/tests/module/test_kallsyms_c.c > GEN lib/tests/module/test_kallsyms_d.c > CC [M] lib/tests/module/test_kallsyms_a.o > CC [M] lib/tests/module/test_kallsyms_b.o > CC [M] lib/tests/module/test_kallsyms_c.o > CC [M] lib/tests/module/test_kallsyms_d.o Odd, I didn't see them. Anyway, I'll just take your Makefile fixes, thanks. Luis
On Wed, Nov 27, 2024 at 07:02:55PM -0800, Luis Chamberlain wrote: > I did. Multiple times. I've split this up now in 2 parts, one with your fixes and then the other boundary fixes which are not related. From 8e4c903fa3079e1c05c9585f78c57e8067024d99 Mon Sep 17 00:00:00 2001 From: Luis Chamberlain <mcgrof@kernel.org> Date: Wed, 27 Nov 2024 14:10:57 -0800 Subject: [PATCH 1/2] selftests: kallsyms: fix double build stupidity The current arrangement will have the test modules rebuilt on any make without having the script or code actually change. Take Masahiro Yamada's suggested fix and cleanups on the Makefile to fix this. Suggested-by: Masahiro Yamada <masahiroy@kernel.org> Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- lib/tests/module/Makefile | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/tests/module/Makefile b/lib/tests/module/Makefile index af5c27b996cb..2f3e1a772c2c 100644 --- a/lib/tests/module/Makefile +++ b/lib/tests/module/Makefile @@ -3,13 +3,12 @@ obj-$(CONFIG_TEST_KALLSYMS_B) += test_kallsyms_b.o obj-$(CONFIG_TEST_KALLSYMS_C) += test_kallsyms_c.o obj-$(CONFIG_TEST_KALLSYMS_D) += test_kallsyms_d.o -$(obj)/%.c: FORCE - @$(kecho) " GEN $@" - $(Q)$(srctree)/lib/tests/module/gen_test_kallsyms.sh $@\ - $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ - $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) +quiet_cmd_gen_test_kallsyms = GEN $@ + cmd_gen_test_kallsyms = $< $@ \ + $(CONFIG_TEST_KALLSYMS_NUMSYMS) \ + $(CONFIG_TEST_KALLSYMS_SCALE_FACTOR) -clean-files += test_kallsyms_a.c -clean-files += test_kallsyms_b.c -clean-files += test_kallsyms_c.c -clean-files += test_kallsyms_d.c +$(obj)/%.c: $(src)/gen_test_kallsyms.sh FORCE + $(call if_changed,gen_test_kallsyms) + +targets += $(foreach x, a b c d, test_kallsyms_$(x).c)
On Thu, Nov 28, 2024 at 12:02 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Thu, Nov 28, 2024 at 11:56:44AM +0900, Masahiro Yamada wrote: > > On Thu, Nov 28, 2024 at 11:42 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > > > Now with Masahiro's cleanups, in my testing we don't need the FORCE anymore. > > > > > > > > > From 83497e0e83d81950df54d82461b1dae629842147 Mon Sep 17 00:00:00 2001 > > > From: Luis Chamberlain <mcgrof@kernel.org> > > > Date: Wed, 27 Nov 2024 14:10:57 -0800 > > > Subject: [PATCH v3] selftests: kallsyms: fix double build stupidity > > > > > > Fix the stupid FORCE so that re-builds will only trigger > > > when really needed. While at it, document the sensible ranges > > > supported and fix the script to accept these alternatives. > > > > > > Also adopt Masahiro Yamada's suggested cleanups on the Makefile. > > > > > > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > > > > > > Did you run any compile-test before submitting this? > > I did. Multiple times. $ touch lib/tests/module/gen_test_kallsyms.sh $ make or Try full building from a clean state. > > > if_changed requires FORCE. > > Your patch introduces new warnings. > > mkdir -p /home/masahiro/ref/linux/tools/objtool && make > > O=/home/masahiro/ref/linux subdir=tools/objtool --no-print-directory > > -C objtool > > INSTALL libsubcmd_headers > > CALL scripts/checksyscalls.sh > > lib/tests/module/Makefile:12: FORCE prerequisite is missing > > GEN lib/tests/module/test_kallsyms_a.c > > lib/tests/module/Makefile:12: FORCE prerequisite is missing > > lib/tests/module/Makefile:12: FORCE prerequisite is missing > > GEN lib/tests/module/test_kallsyms_b.c > > lib/tests/module/Makefile:12: FORCE prerequisite is missing > > GEN lib/tests/module/test_kallsyms_c.c > > GEN lib/tests/module/test_kallsyms_d.c > > CC [M] lib/tests/module/test_kallsyms_a.o > > CC [M] lib/tests/module/test_kallsyms_b.o > > CC [M] lib/tests/module/test_kallsyms_c.o > > CC [M] lib/tests/module/test_kallsyms_d.o > > Odd, I didn't see them. Anyway, I'll just take your Makefile fixes, > thanks. > > Luis
On Thu, Nov 28, 2024 at 12:10 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Wed, Nov 27, 2024 at 07:02:55PM -0800, Luis Chamberlain wrote: > > I did. Multiple times. > > I've split this up now in 2 parts, one with your fixes and then the > other boundary fixes which are not related. > > From 8e4c903fa3079e1c05c9585f78c57e8067024d99 Mon Sep 17 00:00:00 2001 > From: Luis Chamberlain <mcgrof@kernel.org> > Date: Wed, 27 Nov 2024 14:10:57 -0800 > Subject: [PATCH 1/2] selftests: kallsyms: fix double build stupidity > > The current arrangement will have the test modules rebuilt on > any make without having the script or code actually change. > Take Masahiro Yamada's suggested fix and cleanups on the Makefile > to fix this. > Fixes: 84b4a51fce4c ("selftests: add new kallsyms selftests") > Suggested-by: Masahiro Yamada <masahiroy@kernel.org> > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Closes: https://lore.kernel.org/all/CAK7LNATRDODmfz1tE=inV-DQqPA4G9vKH+38zMbaGdpTuFWZFw@mail.gmail.com/T/#me6c8f98e82acbee6e75a31b34bbb543eb4940b15
The following changes since commit 8e929cb546ee42c9a61d24fae60605e9e3192354: Linux 6.12-rc3 (2024-10-13 14:33:32 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git/ tags/modules-6.13-rc1 for you to fetch changes up to 2466b31201424ccb7eda00277222302a4d6576cb: tests/module/gen_test_kallsyms.sh: use 0 value for variables (2024-11-07 14:32:55 -0800) ---------------------------------------------------------------- Modules changes for v6.13-rc1 Highlights for this merge window: * The whole caching of module code into huge pages by Mike Rapoport is going in through Andrew Morton's tree due to some other code dependencies. That's really the biggest highlight for Linux kernel modules in this release. With it we share huge pages for modules, starting off with x86. Expect to see that soon through Andrew! * Helge Deller addressed some lingering low hanging fruit alignment enhancements by. It is worth pointing out that from his old patch series I dropped his vmlinux.lds.h change at Masahiro's request as he would prefer this to be specified in asm code [0]. [0] https://lore.kernel.org/all/20240129192644.3359978-5-mcgrof@kernel.org/T/#m9efef5e700fbecd28b7afb462c15eed8ba78ef5a * Matthew Maurer and Sami Tolvanen have been tag teaming to help get us closer to a modversions for Rust. In this cycle we take in quite a lot of the refactoring for ELF validation. I expect modversions for Rust will be merged by v6.14 as that code is mostly ready now. * Adds a new modules selftests: kallsyms which helps us tests find_symbol() and the limits of kallsyms on Linux today. * We have a realtime mailing list to kernel-ci testing for modules now which relies and combines patchwork, kpd and kdevops: - https://patchwork.kernel.org/project/linux-modules/list/ - https://github.com/linux-kdevops/kdevops/blob/main/docs/kernel-ci/README.md - https://github.com/linux-kdevops/kdevops/blob/main/docs/kernel-ci/kernel-ci-kpd.md - https://github.com/linux-kdevops/kdevops/blob/main/docs/kernel-ci/linux-modules-kdevops-ci.md If you want to help avoid Linux kernel modules regressions, now its simple, just add a new Linux modules sefltests under tools/testing/selftests/module/ That is it. All new selftests will be used and leveraged automatically by the CI. ---------------------------------------------------------------- Arnd Bergmann (1): selftests: kallsyms: add MODULE_DESCRIPTION Helge Deller (2): modules: Ensure 64-bit alignment on __ksymtab_* sections modules: Add missing entry for __ex_table Luis Chamberlain (2): selftests: add new kallsyms selftests tests/module/gen_test_kallsyms.sh: use 0 value for variables Matthew Maurer (13): module: Take const arg in validate_section_offset module: Factor out elf_validity_ehdr module: Factor out elf_validity_cache_sechdrs module: Factor out elf_validity_cache_secstrings module: Factor out elf_validity_cache_index_info module: Factor out elf_validity_cache_index_mod module: Factor out elf_validity_cache_index_sym module: Factor out elf_validity_cache_index_str module: Group section index calculations together module: Factor out elf_validity_cache_strtab module: Additional validation in elf_validity_cache_strtab module: Reformat struct for code style scripts: Remove export_report.pl Makefile | 6 +- kernel/module/internal.h | 7 +- kernel/module/main.c | 569 +++++++++++++++++++------- lib/Kconfig.debug | 105 +++++ lib/Makefile | 1 + lib/tests/Makefile | 1 + lib/tests/module/.gitignore | 4 + lib/tests/module/Makefile | 15 + lib/tests/module/gen_test_kallsyms.sh | 129 ++++++ scripts/export_report.pl | 186 --------- scripts/module.lds.S | 9 +- tools/testing/selftests/module/Makefile | 12 + tools/testing/selftests/module/config | 3 + tools/testing/selftests/module/find_symbol.sh | 81 ++++ 14 files changed, 785 insertions(+), 343 deletions(-) create mode 100644 lib/tests/Makefile create mode 100644 lib/tests/module/.gitignore create mode 100644 lib/tests/module/Makefile create mode 100755 lib/tests/module/gen_test_kallsyms.sh delete mode 100755 scripts/export_report.pl create mode 100644 tools/testing/selftests/module/Makefile create mode 100644 tools/testing/selftests/module/config create mode 100755 tools/testing/selftests/module/find_symbol.sh