| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c2e6bb8b-c9d9-ad39-7a8e-3df6849b2fb2@oracle.com>
Date: Wed, 13 Apr 2022 12:28:38 -0700
From: Libo Chen <libo.chen@...cle.com>
To: Randy Dunlap <rdunlap@...radead.org>, gregkh@...uxfoundation.org,
masahiroy@...nel.org, tglx@...utronix.de, peterz@...radead.org,
mingo@...nel.org, vbabka@...e.cz, akpm@...ux-foundation.org
Cc: linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH RESEND 1/1] lib/Kconfig: remove DEBUG_PER_CPU_MAPS
dependency for CPUMASK_OFFSTACK
On 4/13/22 08:41, Randy Dunlap wrote:
>
> On 4/12/22 23:56, Libo Chen wrote:
>> Hi Randy
>>
>> On 4/12/22 22:54, Randy Dunlap wrote:
>>> Hi Libo,
>>>
>>> On 4/12/22 19:34, Libo Chen wrote:
>>>> On 4/12/22 19:13, Randy Dunlap wrote:
>>>>> Hi,
>>>>>
>>>>> On 4/12/22 18:35, Libo Chen wrote:
>>>>>> Hi Randy,
>>>>>>
>>>>>> On 4/12/22 17:18, Randy Dunlap wrote:
>>>>>>> Hi--
>>>>>>>
>>>>>>> On 4/12/22 16:15, Libo Chen wrote:
>>>>>>>> Forcing CPUMASK_OFFSTACK to be conditoned on DEBUG_PER_CPU_MAPS doesn't
>>>>>>>> make a lot of sense nowaday. Even the original patch dating back to 2008,
>>>>>>>> aab46da0520a ("cpumask: Add CONFIG_CPUMASK_OFFSTACK") didn't give any
>>>>>>>> rationale for such dependency.
>>>>>>>>
>>>>>>>> Nowhere in the code supports the presumption that DEBUG_PER_CPU_MAPS is
>>>>>>>> necessary for CONFIG_CPUMASK_OFFSTACK. Make no mistake, it's good to
>>>>>>>> have DEBUG_PER_CPU_MAPS for debugging purpose or precaution, but it's
>>>>>>>> simply not a hard requirement for CPUMASK_OFFSTACK. Moreover, x86 Kconfig
>>>>>>>> already can set CPUMASK_OFFSTACK=y without DEBUG_PER_CPU_MAPS=y.
>>>>>>>> There is no reason other architectures cannot given the fact that they
>>>>>>>> have even fewer, if any, arch-specific CONFIG_DEBUG_PER_CPU_MAPS code than
>>>>>>>> x86.
>>>>>>>>
>>>>>>>> Signed-off-by: Libo Chen <libo.chen@...cle.com>
>>>>>>>> ---
>>>>>>>> lib/Kconfig | 2 +-
>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>>>>>>> index 087e06b4cdfd..7209039dfb59 100644
>>>>>>>> --- a/lib/Kconfig
>>>>>>>> +++ b/lib/Kconfig
>>>>>>>> @@ -511,7 +511,7 @@ config CHECK_SIGNATURE
>>>>>>>> bool
>>>>>>>> config CPUMASK_OFFSTACK
>>>>>>>> - bool "Force CPU masks off stack" if DEBUG_PER_CPU_MAPS
>>>>>>> This "if" dependency only controls whether the Kconfig symbol's prompt is
>>>>>>> displayed (presented) in kconfig tools. Removing it makes the prompt always
>>>>>>> be displayed.
>>>>>>>
>>>>>>> Any architecture could select (should be able to) CPUMASK_OFFSTACK independently
>>>>>>> of DEBUG_PER_CPU_MAPS.
>>>>>> Do you mean changing arch/xxxx/Kconfig to select CPUMASK_OFFSTACK under some config xxx? That will work but it requires code changes for each architecture.
>>>>>> But if you are talking about setting CONFIG_CPUMASK_OFFSTACK=y without CONFIG_DEBUG_PER_CPU_MAPS directly in config file, I have tried, it doesn't work.
>>>>> I'm just talking about the Kconfig change below. Not talking about whatever else
>>>>> it might require per architecture.
>>>>>
>>>>> But you say you have tried that and it doesn't work. What part of it doesn't work?
>>>>> The Kconfig part or some code execution?
>>>> oh the Kconfig part. For example, make olddefconfig on a config file with CPUMASK_OFFSTACK=y only turns off CPUMASK_OFFSTACK unless I explicitly set DEBUG_PER_CPU_MAPS=y
>>> I can enable CPUMASK_OFFSTACK for arm64 without having DEBUG_PER_CPU_MAPS enabled.
>>> (with a patch, of course.)
>>> It builds OK. I don't know if it will run OK.
>> I am a little confused, did you succeed with your patch (replacing "if" with "depends on") or my patch (removing "if")? Because I definitely cannot enable CPUMASK_OFFSTACK for arm64 without DEBUG_PER_CPUMAPS enabled using your change.
> This patch builds cleanly for me:
>
> ---
> arch/arm64/Kconfig | 1 +
> lib/Kconfig | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -511,7 +511,7 @@ config CHECK_SIGNATURE
> bool
>
> config CPUMASK_OFFSTACK
> - bool "Force CPU masks off stack" if DEBUG_PER_CPU_MAPS
> + bool "Force CPU masks off stack"
> help
> Use dynamic allocation for cpumask_var_t, instead of putting
> them on the stack. This is a bit more expensive, but avoids
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -316,6 +316,7 @@ config ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
>
> config SMP
> def_bool y
> + select CPUMASK_OFFSTACK
>
> config KERNEL_MODE_NEON
> def_bool y
>
> along with:
> # CONFIG_DEBUG_PER_CPU_MAPS is not set
>
>
>>> I think that you are arguing for a patch like this:
>> I am actually arguing for the opposite, I don't think CPUMASK_OFFSTACK should require DEBUG_PER_CPU_MAPS. They should be separate and independent to each other. So removing "if ..." should be enough in my opinion.
> I agree.
>
>>> --- a/lib/Kconfig
>>> +++ b/lib/Kconfig
>>> @@ -511,7 +511,8 @@ config CHECK_SIGNATURE
>>> bool
>>> config CPUMASK_OFFSTACK
>>> - bool "Force CPU masks off stack" if DEBUG_PER_CPU_MAPS
>>> + bool "Force CPU masks off stack"
>>> + depends on DEBUG_PER_CPU_MAPS
>> This forces every arch to enable DEBUG_PER_CPU_MAPS if they want to enable CPUMASK_OFFSTACK, it's even stronger than "if". My whole argument is CPUMASK_OFFSTACK should be enable/disabled independent of DEBUG_PER_CPU_MASK
>>> help
>>> Use dynamic allocation for cpumask_var_t, instead of putting
>>> them on the stack. This is a bit more expensive, but avoids
>>>
>>>
>>> As I said earlier, the "if" on the "bool" line just controls the prompt message.
>>> This patch make CPUMASK_OFFSTACK require DEBUG_PER_CPU_MAPS -- which might be overkill.
>>>
>> Okay I understand now "if" on the "boot" is not a dependency and it only controls the prompt message, then the question is why we cannot enable CPUMASK_OFFSTACK without DEBUG_PER_CPU_MAPS if it only controls prompt message? Is it not the behavior we expect?
> Yes, it is. I don't know that the problem is...
Masahiro explained that CPUMASK_OFFSTACK can only be configured by
options not users if DEBUG_PER_CPU_MASK is not enabled. This doesn't
seem to be what we want.
Libo
Powered by blists - more mailing lists