Re: [PATCH v3 3/4] drm/i915: Add a partial GGTT view type

[Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>]

On Wed, May 06, 2015 at 02:35:38PM +0300, Joonas Lahtinen wrote:
> +static struct sg_table *
> +intel_partial_pages(const struct i915_ggtt_view *view,
> +		    struct drm_i915_gem_object *obj)
> +{
> +	struct sg_table *st;
> +	struct scatterlist *sg;
> +	struct sg_page_iter obj_sg_iter;
> +	int ret = -ENOMEM;
> +
> +	st = kmalloc(sizeof(*st), GFP_KERNEL);
> +	if (!st)
> +		goto err_st_alloc;
> +
> +	ret = sg_alloc_table(st, view->params.partial.size, GFP_KERNEL);
> +	if (ret)
> +		goto err_sg_alloc;
> +
> +	sg = st->sgl;
> +	st->nents = 0;
> +	for_each_sg_page(obj->pages->sgl, &obj_sg_iter, obj->pages->nents,
> +		view->params.partial.offset)
> +	{
> +		if (st->nents >= view->params.partial.size)
> +			break;

This is a nasty bug, as is the converse where st->nents <
st->orig_nents.

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a7e39d4..115df10 100644
@@ -2890,7 +2900,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
                    struct drm_i915_gem_object *obj)
 {
        struct sg_table *st;
-       struct scatterlist *sg;
+       struct scatterlist *sg, *end;
        struct sg_page_iter obj_sg_iter;
        int ret = -ENOMEM;
 
@@ -2902,24 +2912,31 @@ intel_partial_pages(const struct i915_ggtt_view *view,
        if (ret)
                goto err_sg_alloc;
 
-       sg = st->sgl;
+       end = sg = st->sgl;
        st->nents = 0;
        for_each_sg_page(obj->pages->sgl, &obj_sg_iter, obj->pages->nents,
                view->params.partial.offset)
        {
-               if (st->nents >= view->params.partial.size)
-                       break;
+               if (WARN_ON(st->nents >= view->params.partial.size)) {
+                       ret = -ENODEV;
+                       goto err_pages;
+               }
 
                sg_set_page(sg, NULL, PAGE_SIZE, 0);
                sg_dma_address(sg) = sg_page_iter_dma_address(&obj_sg_iter);
                sg_dma_len(sg) = PAGE_SIZE;
 
+               end = sg;
                sg = sg_next(sg);
                st->nents++;
        }
+       sg_mark_end(end);
 
        return st;
 
+err_pages:
+       sg_free_table(st);
 err_sg_alloc:
        kfree(st);
 err_st_alloc:

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx