From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula 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 Message-ID: <871tsjqt9x.fsf@intel.com> References: <20140813113922.GU10500@phenom.ffwll.local> <1407931786-22097-1-git-send-email-chris@chris-wilson.co.uk> <20140813124653.GZ10500@phenom.ffwll.local> <20140813125830.GG10052@nuc-i3427.alporthouse.com> <20140813130402.GB10500@phenom.ffwll.local> <20140813130922.GH10052@nuc-i3427.alporthouse.com> <20140813134024.GC10500@phenom.ffwll.local> <20140814065431.GK10052@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 7806C6E650 for ; Thu, 14 Aug 2014 03:09:16 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter , Chris Wilson Daniel Vetter , intel-gfx , Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org On Thu, 14 Aug 2014, Daniel Vetter wrote: > On Thu, Aug 14, 2014 at 8:54 AM, Chris Wilson 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