| Message ID | 56A66222.2020206@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hey, Op 25-01-16 om 18:57 schreef Dave Gordon: > In the error-handling paths of i915_gem_do_execbuffer() and > intel_crtc_page_flip(), the local pointer-to-request variables > were expected to be either valid pointers or NULL. Since > > 2682708 drm/i915: simplify allocation of driver-internal requests > > they could also be ERR_PTR() values, so the tests need to be > updated to accommodate this case. > > v2: Added testcase (Chris Wilson) > > Testcase: igt/gem_close_race > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> The testcase you mentioned doesn't work, it crashes on my machine instead of running to completion or triggering the bug. gem_ringfill.default-interruptible seems to trigger it. ~Maarten
On 27/01/16 12:33, Maarten Lankhorst wrote: > Hey, > > Op 25-01-16 om 18:57 schreef Dave Gordon: >> In the error-handling paths of i915_gem_do_execbuffer() and >> intel_crtc_page_flip(), the local pointer-to-request variables >> were expected to be either valid pointers or NULL. Since >> >> 2682708 drm/i915: simplify allocation of driver-internal requests >> >> they could also be ERR_PTR() values, so the tests need to be >> updated to accommodate this case. >> >> v2: Added testcase (Chris Wilson) >> >> Testcase: igt/gem_close_race >> Signed-off-by: Dave Gordon <david.s.gordon@intel.com> >> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > The testcase you mentioned doesn't work, it crashes on my machine instead of running to completion or triggering the bug. It worked for me but I think I was running with GuC enabled at the time so that might be the differenct. gem_close_race in general worked for me just yesterday. Regards, Tvrtko
Hey, Op 27-01-16 om 13:41 schreef Tvrtko Ursulin: > > On 27/01/16 12:33, Maarten Lankhorst wrote: >> Hey, >> >> Op 25-01-16 om 18:57 schreef Dave Gordon: >>> In the error-handling paths of i915_gem_do_execbuffer() and >>> intel_crtc_page_flip(), the local pointer-to-request variables >>> were expected to be either valid pointers or NULL. Since >>> >>> 2682708 drm/i915: simplify allocation of driver-internal requests >>> >>> they could also be ERR_PTR() values, so the tests need to be >>> updated to accommodate this case. >>> >>> v2: Added testcase (Chris Wilson) >>> >>> Testcase: igt/gem_close_race >>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com> >>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> The testcase you mentioned doesn't work, it crashes on my machine instead of running to completion or triggering the bug. > > It worked for me but I think I was running with GuC enabled at the time so that might be the differenct. > > gem_close_race in general worked for me just yesterday. It crashes in __lll_unlock_elision called from pthread_mutex_unlock, but I don't see anything weird there that could cause it. Broadwell with the exact same config works. It's tempting to blame libc, it looks like a bug in its tsx stuff. ~Maarten
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 2dc08ce..a7bd555 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1654,7 +1654,7 @@ static bool only_mappable_for_reloc(unsigned int flags) * must be freed again. If it was submitted then it is being tracked * on the active request list and no clean up is required here. */ - if (ret && req) + if (ret && !IS_ERR_OR_NULL(req)) i915_gem_request_cancel(req); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8104511..b88cdac 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11726,7 +11726,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, cleanup_unpin: intel_unpin_fb_obj(fb, crtc->primary->state); cleanup_pending: - if (request) + if (!IS_ERR_OR_NULL(request)) i915_gem_request_cancel(request); atomic_dec(&intel_crtc->unpin_work_count);