On 06/03/2015 17:36, John Harrison wrote:
On 06/03/2015 16:18, Daniel Vetter wrote:
On Thu, Mar 05, 2015 at 08:13:07PM +0000, Tomas Elf wrote:
On 05/03/2015 15:46, John Harrison wrote:
On 05/03/2015 15:27, Tomas Elf wrote:
On 19/02/2015 17:17, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
The alloc_request() function does not actually return the newly
allocated
request. Instead, it must be pulled from
ring->outstanding_lazy_request. This
patch fixes this so that code can create a request and start using it
knowing
exactly which request it actually owns.
For: VIZ-5115
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++-
drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++----
drivers/gpu/drm/i915/intel_lrc.h | 3 ++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++----
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++-
6 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 87a4a2e..90223f208 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1909,7 +1909,8 @@ struct drm_i915_private {
/* Abstract the submission mechanism (legacy ringbuffer or
execlists) away */
struct {
int (*alloc_request)(struct intel_engine_cs *ring,
- struct intel_context *ctx);
+ struct intel_context *ctx,
+ struct drm_i915_gem_request **req_out);
int (*do_execbuf)(struct i915_execbuffer_params *params,
struct drm_i915_gem_execbuffer2 *args,
struct list_head *vmas);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 61471e9..37dcc6f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
void *data,
struct i915_address_space *vm;
struct i915_execbuffer_params params_master; /* XXX: will be
removed later */
struct i915_execbuffer_params *params = ¶ms_master;
+ struct drm_i915_gem_request *request;
Please initialize request to NULL. If you accidentally dereference it
before it is allocated (seeing as the allocation is several pages down
>from here) you get a null pointer exception, which is a clear
indication that you did something stupid. Otherwise it's not clear
what will happen (sure, page fault, but null pointer exception is a
better failure indication).
That should generate a 'use before assignment' compiler warning.
That assumes that the developer in question isn't too busy hacking to
check
for warnings (in all honesty that developer probably would've been me
;)).
Sure, you should always check for warnings but if we can save this
developer
some time by giving them a clear run-time indication aside from the
compile-time warning then that would not be a bad thing. I've been there
myself a few times and I know times in the past where this would've
saved me
the time it takes to rebuild and redeploy the kernel once.
kbuild is _very_ good at catching and reporting these. And the linux
kernel generally has a 0 warnings rule, which means even when it slips
through a lot of developers will get mad at me for not spotting it.
I think relying upon gcc to do its job here is safe enough. We only
initialize stack variables if it's really needed, or if gcc is too dense
to figure it out itself.
-Daniel
I agree with Daniel. If you ignore warnings in your own test builds, you
get what you deserve. If you check warnings into the tree then you
should be shot. Or worse.
Reviewed-by: Tomas Elf <tomas.elf@xxxxxxxxx>
Thanks,
Tomas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
