Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <506db9a9-47ff-658a-a821-27315949e8c3@oracle.com>
Date:   Tue, 12 Apr 2022 23:56:23 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH RESEND 1/1] lib/Kconfig: remove DEBUG_PER_CPU_MAPS
 dependency for CPUMASK_OFFSTACK
Content-Language: en-US
To:     Randy Dunlap <rdunlap@infradead.org>, gregkh@linuxfoundation.org,
        masahiroy@kernel.org, tglx@linutronix.de, peterz@infradead.org,
        mingo@kernel.org, vbabka@suse.cz, akpm@linux-foundation.org
Cc:     linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org
References: <20220412231508.32629-1-libo.chen@oracle.com>
 <20220412231508.32629-2-libo.chen@oracle.com>
 <c7d26e9d-8c70-86a6-cdab-b180a365804f@infradead.org>
 <157cb46a-d134-2e72-4a65-14e378dd2b8e@oracle.com>
 <26855467-107d-4ba1-4f32-2afd5918d5b7@infradead.org>
 <cbb6b94e-3b9d-c7b6-a10e-6203a3a8b3f3@oracle.com>
 <4c6b3445-78b2-090f-c7c9-291d49c45019@infradead.org>
From:   Libo Chen <libo.chen@oracle.com>
In-Reply-To: <4c6b3445-78b2-090f-c7c9-291d49c45019@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VGROVnYxSjRpNWt0NDVSVU1VSjYvUmU0ZFIxc0NaR1J6RkVnU1lYeFNIZUlY?=
 =?utf-8?B?VWRVcndia2FSTHVmdHFrbjEydXZMV2Z6WWZjTHdwZTBOQkdKRFZCUmd3ZU1R?=
 =?utf-8?B?Y3NRM3FlWExNSjJCMWNiMnJJeEM2a0NPUXpLZ2NFSzBId283NXRER295a0ll?=
 =?utf-8?B?Szl5dkQrMzRPSFpyN0RHT1ExekNaa0pNSHdwcmRpbVJ5SDhBN3YvMXdyai81?=
 =?utf-8?B?YzZGMmdoMmF0N0JxUVNhVnBjTTUxVWFyUmdFMk9TQllFTVM3dlhKLzdKbmQ4?=
 =?utf-8?B?Mnl0ZWExaFVxWWdVa25aM2gwTDZ3dE9KWUVaaWRnWk5QZFhYV3ZqSnpPeS8x?=
 =?utf-8?B?NFdiRmw0cWFQck1VZjVuWGthT01GZW9JalVQZFZPSUtwUEY3eTVMeXRmT3Fr?=
 =?utf-8?B?bGxUZ0JWQ3RNa2dySzludGFlSzM1eHZ4K2IxbTNBSmtTemNocjExVlFvc0hF?=
 =?utf-8?B?QjV0QUk0LzFpSHgxdGlLMDQzc2xMTmViRy9YNGtQN0R3ODFxY1laRG5SR1Vs?=
 =?utf-8?B?bGZaNThscmo3WWpUcVJZTmlNUkZ5eVlHb05oOVNIU2ZIMG1GczRpd2lmM1FR?=
 =?utf-8?B?TFp4dm5UTDVOOW1qd1lPUlc2UFlhaEdBQ3dVQlRqSWdSOFJKbDI5MnhJSDRS?=
 =?utf-8?B?NHhSRXRzNFNOMHpRRCtRSG9qZCtWSFllMFBVRzhsRzQ3TDRTUTZFMWNxS25x?=
 =?utf-8?B?ck9iSElTTGxMWVd1ZzRmeUFQQzV1ditYNzFHVXlLWm1xVm81S2plc1pkUzcr?=
 =?utf-8?B?QmZkakpMdVg0R2ptNUtZSm1zSkxyVnFUVU4wM2FTTkZsMm41NTJhdEtzTkxG?=
 =?utf-8?B?YzJqRmFKZGRMWnBXSkgyTHJUbUp1M0Fzai9HTUpyZnIvaE8xQWZoVzZwZDRR?=
 =?utf-8?B?NENxc0hBUlpua1gweG13RC9lRjFqZzJLMUtka1ZvZC8xUndqSGRjaDdvS1g2?=
 =?utf-8?B?RU1HY0pvdHA0OTJTQVNTTWltUTBJQVZLZWtPbS9OTW5hSE5DY3VqaWp1Y3h2?=
 =?utf-8?B?QzRramhOOUNzY2tIUDBJblVEbE1uaCtuS2Y0RHZYV0s5WDhWQ0tzdlMzQWg0?=
 =?utf-8?B?UDl2QmUwenkzNEh5WkY2ZldpaVRkV0QvcGtMcHN4a2xGdGczdmE3Y3lKYUZu?=
 =?utf-8?B?WURHdWRCaXM0UEpCaVdFZ1RJZHUyTEVHR21zdUZISnZVQnZCdExnYU94VEFs?=
 =?utf-8?B?aGRNSDZLdFlXeTFOcGJIVnEzeTZwWlU4Wm93NEs1WlNQb0tTMndVZ3ZKb1lx?=
 =?utf-8?B?aUw1QVE2ekpLMXVSUGwxbkRoU2hMbnlZMmFPMi8vVzFicUVvZm9Gak92SlZn?=
 =?utf-8?B?UFlkR2xkSGV5WUlHb0dmZXNEMEowMVJoM1pRbm5kN1JVYmh4bUlZelYwWkZp?=
 =?utf-8?B?T3pqYThUVW52RGp2R3Fsck83a3Zhc2xBbXZIRFdvYldwVFFzVTZhcW9lYkY2?=
 =?utf-8?B?UGVYOVNuUVc5N2tJZXlSczMwcDRSQ3ZVVTNIdzJSOVZMaEozTG1yU2pxeDB2?=
 =?utf-8?B?VXZ2b0QyR2xsYm1EandqRnlsbldXN1F0WVRIOXd5RnM0Q2lGS3EzcFBXUVRn?=
 =?utf-8?B?a2FFVmJKUHg0SS9nemdzQXJnY0lRenVFMlZ4UzlwQWxORy9XdnNHcTVWZkRQ?=
 =?utf-8?B?bkF1c2s4WXE0YVd3ajR6bzlTK2NTV2dlYjh2bFhGdU1yditOY0pMaXRTa2tq?=
 =?utf-8?B?ejJ1a1ZQNnYzK082MHlYaGEwYjNxWkZza0FRR0pMZmtaWGx5RGFnVFFyOEZ0?=
 =?utf-8?B?ZGJKTnkvZ29jRlI0clVsd005TE0yUW5NVFhxYjBYTzd0bnZkYnAyaldlZmR4?=
 =?utf-8?B?Yno5MVNQanFNWHYvRGpwMzhkMXhBTUdZSTNNektmV3ZsRkhhczJDbXkzNDVt?=
 =?utf-8?B?SjcxMHVEQlMraTdQV2JTbWFLNVYwMVN5ckNhUkdnRmhjQ1NHOWFnbkZHdytS?=
 =?utf-8?B?OUR2TjdBZUo0Rk5lcklLdWlCY3ExU2hKTE1YaHBpTDYzYzN0bWxZMHMzamlq?=
 =?utf-8?B?MEJCM2RTcWNzSEJpZVBxWG1NQjVRdlFNMzU4Z1N1eDVVMHptQ3l2VnkzUms5?=
 =?utf-8?B?ZG5tLzhNVkhBSUM4aG5qa0ZJNXBkYXl5WnVoMU9qcC9zSmtrelFTQStwQlZP?=
 =?utf-8?B?WXdwSkc0N0pKeXN4Y2FKdXh0VEhIRjZjWms1SlpRWlhGR0k0UTMxMjE1TENj?=
 =?utf-8?B?bWtGWE94QXdSaHZmQy9VYUpvQ1Ewcis5cFFHUUFGZ0hFNlhOYVdNS1NKb2RN?=
 =?utf-8?B?SmtpVWF6SEk4WW0vNUhpYlZNZHpHQjUwa0hqcXB2OGFiNmFiaURmNDM5YzNF?=
 =?utf-8?B?ZlVBbXNKR09Odkt2ZHZuR3dhbEsrTy83Szd4dFpYcHlkQkNWY0dGcGt2cUNY?=
 =?utf-8?Q?s2RyHcBRapqSO3LeJlZe2hIc9biy0TYPlG2ug?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0f9137e4-3f10-434f-6733-08da1d1abbb1
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4576.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Apr 2022 06:56:28.2761
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: B2IWfwkIhD3MPp5RoLs8PtUto2oWByBypCqrZIVa6frbkH1v9Q1mQRm2Yqrhlsx0wbZGr+RzZiK5fG2g2lPEsQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4487
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-12_08:2022-04-11,2022-04-12 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0 mlxscore=0
 bulkscore=0 phishscore=0 spamscore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204130038
X-Proofpoint-ORIG-GUID: YHjxY5NXOj_B0bhn0PwiuClA4QznLon2
X-Proofpoint-GUID: YHjxY5NXOj_B0bhn0PwiuClA4QznLon2
X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@oracle.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.
> 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.
> --- 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?

Libo

>> Libo
>>> I'll test the Kconfig part of it later (in a few hours).
>>>
>>>> Libo
>>>>> Is there another problem here?
>>>>>
>>>>>> +    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