drm/i915: double unlock in intel_setup_overlay()

drm/i915: double unlock in intel_setup_overlay()

[Dan Carpenter]

Smatch complains about the recent locking changes in
intel_setup_overlay() that it unlocks twice on an error path.  I was
also curious if we shouldn't call i915_gem_object_unpin() on that
error path.  I don't know the drm code well enough to know about
unpinning so I'm sending this bug report instead of patching it
myself.  :)

regards,
dan carpenter

drivers/gpu/drm/i915/intel_overlay.c +1471 intel_setup_overlay(73)
	error: double unlock 'mutex:&dev->struct_mutex'

  1445          mutex_unlock(&dev->struct_mutex);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	unlock.

  1446  
  1447          /* init all values */
  1448          overlay->color_key = 0x0101fe;
  1449          overlay->brightness = -19;
  1450          overlay->contrast = 75;
  1451          overlay->saturation = 146;
  1452  
  1453          regs = intel_overlay_map_regs(overlay);
  1454          if (!regs)
  1455                  goto out_free_bo;
                        ^^^^^^^^^^^^^^^^^
	we hit this goto here.

  1456  
  1457          memset(regs, 0, sizeof(struct overlay_registers));
  1458          update_polyphase_filter(regs);
  1459          update_reg_attrs(overlay, regs);
  1460  
  1461          intel_overlay_unmap_regs(overlay, regs);
  1462  
  1463          dev_priv->overlay = overlay;
  1464          DRM_INFO("initialized overlay support\n");
  1465          return;
  1466  
  1467  out_unpin_bo:
  1468          i915_gem_object_unpin(reg_bo);
  1469  out_free_bo:
  1470          drm_gem_object_unreference(&reg_bo->base);
  1471          mutex_unlock(&dev->struct_mutex);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	double unlock here.

  1472  out_free:
  1473          kfree(overlay);
  1474          return;
  1475  }

Re: drm/i915: double unlock in intel_setup_overlay()

[Chris Wilson]

On Thu, 23 Jun 2011 21:06:16 +0300, Dan Carpenter <error27@gmail.com> wrote:
> Smatch complains about the recent locking changes in
> intel_setup_overlay() that it unlocks twice on an error path.  I was
> also curious if we shouldn't call i915_gem_object_unpin() on that
> error path.  I don't know the drm code well enough to know about
> unpinning so I'm sending this bug report instead of patching it
> myself.  :)

Right, along the 945+ error path we need to unpin. Where did the locking
fixes land, I don't see the updated intel_overlay.c to comment upon the
double unlock?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre