Re: [bug report] drm/i915: Allow compaction upto SWIOTLB max segment size

[Chris Wilson <chris@chris-wilson.co.uk>]

On Mon, Nov 14, 2016 at 02:14:56PM +0300, Dan Carpenter wrote:
> Hello Chris Wilson,
> 
> The patch 871dfbd67d4e: "drm/i915: Allow compaction upto SWIOTLB max
> segment size" from Oct 11, 2016, leads to the following static
> checker warning:
> 
> 	drivers/gpu/drm/i915/i915_gem.c:2357 i915_gem_object_get_pages_gtt()
> 	error: we previously assumed 'sg' could be null (see line 2341)
> 
> drivers/gpu/drm/i915/i915_gem.c
>   2339                  /* Check that the i965g/gm workaround works. */
>   2340                  WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
>   2341          }
>   2342          if (sg) /* loop terminated early; short sg table */
> 
> We added a new check for NULL.
> 
>   2343                  sg_mark_end(sg);
>   2344  
>   2345          /* Trim unused sg entries to avoid wasting memory. */
>   2346          i915_sg_trim(st);
>   2347  
>   2348          ret = i915_gem_gtt_prepare_pages(obj, st);
>   2349          if (ret)
>   2350                  goto err_pages;
> 
> but we hit this goto
> 
>   2351  
>   2352          if (i915_gem_object_needs_bit17_swizzle(obj))
>   2353                  i915_gem_object_do_bit_17_swizzle(obj, st);
>   2354  
>   2355          return st;
>   2356  
>   2357  err_pages:
>   2358          sg_mark_end(sg);
> 
> Then don't check here.  Also do we really need to sg_mark_end() twice?

We need the mark_end when escaping from the loop, but after the loop, sg
may be NULL.

Archaelogy, suggests commit e227330223a7 ("drm/i915: avoid leaking DMA
mappings") as the original culprit (i.e. the introduction of goto
err_pages after the loop).
-Chris

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