2023-11-25 16:36:26

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

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 <[email protected]>
---

scripts/kconfig/symbol.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

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);
--
2.40.1


2023-11-25 16:36:43

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 2/2] kconfig: default to zero if int/hex symbol lacks default property

When a default property is missing in an int or hex symbol, it defaults
to an empty string, which is not a valid symbol value.

It results in a incorrect .config, and can also lead to an infinite
loop in scripting.

Use "0" for int and "0x0" for hex as a default value.

Signed-off-by: Masahiro Yamada <[email protected]>
---

scripts/kconfig/symbol.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index f7075d148ac7..a5a4f9153eb7 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -338,7 +338,11 @@ void sym_calc_value(struct symbol *sym)

switch (sym->type) {
case S_INT:
+ newval.val = "0";
+ break;
case S_HEX:
+ newval.val = "0x0";
+ break;
case S_STRING:
newval.val = "";
break;
@@ -746,14 +750,17 @@ const char *sym_get_string_default(struct symbol *sym)
case yes: return "y";
}
case S_INT:
+ if (!str[0])
+ str = "0";
+ break;
case S_HEX:
- return str;
- case S_STRING:
- return str;
- case S_UNKNOWN:
+ if (!str[0])
+ str = "0x0";
+ break;
+ default:
break;
}
- return "";
+ return str;
}

const char *sym_get_string_value(struct symbol *sym)
--
2.40.1

2023-11-25 23:11:34

by Yoann Congal

[permalink] [raw]
Subject: Re: [PATCH 2/2] kconfig: default to zero if int/hex symbol lacks default property

Le 25/11/2023 à 17:35, Masahiro Yamada a écrit :
> When a default property is missing in an int or hex symbol, it defaults
> to an empty string, which is not a valid symbol value.
>
> It results in a incorrect .config, and can also lead to an infinite
> loop in scripting.
>
> Use "0" for int and "0x0" for hex as a default value.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

Thanks! It does fix our problem.

Reviewed-by: Yoann Congal <[email protected]>

> ---
>
> scripts/kconfig/symbol.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index f7075d148ac7..a5a4f9153eb7 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -338,7 +338,11 @@ void sym_calc_value(struct symbol *sym)
>
> switch (sym->type) {
> case S_INT:
> + newval.val = "0";
> + break;
> case S_HEX:
> + newval.val = "0x0";
> + break;
> case S_STRING:
> newval.val = "";
> break;
> @@ -746,14 +750,17 @@ const char *sym_get_string_default(struct symbol *sym)
> case yes: return "y";
> }
> case S_INT:
> + if (!str[0])
> + str = "0";
> + break;
> case S_HEX:
> - return str;
> - case S_STRING:
> - return str;
> - case S_UNKNOWN:
> + if (!str[0])
> + str = "0x0";
> + break;
> + default:
> break;
> }
> - return "";
> + return str;
> }
>
> const char *sym_get_string_value(struct symbol *sym)

--
Yoann Congal
Smile ECS - Tech Expert

2024-01-23 12:59:03

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

Hi Yamada-san,

On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <[email protected]> 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 <[email protected]>

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-m68korg

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

2024-01-23 15:11:23

by Yoann Congal

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

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 <[email protected]> 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 <[email protected]>
>
> 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,
--
Yoann Congal
Smile ECS - Tech Expert

2024-01-24 08:10:38

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >
> > 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
>


--
Best Regards
Masahiro Yamada

2024-01-24 08:57:23

by Yoann Congal

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable



Le 24/01/2024 à 09:09, Masahiro Yamada a écrit :
> On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>
>>> 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,
--
Yoann Congal
Smile ECS - Tech Expert

2024-01-24 09:52:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

Hi Yamada-san,

On Wed, Jan 24, 2024 at 9:10 AM Masahiro Yamada <[email protected]> wrote:
> On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <yoann.congal@smilefr> wrote:
> > Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit :
> > > On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <[email protected]> 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 <[email protected]>
> > >
> > > 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

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

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

2024-01-24 11:48:27

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

On Wed, Jan 24, 2024 at 6:52 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Yamada-san,
>
> On Wed, Jan 24, 2024 at 9:10 AM Masahiro Yamada <masahiroy@kernelorg> wrote:
> > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <[email protected]> wrote:
> > > Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit :
> > > > On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <[email protected]> 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 <[email protected]>
> > > >
> > > > 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

2024-01-24 20:13:25

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <[email protected]> wrote:
>
>
>
> Le 24/01/2024 à 09:09, Masahiro Yamada a écrit :
> > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >>>
> >>> 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



--
Best Regards
Masahiro Yamada

2024-01-25 14:45:39

by Yoann Congal

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

Hi,

Le 24/01/2024 à 21:12, Masahiro Yamada a écrit :
> On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <[email protected]> wrote:
>> Le 24/01/2024 à 09:09, Masahiro Yamada a écrit :
>>> On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <[email protected]> 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,
--
Yoann Congal
Smile ECS - Tech Expert

2024-01-25 14:58:18

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

Hi Yamada-san,

On Wed, Jan 24, 2024 at 9:13 PM Masahiro Yamada <[email protected]> wrote:
> On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <[email protected]> wrote:
> > Le 24/01/2024 à 09:09, Masahiro Yamada a écrit :
> > > On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <[email protected]> wrote:
> > >> Le 23/01/2024 à 13:54, Geert Uytterhoeven a écrit :
> > >>> On Sat, Nov 25, 2023 at 5:36 PM Masahiro Yamada <[email protected]> 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 <[email protected]>
> > >>>
> > >>> 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->currtri 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 <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

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

2024-01-26 13:38:41

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable

On Thu, Jan 25, 2024 at 11:42 PM Yoann Congal <[email protected]> wrote:
>
> Hi,
>
> Le 24/01/2024 à 21:12, Masahiro Yamada a écrit :
> > On Wed, Jan 24, 2024 at 5:56 PM Yoann Congal <[email protected]> wrote:
> >> Le 24/01/2024 à 09:09, Masahiro Yamada a écrit :
> >>> On Wed, Jan 24, 2024 at 12:11 AM Yoann Congal <[email protected]> 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.




--
Best Regards
Masahiro Yamada