On Tue, Mar 31, 2015 at 05:32:07PM +0100, Tomas Elf wrote:
> On 19/03/2015 12:30, john.c.harri...@intel.com wrote:
> >@@ -2361,12 +2362,14 @@ void __i915_add_request(struct intel_engine_cs *ring,
> > * is that the flush _must_ happen before the next request, no matter
> > * what.
> > */
> >- if (i915.enable_execlists)
> >- ret = logical_ring_flush_all_caches(ringbuf, request->ctx);
> >- else
> >- ret = intel_ring_flush_all_caches(ring);
> >- /* Not allowed to fail! */
> >- WARN_ON(ret);
> >+ if (flush_caches) {
> >+ if (i915.enable_execlists)
> >+ ret = logical_ring_flush_all_caches(ringbuf,
> >request->ctx);
> >+ else
> >+ ret = intel_ring_flush_all_caches(ring);
> >+ /* Not allowed to fail! */
> >+ WARN_ON(ret);
>
> There has been a discussion about whether it's ok to use
> WARN_ON(<variable/constant>) since it doesn't add any useful information
> about the actual failure case to the kernel log. By that logic you should
> add WARN_ON to each individual call to _flush_all_caches above but I would
> be inclined to say that that would be slightly redundant. It could be argued
> that if you get a WARN stack_dump in the kernel log at this point you pretty
> much know from where it came.
>
> Although, if you want to be able to rely entirely on the kernel log and
> don't want to read the source code to figure out what function call failed
> then you would have to either add WARN_ONs to each individual function call
> or replace WARN_ON(ret) with something like WARN(ret, "flush_all_caches
> returned %d", ret) or something.
>
> Any other opinion on how we should do this from anyone? What pattern should
> we be following here?
WARN_ON dumps the full backtrace plus current EIP/RIP including basic
decoding using debug information (if compiled in). Maybe you don't have
that enabled in Android builds? Or is there additional information you
want to dump?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
