All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma()
@ 2017-09-12 15:07 Chris Wilson
  2017-09-12 15:21 ` Tvrtko Ursulin
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Chris Wilson @ 2017-09-12 15:07 UTC (permalink / raw)
  To: intel-gfx

Following the simplification to a single lookup loop in commit
170fa29b14fa ("drm/i915: Simplify eb_lookup_vmas()") and commit
d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr"), we can go
one setup further and reorder the error paths so that the state of the
local variable obj is always known to the compiler and doesn't need the
uninitialized_var markup to squelch a compiler warning.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 7687483ff218..214a850b4b3c 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -679,7 +679,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
 static int eb_lookup_vmas(struct i915_execbuffer *eb)
 {
 	struct radix_tree_root *handles_vma = &eb->ctx->handles_vma;
-	struct drm_i915_gem_object *uninitialized_var(obj);
+	struct drm_i915_gem_object *obj;
 	unsigned int i;
 	int err;
 
@@ -725,19 +725,17 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 			goto err_obj;
 		}
 
+		/* transfer ref to ctx */
 		vma->open_count++;
 		list_add(&lut->obj_link, &obj->lut_list);
 		list_add(&lut->ctx_link, &eb->ctx->handles_list);
 		lut->ctx = eb->ctx;
 		lut->handle = handle;
 
-		/* transfer ref to ctx */
-		obj = NULL;
-
 add_vma:
 		err = eb_add_vma(eb, i, vma);
 		if (unlikely(err))
-			goto err_obj;
+			goto err_vma;
 
 		GEM_BUG_ON(vma != eb->vma[i]);
 		GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
@@ -766,8 +764,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 	return eb_reserve(eb);
 
 err_obj:
-	if (obj)
-		i915_gem_object_put(obj);
+	i915_gem_object_put(obj);
 err_vma:
 	eb->vma[i] = NULL;
 	return err;
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma()
  2017-09-12 15:07 [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma() Chris Wilson
@ 2017-09-12 15:21 ` Tvrtko Ursulin
  2017-09-12 20:04   ` Chris Wilson
  2017-09-12 15:34 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-09-12 19:32 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 1 reply; 5+ messages in thread
From: Tvrtko Ursulin @ 2017-09-12 15:21 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 12/09/2017 16:07, Chris Wilson wrote:
> Following the simplification to a single lookup loop in commit
> 170fa29b14fa ("drm/i915: Simplify eb_lookup_vmas()") and commit
> d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr"), we can go
> one setup further and reorder the error paths so that the state of the
> local variable obj is always known to the compiler and doesn't need the
> uninitialized_var markup to squelch a compiler warning.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 7687483ff218..214a850b4b3c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -679,7 +679,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
>   static int eb_lookup_vmas(struct i915_execbuffer *eb)
>   {
>   	struct radix_tree_root *handles_vma = &eb->ctx->handles_vma;
> -	struct drm_i915_gem_object *uninitialized_var(obj);
> +	struct drm_i915_gem_object *obj;
>   	unsigned int i;
>   	int err;
>   
> @@ -725,19 +725,17 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>   			goto err_obj;
>   		}
>   
> +		/* transfer ref to ctx */
>   		vma->open_count++;
>   		list_add(&lut->obj_link, &obj->lut_list);
>   		list_add(&lut->ctx_link, &eb->ctx->handles_list);
>   		lut->ctx = eb->ctx;
>   		lut->handle = handle;
>   
> -		/* transfer ref to ctx */
> -		obj = NULL;
> -
>   add_vma:
>   		err = eb_add_vma(eb, i, vma);
>   		if (unlikely(err))
> -			goto err_obj;
> +			goto err_vma;
>   
>   		GEM_BUG_ON(vma != eb->vma[i]);
>   		GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
> @@ -766,8 +764,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>   	return eb_reserve(eb);
>   
>   err_obj:
> -	if (obj)
> -		i915_gem_object_put(obj);
> +	i915_gem_object_put(obj);
>   err_vma:
>   	eb->vma[i] = NULL;
>   	return err;
> 

Looks good to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Cleanup error paths through eb_lookup_vma()
  2017-09-12 15:07 [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma() Chris Wilson
  2017-09-12 15:21 ` Tvrtko Ursulin
@ 2017-09-12 15:34 ` Patchwork
  2017-09-12 19:32 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-09-12 15:34 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Cleanup error paths through eb_lookup_vma()
URL   : https://patchwork.freedesktop.org/series/30213/
State : success

== Summary ==

Series 30213v1 drm/i915: Cleanup error paths through eb_lookup_vma()
https://patchwork.freedesktop.org/api/1.0/series/30213/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l) fdo#101781

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:446s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:451s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:375s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:527s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:272s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:513s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:498s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:492s
fi-cfl-s         total:289  pass:250  dwarn:4   dfail:0   fail:0   skip:35  time:454s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:453s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:594s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:428s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:408s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:437s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:488s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:459s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:492s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:580s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:587s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:548s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:455s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:520s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:463s
fi-skl-x1585l    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:568s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:428s

694f07d3df18c02da3f526ae0e1238eb12534e1e drm-tip: 2017y-09m-12d-09h-59m-00s UTC integration manifest
cd93d7d8e24a drm/i915: Cleanup error paths through eb_lookup_vma()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5661/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Cleanup error paths through eb_lookup_vma()
  2017-09-12 15:07 [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma() Chris Wilson
  2017-09-12 15:21 ` Tvrtko Ursulin
  2017-09-12 15:34 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-09-12 19:32 ` Patchwork
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-09-12 19:32 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Cleanup error paths through eb_lookup_vma()
URL   : https://patchwork.freedesktop.org/series/30213/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2301 pass:1236 dwarn:0   dfail:0   fail:13  skip:1052 time:9407s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5661/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma()
  2017-09-12 15:21 ` Tvrtko Ursulin
@ 2017-09-12 20:04   ` Chris Wilson
  0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-09-12 20:04 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2017-09-12 16:21:06)
> 
> On 12/09/2017 16:07, Chris Wilson wrote:
> > Following the simplification to a single lookup loop in commit
> > 170fa29b14fa ("drm/i915: Simplify eb_lookup_vmas()") and commit
> > d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr"), we can go
> > one setup further and reorder the error paths so that the state of the
> > local variable obj is always known to the compiler and doesn't need the
> > uninitialized_var markup to squelch a compiler warning.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
[snip]

> Looks good to me.
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Ta, and pushed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-09-12 20:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-12 15:07 [PATCH] drm/i915: Cleanup error paths through eb_lookup_vma() Chris Wilson
2017-09-12 15:21 ` Tvrtko Ursulin
2017-09-12 20:04   ` Chris Wilson
2017-09-12 15:34 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-09-12 19:32 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.