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

[Chris Wilson]

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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx