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: <d0c59eb8-baca-26fd-5f10-4a1f89bcf7b9@nvidia.com>
Date:   Thu, 22 Sep 2022 19:05:54 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH v1 3/3] checkpatch: warn on usage of VM_BUG_ON() and other
 BUG variants
Content-Language: en-US
To:     David Hildenbrand <david@redhat.com>, linux-kernel@vger.kernel.org
Cc:     linux-mm@kvack.org, linux-doc@vger.kernel.org,
        linuxppc-dev@lists.ozlabs.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Ingo Molnar <mingo@kernel.org>,
        David Laight <David.Laight@ACULAB.COM>,
        Jonathan Corbet <corbet@lwn.net>,
        Andy Whitcroft <apw@canonical.com>,
        Joe Perches <joe@perches.com>,
        Dwaipayan Ray <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>,
        Baoquan He <bhe@redhat.com>, Vivek Goyal <vgoyal@redhat.com>,
        Dave Young <dyoung@redhat.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Nicholas Piggin <npiggin@gmail.com>,
        Christophe Leroy <christophe.leroy@csgroup.eu>
References: <20220920122302.99195-1-david@redhat.com>
 <20220920122302.99195-4-david@redhat.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20220920122302.99195-4-david@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bVdUclBiaytLMW9OOXpmeXJkNjlRZ2FDZE1Uc1VPNVlNTFVsSXJwQVg0elBm?=
 =?utf-8?B?SDhHZjdub2dUNHVENnFTbm5yUDBPeTZ2WDE2OTNlMHpUeFFsYkJhOXJMTlJT?=
 =?utf-8?B?SWRKa1B5TG42Y1pFZXlncXA1a2wySy9QTlN5L0lBc25NbXVZMUNKa0gvUGlC?=
 =?utf-8?B?Q01kM3FVT1NnbjZmR0pETWpwOGhqMWpqVFBvM0VoRHM4WmlUYzFtZHNLN2xu?=
 =?utf-8?B?aG53K1Z4c283VGFxOEJYeVFlUFk5T3p5elFOUGRaM3YvMGxlSVpwZ0M5bW1Q?=
 =?utf-8?B?UWF4Q1cyRXJ6MndtK2c5QUM0NStib0pBODNFTWJKeXFzUkdJdDlmMks2czJn?=
 =?utf-8?B?SmVEYTVxQUdDT2pWSEdhVCtYWFhZRGNodHFOd0pKQ3c1aEZJaGJKU0ZQMkxO?=
 =?utf-8?B?Zys3ZDlGbmR4VGVyUC81cnRqMExlNER0VEJ0R2NvNGt1aDk3YklDSmZzVVg0?=
 =?utf-8?B?RWN6SExrOXRGcVdpNlVVRWQ0cnhvSDYwSXg3YTZoaGhtQlI3NEFMNTFod1V5?=
 =?utf-8?B?VENSRnFoRThjY0xIalRxTUprZld6eG0rZllncTBvT2QyeXdUVndQMkJhQTNR?=
 =?utf-8?B?WnhMQVRJTExROTBPcmp5clBYOFZhTHZraGM0WlZXSmJVUnFQc3gxcVRSMVM0?=
 =?utf-8?B?dW5TdlVaRHYrbGpXcnNqWFp1SEtNV3Zndk1weklEcDFjcW10VHdWTzArRXRW?=
 =?utf-8?B?NHhpMStQUnVOS1dqR2E1YWtLcU1UaGlRNW1TZno5Ry9mbnpDM2tabElUUXVi?=
 =?utf-8?B?b1ovRGc5L3V0OCthanpBN3Mwd2ZYYVpVNHRIa3MxSFA0dGdITm0wL3MzNmll?=
 =?utf-8?B?NnBHNmxMMmRYTlNoTUFrRE9qV2VMZHpFazBiUjFwclgyMXc3bURGM1U3Y2My?=
 =?utf-8?B?Nm9hOTdlQndlaTlWWmNEUklQWEd4dzRCbXk5UzgvL3JCbHBTenFrVnJIVGpk?=
 =?utf-8?B?Y0d1dkFzdUl2bmRLRENGaTc3Y2I1YVlXaGVFaDErYjZ3TlNXYnRzYW1qZ0lM?=
 =?utf-8?B?dW1paVdkMkJSb3VwV3oxMTJsSjUvM1hEZE1EY0lkbU9yMWFQMFNSRFowb3Q3?=
 =?utf-8?B?d0l3OTRLME1LcTdyOHgrN0g1MEV4S0lDcXd6eEg1OUUwMTdFQ250YVZ0ZjFG?=
 =?utf-8?B?MnpFL1NBSU5SV25nUUhPUUpPT1oxNS9IMTNIcnZvcnZ0NE5rSS9ETVJvWXFE?=
 =?utf-8?B?TG5jUnlxVEdqK2ZpajloYmczWXh4YTR3Ulh3SFJIZUVkUjNnZ3pCbTdiOEpw?=
 =?utf-8?B?R2ZrSEwwOEh0K2UxWDFNZm1XcW1rNVM2M085UnhCNk9OUkgreXkxUUYwSEpQ?=
 =?utf-8?B?dnpSK1FJUVRJNjlzc0h2a1l5cWh1TmVHQmFSMTBYbkdWVUpSa0NmaWY4TTd0?=
 =?utf-8?B?SWZUVVF5Szh2VXUvcUFOU3lmMzRVYWtlVlhGMHNYOTVDeW00ZGRCNis4SnAz?=
 =?utf-8?B?Nm9aaGhJcVh2eTQ3UFB2UFlKTWxSMlRLOHo2T2tvR2gyQytjMk5kUXVOeS9u?=
 =?utf-8?B?b2kxY1JXN1lSNEt1ZnhVNXUzYjdtTVh5VXNaMnM0WUVucGp0TXBWOXBLSDFP?=
 =?utf-8?B?RVZGM0JwWjlKV1MvZVJqc0dkT0lXUlgrckk5cC9FR3RBVFNablUrSW1uTFB0?=
 =?utf-8?B?eVRqSnlGYW1TNFJOMGFnODJZQWh1dXhCRGZjMldQbndoRXB1SXJ1VDJTUVFO?=
 =?utf-8?B?QUNBU0I4MUtveDVnQWxIK2srMW5HYnRhTytGbHY1Z1BsaUJ5VmR2N0xaWmd4?=
 =?utf-8?B?ZGRDZHhKZDI2SFdQMjNHQ1c3VldlSEF5Q0VDellQNDk5SVRaeU9VMmxCdkIx?=
 =?utf-8?B?cHhWNGQzekhlU3RPanFZeHRueWhLSFVtczRKeUlQZ0tmeDBCSjJ6RmFZRHA1?=
 =?utf-8?B?QXNPdTZQb0xmUk10V1ROZGVOV2lQNFN5U0NteVVCdnQydUVrTmxwa2RYRVMx?=
 =?utf-8?B?QjNlRjAyVXQ1Tjhtd1RWc1J3OTJrdS9WYzI1S29iR2JGWmJycExIaUxlcXov?=
 =?utf-8?B?ZnB3WFBESU1JSUwyWmJ5b3lkcVJRR1pMNy9MSndjN09aUVdONm0vTHVTTWIz?=
 =?utf-8?B?NVBnU1NDWGFPS2kyWTVMcEtaN1VsT1F0MEZEOFp1eEVwSkhLbDNPNVArZmV3?=
 =?utf-8?Q?rd+0TaP8hN/lJ4qFBclYpb5GO?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 271e93ad-9713-47b8-75f2-08da9d082846
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB4140.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Sep 2022 02:05:59.2363
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: hd+k5PYSAZczENF704qH+V0VZcJDYABccKYxDp7B97yLDjnrEkwEjCn5n6+0RMLZL8EMQT273QVJ9xPEoAWg3g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB6612
X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE 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

On 9/20/22 05:23, David Hildenbrand wrote:
> checkpatch does not point out that VM_BUG_ON() and friends should be
> avoided, however, Linus notes:
> 
>     VM_BUG_ON() has the exact same semantics as BUG_ON. It is literally
>     no different, the only difference is "we can make the code smaller
>     because these are less important". [1]
> 
> So let's warn on VM_BUG_ON() and other BUG variants as well. While at it,
> make it clearer that the kernel really shouldn't be crashed.
> 
> As there are some subsystem BUG macros that actually don't end up crashing
> the kernel -- for example, KVM_BUG_ON() -- exclude these manually.
> 
> [1] https://lore.kernel.org/r/CAHk-=wg40EAZofO16Eviaj7mfqDhZ2gVEbvfsMf6gYzspRjYvw@mail.gmail.com
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  scripts/checkpatch.pl | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 79e759aac543..21f3a79aa46f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -4695,12 +4695,12 @@ sub process {
>  			}
>  		}
>  
> -# avoid BUG() or BUG_ON()
> -		if ($line =~ /\b(?:BUG|BUG_ON)\b/) {
> +# do not use BUG() or variants
> +		if ($line =~ /\b(?!AA_|BUILD_|DCCP_|IDA_|KVM_|RWLOCK_|snd_|SPIN_)(?:[a-zA-Z_]*_)?BUG(?:_ON)?(?:_[A-Z_]+)?\s*\(/) {

Should this be a separate patch? Adding a bunch of exceptions to the BUG() rules is 
a separate and distinct thing from adding VM_BUG_ON() and other *BUG*() variants to
the mix.

>  			my $msg_level = \&WARN;
>  			$msg_level = \&CHK if ($file);
>  			&{$msg_level}("AVOID_BUG",
> -				      "Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr);
> +				      "Do not crash the kernel unless it is unavoidable - use WARN_ON_ONCE & recovery code (if reasonable) rather than BUG() or variants.\n" . $herecurr);

Here's a requested tweak, to clean up the output and fix punctuation:

"Avoid crashing the kernel--use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants.\n" . $herecurr);


thanks,

-- 
John Hubbard
NVIDIA