BUG_ON vs WARN_ON (was: Re: [PATCH] drm/i915: Localise the fbdev console lock frobbing)

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>,
	Chris Wilson <chris@chris-wilson.co.uk>Daniel Vetter
	<daniel@ffwll.ch>, intel-gfx <intel-gfx@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: BUG_ON vs WARN_ON (was: Re: [PATCH] drm/i915: Localise the fbdev console lock frobbing)
Date: Thu, 14 Aug 2014 13:07:06 +0300	[thread overview]
Message-ID: <871tsjqt9x.fsf@intel.com> (raw)
In-Reply-To: <CAKMK7uHcEaHD1T7jRdA24dmjM8UexNC5c8J1oDn2x21yT0C-qw@mail.gmail.com>

On Thu, 14 Aug 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Aug 14, 2014 at 8:54 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> I disaggree with the conversion of the BUG_ON though, a WARN there is
>> going to screw up unpredictably (well, a hard hang without any output
>> is the predictable outcome). I'd like to have asserts for things that
>> could and should be statically analyzed...
>
> Well I've put a zero-tolerance rule for BUG_ON into place with the
> only exception if the kernel will die anyway in the next few lines.
> Which means I trade in a limping (and potentially dangerous) kernel
> for the ability to be able to read the backtrace somewhere. I agree
> that any such extreme policy will end up looking stupid in some cases,
> but I've just decided that I wasted too much time on chasing lookups
> which would have been trivial to debug with a WARN_ON instead of a
> BUG_ON.
>
> Until I've wasted too much time with WARN_ON instead of BUG_ON I'll
> let it stick.  And it's supported by my patch scripts, so small chance
> I'll miss one. Ofc I'll never change it without a notice in the commit
> message, so people can always blame me for it.

In other words, WARN_ON is the new BUG_ON. But what's the new WARN_ON?
Now we're conflating two things (limp home mode and crashing) into
one. When I see WARN_ON in code, it's no longer clear to me whether this
is a condition that we're supposed to survive or not. For example, does
the code below a WARN_ON need to properly handle errors due to the
condition? To me, BUG_ON is a code reading aide that sets the absolute
precondition for the following code. Something that absolutely must be
fixed if someone hits it, while WARN_ON can sometimes be ignored, or
even replaced with DRM_DEBUG.

If we have zero-tolerance for BUG_ON, and replace all of those with
WARN_ON, we'll need to start being *very* selective about adding WARN_ON
as well.

BR,
Jani.

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center