Received-SPF: pass (google.com: domain of linux-kernel+bounces-36662-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <b65a68eb-6b96-41ff-bbb9-38cb2dee940e@smile.fr>
Date: Wed, 24 Jan 2024 09:56:52 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] kconfig: remove unneeded symbol_empty variable
Content-Language: en-US
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>, linux-kbuild@vger.kernel.org,
 linux-kernel@vger.kernel.org,
 Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
 Vegard Nossum <vegard.nossum@oracle.com>
References: <20231125163559.824210-1-masahiroy@kernel.org>
 <CAMuHMdWm6u1wX7efZQf=2XUAHascps76YQac6rdnQGhc8nop_Q@mail.gmail.com>
 <d21298d9-fed6-4e08-9780-dbcb388b9ccc@smile.fr>
 <CAK7LNASaG4DpHTb3YHMd8d8DJ5H3z0aiUcSqX+=7CZb99kRU8A@mail.gmail.com>
From: Yoann Congal <yoann.congal@smile.fr>
Organization: Smile ECS
In-Reply-To: <CAK7LNASaG4DpHTb3YHMd8d8DJ5H3z0aiUcSqX+=7CZb99kRU8A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


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