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: <60fa5b16-021e-1e54-7e06-39bbb871eaef@nvidia.com>
Date:   Thu, 22 Sep 2022 19:20:56 -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:     Joe Perches <joe@perches.com>,
        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>,
        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>
 <d0c59eb8-baca-26fd-5f10-4a1f89bcf7b9@nvidia.com>
 <3179f4545d3f71358da3e6c6ee18085af601b2eb.camel@perches.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <3179f4545d3f71358da3e6c6ee18085af601b2eb.camel@perches.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?RnNPaGtGSjhjQ1d3VGlhNjVkVVRsQU96Rm4rc295eTJ4dFRyOUZYRlExTjBi?=
 =?utf-8?B?T2hNQlRObXVQZFhtNlBFckVXQXVFNDR1TURld010Wm96YU1ad1lEa2tjRjds?=
 =?utf-8?B?ODNObHM3NlJXZEt4K1JZeVFYa3FCbXZKcmZGK2J0K0lZSk9PWCsvYXo1UDlB?=
 =?utf-8?B?UEpYeDJadmExaElvK1NTUlFzTnlyWEM0d3RWT0pkTVIvd1dRWDlncGdmcjBr?=
 =?utf-8?B?UkIrd0Npbkxxc1NtVHlmM3N5aEJ5VUFzdmZHVkNEWmZzTEJVVStSdHYyOE92?=
 =?utf-8?B?cGJ6VzAyWFRpRjRzSjNCeGtndGdQNkR0NUZsbmdzVE5ZNlB1SG1DMGl2c0VP?=
 =?utf-8?B?bUpEbkE3TkNMZzUrZU0vVWFDMHk4dk9OL3hYaUxmdEY1d0UrdVBzM2svRDJB?=
 =?utf-8?B?RG92UlpyS3ArcFJMRXlydzJjS2g3K3lmQXR3alZzamJ3UVNtcW9rblJDVzlT?=
 =?utf-8?B?OGQ2UDVGUmxuUlJJVkNxY1ZtQW84dGdacTJuS2VRd2FXSkNEYzRQZlFqc3Z0?=
 =?utf-8?B?emZTc1laSU8vMU1HNHUySDFjRnNoOTdnUDhTWmVFL0NOZEI2U1hpU0N4UUZ4?=
 =?utf-8?B?UTBuSk9aeit6Q3MwYUtPbEVQSlIzR24wMkVIazlvcTJwUUNNUG1XcmJJS3lL?=
 =?utf-8?B?VVlKeC9OaUI1UDFBbkhhTDdQeTVwckl6YWxjUUVZQXJObHZ6VGdpdk9OUGt2?=
 =?utf-8?B?T3M0UkY3MWJwaENPaUVOd1RaODdxTm9TWXczV2JPby9aUFBPbmlWcGwrT1Qr?=
 =?utf-8?B?dUZveDV4ZkVYNStmWWVIWUNzMnhTZkxqTE1KSVYrVUEvRDgxaUh6NldmamdC?=
 =?utf-8?B?dlphSmlhc056NGpsUXM0OVNSVWFvdUlNaUxIckFPNHRUQmRCQkxsT0dDMzRx?=
 =?utf-8?B?WS9lSjJmT3JoNlZkcFpEcDgralFkakt2TjducTdsbUF3dVlSdzBhdlcvbG9o?=
 =?utf-8?B?SkZnSHlXYTYwakR6WG5VUlBFaG41UFRhTjFTTk9DNFFJWi95TTRjTjV3M1Bp?=
 =?utf-8?B?NWhja0U2SUJvMkduc3BFRW5LcWFCTW94aExFb21vRHJvRFFETXZsR3B2c2ds?=
 =?utf-8?B?M3ZqcXFTMlNYR3V4cDRvSVRxK2JGTzd2blE1eWFqUlVaTkxEd3FxY0NYalRv?=
 =?utf-8?B?YWRiKzE0MCtSTWZuWmxlTUxZeWc3QWJGQ09EL0FVZE5yZDY2V2xjc0trelA0?=
 =?utf-8?B?QnowRHVwYk45cm5MVHhoZGxDSHhIWnAxNUZYZ2RQVGVJMTVkb3QxUzB0ckRq?=
 =?utf-8?B?UG1xWEpvVWNwcVFmSUlEM2FaSzNjYVpvU2l4NlltTGFtN3h5a2s3bS9oNGZV?=
 =?utf-8?B?ZXZCVVJ4a1BzSWVpQWJudkFlWDhGOTlxQnRHZWczMEhreVFpcjVpR29CVlBP?=
 =?utf-8?B?Q2ZsQ0lJZEtzMDRuZWVLRlhBVzBOcG9hUDZleDJYS1plclRzOGpGYjFHaCty?=
 =?utf-8?B?UVpWdzZrdDR5NmpNcEVjSHJIVVN1dzNmbk13OVFtb2d3MWFRbTAvQXk1VVZ4?=
 =?utf-8?B?S25sS1FXVUczNDdRbjg4cnN4M1FtbU4wYUFUeXFqVEdwczEzYVdmb0NvRTBX?=
 =?utf-8?B?dGtyUXp3QWx4amJQai9sdjhBcHJyNFU0bDNvWXNjczB3Ryt3YW1TbjRhL21M?=
 =?utf-8?B?Z1pkSkZKL00vTW9hZUdidUZ0bG1CU1Zoa0lERE05UDR1eVByUi9GUzFwRlpT?=
 =?utf-8?B?eDZZV21pUTBSOHlmaTlqc0tIY3h4ajVsN2VvWXo3ZExzbW54YjYwV2QrZ1Vh?=
 =?utf-8?B?cEZoT3E4MExNZXAwTjh1eEU2SFliTy9kWUpFb2NXZ25mLzhUSTJyTFdCQWpq?=
 =?utf-8?B?SzlBaytUUWx1RHNXejVjNnZ4QndJTDhzam02S3NFdWtkSlR4VzJETVhoUFhU?=
 =?utf-8?B?MmFXclV4TW9FTWdMYjZzMnljR1RxWWxqdWtTNWlIdHJNNlBleURtUnJhTnpB?=
 =?utf-8?B?eVpTT1cvdTNZcFdZMC9TK0tuL1Z2eGlEU01VSUZrUHFpTGtBMjFsdjlwZXp5?=
 =?utf-8?B?anN1eTV3d0RVamp2OTU5YzNiem5YUHQ3YzdLYmxDdzFWc3Z2TWRyZDZESmx0?=
 =?utf-8?B?YlJ2MlFRbFQzVnR0ZUFQQ1A3cUppd3U2SnRlRVdEUVRURWhYYTdkYU40eWpC?=
 =?utf-8?Q?wl5QOo/fHbq7/Qgzz577BLjMy?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 656eeae4-ef55-4292-2707-08da9d0a4129
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:20:59.6670
 (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: 4Te6oJrffCtnTSwx97hzHDtGYOfEYDQxNqnZIZglKsfH5kYBcYqvW7K3yniZbDdSQohZYqLnINEda3LKPdJJUw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5898
X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,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/22/22 19:11, Joe Perches wrote:
>> 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.
> 
> Not in my opinion.

OK, then. :)

> 
>>>  			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);
> 
> Fixing punctuation here would be removing the trailing period as checkpatch
> only has periods for multi-sentence output messages.

OK, let's do that too. 

> 
> And I think that "Do not crash" is a stronger statement than "Avoid crashing"
> so I prefer the original suggestion but it's not a big deal either way.

Yes, stronger wording is better. So how about this:

"Do not crash the kernel unless it is absolutely unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants\n" . $herecurr);


thanks,

-- 
John Hubbard
NVIDIA