| Message ID | 1454411190-15721-8-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 02, 2016 at 11:06:25AM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > VMA creation and GEM list management need the big lock. > > v2: > > Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall) > > Not to mention drm_gem_object_unreference was there in existing > code with no mutex held. > > v3: > > Some callers of i915_gem_object_create_stolen_for_preallocated > already hold the lock so move the mutex into the other caller > as well. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_stolen.c | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 8 ++++++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index c384dc9c8a63..a97a5e762c0f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -635,6 +635,9 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, > if (!drm_mm_initialized(&dev_priv->mm.stolen)) > return NULL; > > + if (WARN_ON_ONCE(!mutex_is_locked(&dev->struct_mutex))) > + return NULL; I'm not keen on !mutex_is_locked(), since it can also just be another thread holding the struct_mutex that grants us protection from this check. At least lockdep checks that the caller holds the lock. -Chris
On Tue, Feb 02, 2016 at 11:06:25AM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > VMA creation and GEM list management need the big lock. > > v2: > > Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall) > > Not to mention drm_gem_object_unreference was there in existing > code with no mutex held. > > v3: > > Some callers of i915_gem_object_create_stolen_for_preallocated > already hold the lock so move the mutex into the other caller > as well. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index c384dc9c8a63..a97a5e762c0f 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -635,6 +635,9 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (!drm_mm_initialized(&dev_priv->mm.stolen)) return NULL; + if (WARN_ON_ONCE(!mutex_is_locked(&dev->struct_mutex))) + return NULL; + DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n", stolen_offset, gtt_offset, size); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a7530cf612d7..5018295cd92b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2551,12 +2551,16 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc, if (size_aligned * 2 > dev_priv->gtt.stolen_usable_size) return false; + mutex_lock(&dev->struct_mutex); + obj = i915_gem_object_create_stolen_for_preallocated(dev, base_aligned, base_aligned, size_aligned); - if (!obj) + if (!obj) { + mutex_unlock(&dev->struct_mutex); return false; + } obj->tiling_mode = plane_config->tiling; if (obj->tiling_mode == I915_TILING_X) @@ -2569,12 +2573,12 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc, mode_cmd.modifier[0] = fb->modifier[0]; mode_cmd.flags = DRM_MODE_FB_MODIFIERS; - mutex_lock(&dev->struct_mutex); if (intel_framebuffer_init(dev, to_intel_framebuffer(fb), &mode_cmd, obj)) { DRM_DEBUG_KMS("intel fb init failed\n"); goto out_unref_obj; } + mutex_unlock(&dev->struct_mutex); DRM_DEBUG_KMS("initial plane fb obj %p\n", obj);