[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNATRDODmfz1tE=inV-DQqPA4G9vKH+38zMbaGdpTuFWZFw@mail.gmail.com>
Date: Thu, 28 Nov 2024 11:44:39 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Luis Chamberlain <mcgrof@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, samitolvanen@...gle.com,
petr.pavlu@...e.com, da.gomez@...sung.com, linux-modules@...r.kernel.org,
patches@...ts.linux.dev, linux-kernel@...r.kernel.org, mmaurer@...gle.com,
arnd@...db.de, deller@....de, song@...nel.org
Subject: Re: [GIT PULL] Modules changes for v6.13-rc1
On Thu, Nov 28, 2024 at 11:22 AM Luis Chamberlain <mcgrof@...nel.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@...nel.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@...nel.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@...ux-foundation.org>
> Signed-off-by: Luis Chamberlain <mcgrof@...nel.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
Powered by blists - more mailing lists