--------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 > -----Original Message----- > From: Chris Wilson [mailto:chris@xxxxxxxxxxxxxxxxxx] > Sent: Monday, May 12, 2014 11:09 AM > To: Mateo Lozano, Oscar > Cc: Daniel Vetter; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] drm/i915: Gracefully handle obj not bound to > GGTT in is_pin_display > > On Mon, May 12, 2014 at 09:05:45AM +0000, Mateo Lozano, Oscar wrote: > > Hi Daniel, > > > > Sorry, this fell through the cracks: > > > > > Subject: Re: [PATCH] drm/i915: Gracefully handle obj not > > > bound to GGTT in is_pin_display > > > > > > On Wed, Apr 02, 2014 at 07:21:01PM +0100, oscar.mateo@xxxxxxxxx > wrote: > > > > From: Oscar Mateo <oscar.mateo@xxxxxxxxx> > > > > > > > > Otherwise, we do a NULL pointer dereference. > > > > > > > > I've seen this happen while handling an error in > > > > i915_gem_object_pin_to_display_plane(): > > > > > > > > If i915_gem_object_set_cache_level() fails, we call > > > > is_pin_display() to handle the error. At this point, the object is > > > > still not pinned to GGTT and maybe not even bound, so we have to > > > > check before we dereference its GGTT vma. > > > > > > > > Issue: VIZ-3772 > > > > Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> > > > > > > Have you looked into provoking this with an igt testcase? On a hunch > > > a busy load (to extend the race window) plus the usual interruptor > > > trick to jump out of wait_seqno calls should be able to make this go > > > kaboom on command. But I haven't analyzed the bug in detail. > > > > AFAICT, the only sequence where this likely to happen (because we are > handling a recently created object) is: > > > > intelfb_alloc -> intel_pin_and_fence_fb_obj -> > > i915_gem_object_pin_to_display_plane -> > > i915_gem_object_set_cache_level -> is_pin_display > > > > Now, I think intelfb_alloc only happens during bootup, so reproducing it on > IGT would be difficult. > > I still feel the fix is valid though :) > > Did you consider my alternative fix of restoring the old value in the error path? > -Chris Is that directed to Daniel or me? Restoring the old value is way easier, but I thought you wanted to keep is_pin_display as a theory of operation? And Daniel might still want an IGT test, I suppose... _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
