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

Damien Lespiau
2013-08-30T14:28:14Z

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