[patch v2] i915: take struct_mutex lock in intel_setup_overlay()
Dan Carpenter
error27 at gmail.com
Sat Jun 19 06:39:01 PDT 2010
This patch never made it into 2.6.35 but it's still valid. Can you pick
it up for 2.6.36? It still applies cleanly to 2.6.35-rc3 with fuzz (8
lines offset).
regards,
dan carpenter
On Mon, May 03, 2010 at 09:58:32AM +0200, Dan Carpenter wrote:
> I changed the drm_gem_object_unreference() to
> drm_gem_object_unreference_unlocked().
>
> "reg_bo" is a local variable so there are no synchronization issues, but
> the problem is that if struct_mutex isn't held, it triggers a BUG_ON()
> in drm_gem_object_free().
>
> I also took the "struct_mutex" when calling i915_gem_object_pin().
> The i915_gem_object_pin() function calls i915_gem_object_bind_to_gtt()
> which unreferences stuff if we are low on gtt space. The truth is that
> if we run out of gtt space on this code path, we're sunk already
> regardless of the locking. But we should take the lock anyway for
> consistency and to make the static checkers happy.
>
> That call tree is:
> i915_gem_object_pin()
> => i915_gem_object_bind_to_gtt()
> => i915_gem_evict_something()
> => i915_gem_retire_requests()
> => i915_gem_retire_request()
> => drm_gem_object_unreference()
>
> intel_setup_overlay() is always called with "struct_mutex" unlocked.
> There is only one call tree and it is:
>
> i915_load_modeset_init()
> => intel_modeset_init()
> => intel_setup_overlay()
>
> Signed-off-by: Dan Carpenter <error27 at gmail.com>
> ---
> V2: The code is the same, but the change log is updated.
> No one has complained about hitting the BUG_ON() and I can't tested it
> myself, so this patch is probably linux-next material.
>
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 6d524a1..a103582 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1347,7 +1347,9 @@ void intel_setup_overlay(struct drm_device *dev)
> overlay->reg_bo = to_intel_bo(reg_bo);
>
> if (OVERLAY_NONPHYSICAL(dev)) {
> + mutex_lock(&dev->struct_mutex);
> ret = i915_gem_object_pin(reg_bo, PAGE_SIZE);
> + mutex_unlock(&dev->struct_mutex);
> if (ret) {
> DRM_ERROR("failed to pin overlay register bo\n");
> goto out_free_bo;
> @@ -1385,7 +1387,7 @@ void intel_setup_overlay(struct drm_device *dev)
> return;
>
> out_free_bo:
> - drm_gem_object_unreference(reg_bo);
> + drm_gem_object_unreference_unlocked(reg_bo);
> out_free:
> kfree(overlay);
> return;
More information about the dri-devel
mailing list