From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Widawsky <ben@bwidawsk.net>
Subject: Re: [PATCH] drm/i915/overlay: Fix unpinning along init
 error paths
Date: Tue, 28 Jun 2011 08:57:14 -0700
Message-ID: <20110628155714.GA2961@snipes.kumite>
References: <1309256867-26210-1-git-send-email-chris@chris-wilson.co.uk>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com
	[184.106.247.128])
	by gabe.freedesktop.org (Postfix) with ESMTP id D1F639E79D
	for <intel-gfx@lists.freedesktop.org>;
	Tue, 28 Jun 2011 08:57:44 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <1309256867-26210-1-git-send-email-chris@chris-wilson.co.uk>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Tue, Jun 28, 2011 at 11:27:47AM +0100, Chris Wilson wrote:
> As pointed out by Dan Carpenter, it was seemingly possible to hit an error
> whilst mapping the buffer for the regs (except the only likely error
> returns should not happen during init) and so leak a pin count on the
> bo. To handle this we would need to reacquire the struct mutex, so for
> simplicity rearrange for the lock to be held for the entire function.
> For extra pedagogy, test that we only call init once.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index cffd3ed..d360380 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1405,6 +1405,11 @@ void intel_setup_overlay(struct drm_device *dev)
>  	overlay = kzalloc(sizeof(struct intel_overlay), GFP_KERNEL);
>  	if (!overlay)
>  		return;
> +
> +	mutex_lock(&dev->struct_mutex);
> +	if (WARN_ON(dev_priv->overlay))
> +		goto out_free;
> +
>  	overlay->dev = dev;
>  
>  	reg_bo = i915_gem_alloc_object(dev, PAGE_SIZE);

For my curiosity, was there a reason you chose to acquire the lock after
the kzalloc?