Re: [PATCH RFC 2/2] checkpatch: warn on usage of VM_BUG_ON() and friends
From: David Hildenbrand <hidden>
Date: 2022-08-25 11:51:33
Also in:
kexec, linux-mm, lkml
On 25.08.22 13:43, Jani Nikula wrote:
On Thu, 25 Aug 2022, David Hildenbrand [off-list ref] wrote:quoted
On 24.08.22 18:52, Joe Perches wrote:quoted
On Wed, 2022-08-24 at 18:31 +0200, David Hildenbrand wrote:quoted
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 friends as well. While at it, make it clearer that the kernel really shouldn't be crashed. Note that there are some other *_BUG_ON flavors, but they are not all bad: for example, KVM_BUG_ON() only triggers a WARN_ON_ONCE and then flags KVM as being buggy, so we'll not care about them for now here.[]quoted
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl[]quoted
@@ -4695,12 +4695,12 @@ sub process { } } -# avoid BUG() or BUG_ON() - if ($line =~ /\b(?:BUG|BUG_ON)\b/) { +# do not use BUG(), BUG_ON(), VM_BUG_ON() and friends. + if ($line =~ /\b(?:BUG|BUG_ON|VM_BUG_ON|VM_BUG_ON_[A-Z]+)\b/) {Perhaps better as something like the below to pick up more variantsTrying to find more possible variants and exceptionsquoted
CI_BUG_ON( -> Bad with CONFIG_DRM_I915_DEBUG GEM_BUG_ON( -> Bad with CONFIG_DRM_I915_DEBUG_GEM_ONCEThese are hidden behind debug knobs that we use in our CI to specifically catch "should not happen" cases fast and loud. Should not be a problem for regular users.
I tend to agree but I don't think this is worth an exception. VM_BUG_ON also requires CONFIG_DEBUG_VM and absolutely shouldn't be used as I learned. Quoting Linus: Really. BUG_ON() IS NOT FOR DEBUGGING. [1] This kind of "I don't think this can happen" is _never_ an excuse for it. [2] For CI work, it might be sufficient to use WARN_ON_ONCE() combined with panic_on_warn. [1] https://lore.kernel.org/all/CAHk-=wiEAH+ojSpAgx_Ep=NKPWHU8AdO3V56BXcCsU97oYJ1EA@mail.gmail.com/ (local) [2] https://lore.kernel.org/all/CAHk-=wg40EAZofO16Eviaj7mfqDhZ2gVEbvfsMf6gYzspRjYvw@mail.gmail.com/ (local) -- Thanks, David / dhildenb