Re: [PATCH 03/17] drm/i915: Pin pages whilst mapping the dma-buf

[Damien Lespiau <damien.lespiau@intel.com>]

On Mon, Aug 26, 2013 at 07:50:55PM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> As we attempt to kmalloc after calling get_pages, there is a possibility
> that the shrinker may reap the pages we just acquired. To prevent this
> we need to increment the pages_pin_count early, so rearrange the code
> and error paths to make it so.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 41 ++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index e918b05..7d5752f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -42,27 +42,24 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
>  
>  	ret = i915_mutex_lock_interruptible(obj->base.dev);
>  	if (ret)
> -		return ERR_PTR(ret);
> +		goto err;
>  
>  	ret = i915_gem_object_get_pages(obj);
> -	if (ret) {
> -		st = ERR_PTR(ret);
> -		goto out;
> -	}
> +	if (ret)
> +		goto err_unlock;
> +
> +	i915_gem_object_pin_pages(obj);
>  
>  	/* Copy sg so that we make an independent mapping */
>  	st = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
>  	if (st == NULL) {
> -		st = ERR_PTR(-ENOMEM);
> -		goto out;
> +		ret = -ENOMEM;
> +		goto err_unpin;
>  	}
>  
>  	ret = sg_alloc_table(st, obj->pages->nents, GFP_KERNEL);
> -	if (ret) {
> -		kfree(st);
> -		st = ERR_PTR(ret);
> -		goto out;
> -	}
> +	if (ret)
> +		goto err_free;
>  
>  	src = obj->pages->sgl;
>  	dst = st->sgl;
> @@ -73,17 +70,23 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
>  	}
>  
>  	if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
> -		sg_free_table(st);
> -		kfree(st);
> -		st = ERR_PTR(-ENOMEM);
> -		goto out;
> +		ret =-ENOMEM;
> +		goto err_free_sg;
>  	}
>  
> -	i915_gem_object_pin_pages(obj);
> -
> -out:
>  	mutex_unlock(&obj->base.dev->struct_mutex);
>  	return st;
> +
> +err_free_sg:
> +	sg_free_table(st);
> +err_free:
> +	kfree(st);
> +err_unpin:
> +	i915_gem_object_unpin_pages(obj);
> +err_unlock:
> +	mutex_unlock(&obj->base.dev->struct_mutex);
> +err:
> +	return ERR_PTR(ret);
>  }
>  
>  static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx