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

Yoann Congal
2024-01-24T08:56:54Z

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,
--
Yoann Congal
Smile ECS - Tech Expert