| Message ID | 20231125163559.824210-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] kconfig: remove unneeded symbol_empty variable | expand |
Hi Yamada-san, On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > This is used only for initializing other variables. > > Use the empty string "". > > Please note newval.tri is unused for S_INT/HEX/STRING. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Thanks for your patch, which is now commit 4e244c10eab345a7 ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. When running "make <foo>_defconfig" with <foo>_defconfig an SMP defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, the aforementioned commit causes a change in the generated .config: -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of the integer number zero? init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 KB, 17 => 128KB)" init/Kconfig- depends on SMP init/Kconfig- range 0 21 init/Kconfig: default 12 if !BASE_SMALL init/Kconfig: default 0 if BASE_SMALL Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") does fix it. Thanks! > --- a/scripts/kconfig/symbol.c > +++ b/scripts/kconfig/symbol.c > @@ -29,12 +29,6 @@ struct symbol symbol_no = { > .flags = SYMBOL_CONST|SYMBOL_VALID, > }; > > -static struct symbol symbol_empty = { > - .name = "", > - .curr = { "", no }, > - .flags = SYMBOL_VALID, > -}; > - > struct symbol *modules_sym; > static tristate modules_val; > > @@ -346,7 +340,7 @@ void sym_calc_value(struct symbol *sym) > case S_INT: > case S_HEX: > case S_STRING: > - newval = symbol_empty.curr; > + newval.val = ""; > break; > case S_BOOLEAN: > case S_TRISTATE: > @@ -697,13 +691,12 @@ const char *sym_get_string_default(struct symbol *sym) > { > struct property *prop; > struct symbol *ds; > - const char *str; > + const char *str = ""; > tristate val; > > sym_calc_visibility(sym); > sym_calc_value(modules_sym); > val = symbol_no.curr.tri; > - str = symbol_empty.curr.val; > > /* If symbol has a default value look it up */ > prop = sym_get_default_prop(sym); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit : > Hi Yamada-san, Hello, > On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: >> This is used only for initializing other variables. >> >> Use the empty string "". >> >> Please note newval.tri is unused for S_INT/HEX/STRING. >> >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Thanks for your patch, which is now commit 4e244c10eab345a7 > ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. > > When running "make <foo>_defconfig" with <foo>_defconfig an SMP > defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, > the aforementioned commit causes a change in the generated .config: > > -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of > the integer number zero? > > init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT > init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 > KB, 17 => 128KB)" > init/Kconfig- depends on SMP > init/Kconfig- range 0 21 > init/Kconfig: default 12 if !BASE_SMALL > init/Kconfig: default 0 if BASE_SMALL > > Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. > Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if > int/hex symbol lacks default property") does fix it. (Since I'd really like 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") to stay, allow me to try to help) The problem is quite easy to reproduce: $ make x86_64_defconfig $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 CONFIG_BASE_FULL=y CONFIG_BASE_SMALL=0 Here, CONFIG_LOG_CPU_MAX_BUF_SHIFT should be 12 not 0. For what it is worth, CONFIG_BASE_SMALL is defined as an int but is only used as a bool : $ git grep BASE_SMALL arch/x86/include/asm/mpspec.h:#if CONFIG_BASE_SMALL == 0 drivers/tty/vt/vc_screen.c:#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE) include/linux/threads.h:#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000) include/linux/threads.h:#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ include/linux/udp.h:#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) include/linux/xarray.h:#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6) init/Kconfig: default 12 if !BASE_SMALL init/Kconfig: default 0 if BASE_SMALL init/Kconfig:config BASE_SMALL kernel/futex/core.c:#if CONFIG_BASE_SMALL kernel/user.c:#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7) Maybe we should change CONFIG_BASE_SMALL to the bool type? I'll poke around to see if I can understand why a int="0" is true for kconfig. Regards,
On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: > > Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit : > > Hi Yamada-san, > > Hello, > > > On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > >> This is used only for initializing other variables. > >> > >> Use the empty string "". > >> > >> Please note newval.tri is unused for S_INT/HEX/STRING. > >> > >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > Thanks for your patch, which is now commit 4e244c10eab345a7 > > ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. > > > > When running "make <foo>_defconfig" with <foo>_defconfig an SMP > > defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, > > the aforementioned commit causes a change in the generated .config: > > > > -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > > +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > > > It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of > > the integer number zero? > > > > init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT > > init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 > > KB, 17 => 128KB)" > > init/Kconfig- depends on SMP > > init/Kconfig- range 0 21 > > init/Kconfig: default 12 if !BASE_SMALL > > init/Kconfig: default 0 if BASE_SMALL > > > > Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. > > Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if > > int/hex symbol lacks default property") does fix it. > > (Since I'd really like 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") to stay, allow me to try to help) > > The problem is quite easy to reproduce: > $ make x86_64_defconfig > $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > CONFIG_BASE_FULL=y > CONFIG_BASE_SMALL=0 > Here, CONFIG_LOG_CPU_MAX_BUF_SHIFT should be 12 not 0. I could not produce it in this way. I ran the same commands as yours. CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 for me. masahiro@zoe:~/ref/linux(master)$ git describe v6.8-rc1-29-g615d30064886 masahiro@zoe:~/ref/linux(master)$ git diff masahiro@zoe:~/ref/linux(master)$ make x86_64_defconfig # # No change to .config # masahiro@zoe:~/ref/linux(master)$ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 CONFIG_BASE_FULL=y CONFIG_BASE_SMALL=0 > > For what it is worth, CONFIG_BASE_SMALL is defined as an int but is only used as a bool : > $ git grep BASE_SMALL > arch/x86/include/asm/mpspec.h:#if CONFIG_BASE_SMALL == 0 > drivers/tty/vt/vc_screen.c:#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE) > include/linux/threads.h:#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000) > include/linux/threads.h:#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ > include/linux/udp.h:#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) > include/linux/xarray.h:#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6) > init/Kconfig: default 12 if !BASE_SMALL > init/Kconfig: default 0 if BASE_SMALL > init/Kconfig:config BASE_SMALL > kernel/futex/core.c:#if CONFIG_BASE_SMALL > kernel/user.c:#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7) > > Maybe we should change CONFIG_BASE_SMALL to the bool type? > > I'll poke around to see if I can understand why a int="0" is true for kconfig. > > Regards, > -- > Yoann Congal > Smile ECS - Tech Expert >
Le 24/01/2024 à 09:09, Masahiro Yamada a écrit : > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: >> >> Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit : >>> Hi Yamada-san, >> >> Hello, >> >>> On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: >>>> This is used only for initializing other variables. >>>> >>>> Use the empty string "". >>>> >>>> Please note newval.tri is unused for S_INT/HEX/STRING. >>>> >>>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> >>> >>> Thanks for your patch, which is now commit 4e244c10eab345a7 >>> ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. >>> >>> When running "make <foo>_defconfig" with <foo>_defconfig an SMP >>> defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, >>> the aforementioned commit causes a change in the generated .config: >>> >>> -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 >>> +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 >>> >>> It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of >>> the integer number zero? >>> >>> init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT >>> init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 >>> KB, 17 => 128KB)" >>> init/Kconfig- depends on SMP >>> init/Kconfig- range 0 21 >>> init/Kconfig: default 12 if !BASE_SMALL >>> init/Kconfig: default 0 if BASE_SMALL >>> >>> Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. >>> Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if >>> int/hex symbol lacks default property") does fix it. >> >> (Since I'd really like 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") to stay, allow me to try to help) >> >> The problem is quite easy to reproduce: >> $ make x86_64_defconfig >> $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config >> CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 >> CONFIG_BASE_FULL=y >> CONFIG_BASE_SMALL=0 >> Here, CONFIG_LOG_CPU_MAX_BUF_SHIFT should be 12 not 0. > > > > I could not produce it in this way. > I ran the same commands as yours. > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 for me. > > > > masahiro@zoe:~/ref/linux(master)$ git describe > v6.8-rc1-29-g615d30064886 > masahiro@zoe:~/ref/linux(master)$ git diff > masahiro@zoe:~/ref/linux(master)$ make x86_64_defconfig > # > # No change to .config > # You already had a .config with the correct value of LOG_CPU_MAX_BUF_SHIFT (Maybe?) > masahiro@zoe:~/ref/linux(master)$ grep > 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > CONFIG_BASE_FULL=y > CONFIG_BASE_SMALL=0 Try to remove the existing .config: $ git describe v6.8-rc1 $ git diff $ rm .config -f $ make x86_64_defconfig # # configuration written to .config # $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 CONFIG_BASE_FULL=y CONFIG_BASE_SMALL=0 >> >> For what it is worth, CONFIG_BASE_SMALL is defined as an int but is only used as a bool : >> $ git grep BASE_SMALL >> arch/x86/include/asm/mpspec.h:#if CONFIG_BASE_SMALL == 0 >> drivers/tty/vt/vc_screen.c:#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE) >> include/linux/threads.h:#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000) >> include/linux/threads.h:#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ >> include/linux/udp.h:#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) >> include/linux/xarray.h:#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6) >> init/Kconfig: default 12 if !BASE_SMALL >> init/Kconfig: default 0 if BASE_SMALL >> init/Kconfig:config BASE_SMALL >> kernel/futex/core.c:#if CONFIG_BASE_SMALL >> kernel/user.c:#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7) >> >> Maybe we should change CONFIG_BASE_SMALL to the bool type? My first test shows that switching CONFIG_BASE_SMALL to bool type does fix the LOG_CPU_MAX_BUF_SHIFT default value. >> I'll poke around to see if I can understand why a int="0" is true for kconfig. Here's what I understood: To get the default value of LOG_CPU_MAX_BUF_SHIFT, kconfig calls sym_get_default_prop(LOG_CPU_MAX_BUF_SHIFT) -> expr_calc_value("BASE_SMALL" as an expr) -> sym_calc_value(BASE_SMALL as a symbol) and returns sym->curr.tri But, if I understood correctly, sym_calc_value() does not set sym->curr.tri in case of a int type config. Regards,
Hi Yamada-san, On Wed, Jan 24, 2024 at 9:10 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: > > Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit : > > > On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > >> This is used only for initializing other variables. > > >> > > >> Use the empty string "". > > >> > > >> Please note newval.tri is unused for S_INT/HEX/STRING. > > >> > > >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > Thanks for your patch, which is now commit 4e244c10eab345a7 > > > ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. > > > > > > When running "make <foo>_defconfig" with <foo>_defconfig an SMP > > > defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, > > > the aforementioned commit causes a change in the generated .config: > > > > > > -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > > > +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > > > > > It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of > > > the integer number zero? > > > > > > init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT > > > init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 > > > KB, 17 => 128KB)" > > > init/Kconfig- depends on SMP > > > init/Kconfig- range 0 21 > > > init/Kconfig: default 12 if !BASE_SMALL > > > init/Kconfig: default 0 if BASE_SMALL > > > > > > Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. > > > Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if > > > int/hex symbol lacks default property") does fix it. > > > > (Since I'd really like 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") to stay, allow me to try to help) > > > > The problem is quite easy to reproduce: > > $ make x86_64_defconfig > > $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > CONFIG_BASE_FULL=y > > CONFIG_BASE_SMALL=0 > > Here, CONFIG_LOG_CPU_MAX_BUF_SHIFT should be 12 not 0. > > > > I could not produce it in this way. > I ran the same commands as yours. > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 for me. > > > > masahiro@zoe:~/ref/linux(master)$ git describe > v6.8-rc1-29-g615d30064886 > masahiro@zoe:~/ref/linux(master)$ git diff > masahiro@zoe:~/ref/linux(master)$ make x86_64_defconfig > # > # No change to .config > # > masahiro@zoe:~/ref/linux(master)$ grep > 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > CONFIG_BASE_FULL=y > CONFIG_BASE_SMALL=0 Interesting... $ git describe v6.8-rc1-29-g615d300648869c77 $ make x86_64_defconfig [...] $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 CONFIG_BASE_FULL=y CONFIG_BASE_SMALL=0 Does it depend on the flex/bison version? I have Ubuntu LTS flex 2.6.4-8build2 and bison 2:3.8.2+dfsg-1build1. Gr{oetje,eeting}s, Geert
On Wed, Jan 24, 2024 at 6:52 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Yamada-san, > > On Wed, Jan 24, 2024 at 9:10 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: > > > Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit : > > > > On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > >> This is used only for initializing other variables. > > > >> > > > >> Use the empty string "". > > > >> > > > >> Please note newval.tri is unused for S_INT/HEX/STRING. > > > >> > > > >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > > > Thanks for your patch, which is now commit 4e244c10eab345a7 > > > > ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. > > > > > > > > When running "make <foo>_defconfig" with <foo>_defconfig an SMP > > > > defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, > > > > the aforementioned commit causes a change in the generated .config: > > > > > > > > -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > > > > +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > > > > > > > It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of > > > > the integer number zero? > > > > > > > > init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT > > > > init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 > > > > KB, 17 => 128KB)" > > > > init/Kconfig- depends on SMP > > > > init/Kconfig- range 0 21 > > > > init/Kconfig: default 12 if !BASE_SMALL > > > > init/Kconfig: default 0 if BASE_SMALL > > > > > > > > Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. > > > > Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if > > > > int/hex symbol lacks default property") does fix it. > > > > > > (Since I'd really like 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") to stay, allow me to try to help) > > > > > > The problem is quite easy to reproduce: > > > $ make x86_64_defconfig > > > $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > > CONFIG_BASE_FULL=y > > > CONFIG_BASE_SMALL=0 > > > Here, CONFIG_LOG_CPU_MAX_BUF_SHIFT should be 12 not 0. > > > > > > > > I could not produce it in this way. > > I ran the same commands as yours. > > > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 for me. > > > > > > > > masahiro@zoe:~/ref/linux(master)$ git describe > > v6.8-rc1-29-g615d30064886 > > masahiro@zoe:~/ref/linux(master)$ git diff > > masahiro@zoe:~/ref/linux(master)$ make x86_64_defconfig > > # > > # No change to .config > > # > > masahiro@zoe:~/ref/linux(master)$ grep > > 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > > CONFIG_BASE_FULL=y > > CONFIG_BASE_SMALL=0 > > Interesting... > > $ git describe > v6.8-rc1-29-g615d300648869c77 > $ make x86_64_defconfig > [...] > $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > CONFIG_BASE_FULL=y > CONFIG_BASE_SMALL=0 > > Does it depend on the flex/bison version? > I have Ubuntu LTS flex 2.6.4-8build2 and bison 2:3.8.2+dfsg-1build1. > Interesting. The result depends on the distro. I got CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 in the Ubuntu 22.04 docker container, but CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 in the Ubuntu 23.10 docker container. -- Best Regards Masahiro Yamada
On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <yoann.congal@smile.fr> wrote: > > > > Le 24/01/2024 à 09:09, Masahiro Yamada a écrit : > > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: > >> > >> Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit : > >>> Hi Yamada-san, > >> > >> Hello, > >> > >>> On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > >>>> This is used only for initializing other variables. > >>>> > >>>> Use the empty string "". > >>>> > >>>> Please note newval.tri is unused for S_INT/HEX/STRING. > >>>> > >>>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > >>> > >>> Thanks for your patch, which is now commit 4e244c10eab345a7 > >>> ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. > >>> > >>> When running "make <foo>_defconfig" with <foo>_defconfig an SMP > >>> defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, > >>> the aforementioned commit causes a change in the generated .config: > >>> > >>> -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > >>> +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > >>> > >>> It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of > >>> the integer number zero? > >>> > >>> init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT > >>> init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 > >>> KB, 17 => 128KB)" > >>> init/Kconfig- depends on SMP > >>> init/Kconfig- range 0 21 > >>> init/Kconfig: default 12 if !BASE_SMALL > >>> init/Kconfig: default 0 if BASE_SMALL > >>> > >>> Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. > >>> Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if > >>> int/hex symbol lacks default property") does fix it. > >> > >> (Since I'd really like 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") to stay, allow me to try to help) > >> > >> The problem is quite easy to reproduce: > >> $ make x86_64_defconfig > >> $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > >> CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > >> CONFIG_BASE_FULL=y > >> CONFIG_BASE_SMALL=0 > >> Here, CONFIG_LOG_CPU_MAX_BUF_SHIFT should be 12 not 0. > > > > > > > > I could not produce it in this way. > > I ran the same commands as yours. > > > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 for me. > > > > > > > > masahiro@zoe:~/ref/linux(master)$ git describe > > v6.8-rc1-29-g615d30064886 > > masahiro@zoe:~/ref/linux(master)$ git diff > > masahiro@zoe:~/ref/linux(master)$ make x86_64_defconfig > > # > > # No change to .config > > # > > You already had a .config with the correct value of LOG_CPU_MAX_BUF_SHIFT (Maybe?) > > > masahiro@zoe:~/ref/linux(master)$ grep > > 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > > CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > > CONFIG_BASE_FULL=y > > CONFIG_BASE_SMALL=0 > > Try to remove the existing .config: > > $ git describe > v6.8-rc1 > $ git diff > $ rm .config -f > $ make x86_64_defconfig > # > # configuration written to .config > # > $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > CONFIG_BASE_FULL=y > CONFIG_BASE_SMALL=0 > > >> > >> For what it is worth, CONFIG_BASE_SMALL is defined as an int but is only used as a bool : > >> $ git grep BASE_SMALL > >> arch/x86/include/asm/mpspec.h:#if CONFIG_BASE_SMALL == 0 > >> drivers/tty/vt/vc_screen.c:#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE) > >> include/linux/threads.h:#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000) > >> include/linux/threads.h:#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ > >> include/linux/udp.h:#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) > >> include/linux/xarray.h:#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6) > >> init/Kconfig: default 12 if !BASE_SMALL > >> init/Kconfig: default 0 if BASE_SMALL > >> init/Kconfig:config BASE_SMALL > >> kernel/futex/core.c:#if CONFIG_BASE_SMALL > >> kernel/user.c:#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7) > >> > >> Maybe we should change CONFIG_BASE_SMALL to the bool type? > > My first test shows that switching CONFIG_BASE_SMALL to bool type does fix the LOG_CPU_MAX_BUF_SHIFT default value. > > >> I'll poke around to see if I can understand why a int="0" is true for kconfig. > > Here's what I understood: > To get the default value of LOG_CPU_MAX_BUF_SHIFT, kconfig calls sym_get_default_prop(LOG_CPU_MAX_BUF_SHIFT) > -> expr_calc_value("BASE_SMALL" as an expr) > -> sym_calc_value(BASE_SMALL as a symbol) and returns sym->curr.tri > > But, if I understood correctly, sym_calc_value() does not set sym->curr.tri in case of a int type config. Right. The following will restore the original behavior. --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -349,12 +349,15 @@ void sym_calc_value(struct symbol *sym) switch (sym->type) { case S_INT: newval.val = "0"; + newval.tri = no; break; case S_HEX: newval.val = "0x0"; + newval.tri = no; break; case S_STRING: newval.val = ""; + newval.tri = no; break; case S_BOOLEAN: case S_TRISTATE: But, I do not think that is the right thing to do. Presumably, turning CONFIG_BASE_SMALL is correct. > > Regards, > -- > Yoann Congal > Smile ECS - Tech Expert
Hi, Le 24/01/2024 à 21:12, Masahiro Yamada a écrit : > On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <yoann.congal@smile.fr> wrote: >> Le 24/01/2024 à 09:09, Masahiro Yamada a écrit : >>> On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: >>>> For what it is worth, CONFIG_BASE_SMALL is defined as an int but is only used as a bool : >>>> $ git grep BASE_SMALL >>>> arch/x86/include/asm/mpspec.h:#if CONFIG_BASE_SMALL == 0 >>>> drivers/tty/vt/vc_screen.c:#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE) >>>> include/linux/threads.h:#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000) >>>> include/linux/threads.h:#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ >>>> include/linux/udp.h:#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) >>>> include/linux/xarray.h:#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6) >>>> init/Kconfig: default 12 if !BASE_SMALL >>>> init/Kconfig: default 0 if BASE_SMALL >>>> init/Kconfig:config BASE_SMALL >>>> kernel/futex/core.c:#if CONFIG_BASE_SMALL >>>> kernel/user.c:#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7) >>>> >>>> Maybe we should change CONFIG_BASE_SMALL to the bool type? >> >> My first test shows that switching CONFIG_BASE_SMALL to bool type does fix the LOG_CPU_MAX_BUF_SHIFT default value. >> >>>> I'll poke around to see if I can understand why a int="0" is true for kconfig. >> >> Here's what I understood: >> To get the default value of LOG_CPU_MAX_BUF_SHIFT, kconfig calls sym_get_default_prop(LOG_CPU_MAX_BUF_SHIFT) >> -> expr_calc_value("BASE_SMALL" as an expr) >> -> sym_calc_value(BASE_SMALL as a symbol) and returns sym->curr.tri >> >> But, if I understood correctly, sym_calc_value() does not set sym->curr.tri in case of a int type config. > > Right. Thanks :) > The following will restore the original behavior. > > > --- a/scripts/kconfig/symbol.c > +++ b/scripts/kconfig/symbol.c > @@ -349,12 +349,15 @@ void sym_calc_value(struct symbol *sym) > switch (sym->type) { > case S_INT: > newval.val = "0"; > + newval.tri = no; > break; > case S_HEX: > newval.val = "0x0"; > + newval.tri = no; > break; > case S_STRING: > newval.val = ""; > + newval.tri = no; > break; > case S_BOOLEAN: > case S_TRISTATE: > > > But, I do not think that is the right thing to do. > > Presumably, turning CONFIG_BASE_SMALL is correct. I'm working on a patch to do that. Regards,
Hi Yamada-san, On Wed, Jan 24, 2024 at 9:13 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <yoann.congal@smile.fr> wrote: > > Le 24/01/2024 à 09:09, Masahiro Yamada a écrit : > > > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: > > >> Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit : > > >>> On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > >>>> This is used only for initializing other variables. > > >>>> > > >>>> Use the empty string "". > > >>>> > > >>>> Please note newval.tri is unused for S_INT/HEX/STRING. > > >>>> > > >>>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > >>> > > >>> Thanks for your patch, which is now commit 4e244c10eab345a7 > > >>> ("kconfig: remove unneeded symbol_empty variable") in v6.8-rc1. > > >>> > > >>> When running "make <foo>_defconfig" with <foo>_defconfig an SMP > > >>> defconfig without explicit configuration of CONFIG_LOG_CPU_MAX_BUF_SHIFT, > > >>> the aforementioned commit causes a change in the generated .config: > > >>> > > >>> -CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 > > >>> +CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > >>> > > >>> It looks like CONFIG_BASE_SMALL=0 is treated as a string instead of > > >>> the integer number zero? > > >>> > > >>> init/Kconfig=config LOG_CPU_MAX_BUF_SHIFT > > >>> init/Kconfig- int "CPU kernel log buffer size contribution (13 => 8 > > >>> KB, 17 => 128KB)" > > >>> init/Kconfig- depends on SMP > > >>> init/Kconfig- range 0 21 > > >>> init/Kconfig: default 12 if !BASE_SMALL > > >>> init/Kconfig: default 0 if BASE_SMALL > > >>> > > >>> Note that reverting 4e244c10eab345a7 is not sufficient to fix the issue. > > >>> Also reverting commit 6262afa10ef7cc8f ("kconfig: default to zero if > > >>> int/hex symbol lacks default property") does fix it. > > >> > > >> (Since I'd really like 6262afa10ef7cc8f ("kconfig: default to zero if int/hex symbol lacks default property") to stay, allow me to try to help) > > >> > > >> The problem is quite easy to reproduce: > > >> $ make x86_64_defconfig > > >> $ grep 'LOG_CPU_MAX_BUF_SHIFT\|BASE_SMALL\|BASE_FULL' .config > > >> CONFIG_LOG_CPU_MAX_BUF_SHIFT=0 > > >> CONFIG_BASE_FULL=y > > >> CONFIG_BASE_SMALL=0 > > >> Here, CONFIG_LOG_CPU_MAX_BUF_SHIFT should be 12 not 0. > > >> For what it is worth, CONFIG_BASE_SMALL is defined as an int but is only used as a bool : > > >> $ git grep BASE_SMALL > > >> arch/x86/include/asm/mpspec.h:#if CONFIG_BASE_SMALL == 0 > > >> drivers/tty/vt/vc_screen.c:#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE) > > >> include/linux/threads.h:#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000) > > >> include/linux/threads.h:#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ > > >> include/linux/udp.h:#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) > > >> include/linux/xarray.h:#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6) > > >> init/Kconfig: default 12 if !BASE_SMALL > > >> init/Kconfig: default 0 if BASE_SMALL > > >> init/Kconfig:config BASE_SMALL > > >> kernel/futex/core.c:#if CONFIG_BASE_SMALL > > >> kernel/user.c:#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7) > > >> > > >> Maybe we should change CONFIG_BASE_SMALL to the bool type? > > > > My first test shows that switching CONFIG_BASE_SMALL to bool type does fix the LOG_CPU_MAX_BUF_SHIFT default value. > > > > >> I'll poke around to see if I can understand why a int="0" is true for kconfig. > > > > Here's what I understood: > > To get the default value of LOG_CPU_MAX_BUF_SHIFT, kconfig calls sym_get_default_prop(LOG_CPU_MAX_BUF_SHIFT) > > -> expr_calc_value("BASE_SMALL" as an expr) > > -> sym_calc_value(BASE_SMALL as a symbol) and returns sym->curr.tri > > > > But, if I understood correctly, sym_calc_value() does not set sym->curr.tri in case of a int type config. > > Right. > > The following will restore the original behavior. > > --- a/scripts/kconfig/symbol.c > +++ b/scripts/kconfig/symbol.c > @@ -349,12 +349,15 @@ void sym_calc_value(struct symbol *sym) > switch (sym->type) { > case S_INT: > newval.val = "0"; > + newval.tri = no; > break; > case S_HEX: > newval.val = "0x0"; > + newval.tri = no; > break; > case S_STRING: > newval.val = ""; > + newval.tri = no; > break; > case S_BOOLEAN: > case S_TRISTATE: Thank you, that works for me. Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Thu, Jan 25, 2024 at 11:42 PM Yoann Congal <yoann.congal@smile.fr> wrote: > > Hi, > > Le 24/01/2024 à 21:12, Masahiro Yamada a écrit : > > On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <yoann.congal@smile.fr> wrote: > >> Le 24/01/2024 à 09:09, Masahiro Yamada a écrit : > >>> On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smile.fr> wrote: > >>>> For what it is worth, CONFIG_BASE_SMALL is defined as an int but is only used as a bool : > >>>> $ git grep BASE_SMALL > >>>> arch/x86/include/asm/mpspec.h:#if CONFIG_BASE_SMALL == 0 > >>>> drivers/tty/vt/vc_screen.c:#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE) > >>>> include/linux/threads.h:#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000) > >>>> include/linux/threads.h:#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \ > >>>> include/linux/udp.h:#define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) > >>>> include/linux/xarray.h:#define XA_CHUNK_SHIFT (CONFIG_BASE_SMALL ? 4 : 6) > >>>> init/Kconfig: default 12 if !BASE_SMALL > >>>> init/Kconfig: default 0 if BASE_SMALL > >>>> init/Kconfig:config BASE_SMALL > >>>> kernel/futex/core.c:#if CONFIG_BASE_SMALL > >>>> kernel/user.c:#define UIDHASH_BITS (CONFIG_BASE_SMALL ? 3 : 7) > >>>> > >>>> Maybe we should change CONFIG_BASE_SMALL to the bool type? > >> > >> My first test shows that switching CONFIG_BASE_SMALL to bool type does fix the LOG_CPU_MAX_BUF_SHIFT default value. > >> > >>>> I'll poke around to see if I can understand why a int="0" is true for kconfig. > >> > >> Here's what I understood: > >> To get the default value of LOG_CPU_MAX_BUF_SHIFT, kconfig calls sym_get_default_prop(LOG_CPU_MAX_BUF_SHIFT) > >> -> expr_calc_value("BASE_SMALL" as an expr) > >> -> sym_calc_value(BASE_SMALL as a symbol) and returns sym->curr.tri > >> > >> But, if I understood correctly, sym_calc_value() does not set sym->curr.tri in case of a int type config. > > > > Right. > > Thanks :) > > > The following will restore the original behavior. > > > > > > --- a/scripts/kconfig/symbol.c > > +++ b/scripts/kconfig/symbol.c > > @@ -349,12 +349,15 @@ void sym_calc_value(struct symbol *sym) > > switch (sym->type) { > > case S_INT: > > newval.val = "0"; > > + newval.tri = no; > > break; > > case S_HEX: > > newval.val = "0x0"; > > + newval.tri = no; > > break; > > case S_STRING: > > newval.val = ""; > > + newval.tri = no; > > break; > > case S_BOOLEAN: > > case S_TRISTATE: > > > > > But, I do not think that is the right thing to do. > > > > Presumably, turning CONFIG_BASE_SMALL is correct. > > I'm working on a patch to do that. > OK, please go ahead. I will restore the Kconfig original behavior for now and send a pull-req.
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index a76925b46ce6..f7075d148ac7 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -29,12 +29,6 @@ struct symbol symbol_no = { .flags = SYMBOL_CONST|SYMBOL_VALID, }; -static struct symbol symbol_empty = { - .name = "", - .curr = { "", no }, - .flags = SYMBOL_VALID, -}; - struct symbol *modules_sym; static tristate modules_val; @@ -346,7 +340,7 @@ void sym_calc_value(struct symbol *sym) case S_INT: case S_HEX: case S_STRING: - newval = symbol_empty.curr; + newval.val = ""; break; case S_BOOLEAN: case S_TRISTATE: @@ -697,13 +691,12 @@ const char *sym_get_string_default(struct symbol *sym) { struct property *prop; struct symbol *ds; - const char *str; + const char *str = ""; tristate val; sym_calc_visibility(sym); sym_calc_value(modules_sym); val = symbol_no.curr.tri; - str = symbol_empty.curr.val; /* If symbol has a default value look it up */ prop = sym_get_default_prop(sym);
This is used only for initializing other variables. Use the empty string "". Please note newval.tri is unused for S_INT/HEX/STRING. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/kconfig/symbol.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)