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