Re: [PATCH 4/6] drm/i915: Add support for stealing purgable stolen pages

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	ankitprasad.r.sharma@intel.com, intel-gfx@lists.freedesktop.org,
	akash.goel@intel.com, shashidhar.hiremath@intel.com
Subject: Re: [PATCH 4/6] drm/i915: Add support for stealing purgable stolen pages
Date: Fri, 9 Oct 2015 10:13:18 +0200	[thread overview]
Message-ID: <20151009081318.GD26718@phenom.ffwll.local> (raw)
In-Reply-To: <20151008150852.GT27939@nuc-i3427.alporthouse.com>

On Thu, Oct 08, 2015 at 04:08:52PM +0100, Chris Wilson wrote:
> On Thu, Oct 08, 2015 at 03:31:08PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 08/10/15 12:09, Chris Wilson wrote:
> > >On Thu, Oct 08, 2015 at 11:43:29AM +0100, Tvrtko Ursulin wrote:
> > >>>-struct drm_i915_gem_object *
> > >>>-i915_gem_object_create_stolen(struct drm_device *dev, u64 size)
> > >>>+static bool
> > >>>+mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind)
> > >>>+{
> > >>>+	BUG_ON(obj->stolen == NULL);
> > >>
> > >>I am fundamentally opposed to BUG_ONs which can be avoided. In this
> > >>I see no value in hanging the machine while we could WARN_ON and
> > >>return false.
> > >
> > >Don't bother with the WARN_ON. Either take the BUG_ON or accept that to
> > >get to this point the machine is dead anyway and a warning here doesn't
> > >help identify the root cause (better off with list debugging and memory
> > >debugging). I have personally been converting these asserts over to a
> > >dev-only compiletime option as I still find the BUGs more useful than
> > >WARNs in the GEM code.
> > 
> > This is one of the ones which are to be expected in development
> > only. At that time I much prefer a WARN_ON since it potentially
> > saves you one reboot cycle and there aren't really any downsides to
> > it. Especially if, as you say, machine is dead already.
> 
> panic-on-oops ftw :-p

We killed drm panic handling, and if it gets resurrect it will be
super-minimal to avoid any kind of "my real oops scrolled off the screen
because i915.ko was dying even harder ..." bug reports. We've done this
because it's pretty much impossible to avoid piles of WARN_ON, lockdep
splats and other crap once you're trying to do a full modeset from panic
context.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx