[PATCH] drm/i915: Clear execbuf's vma backpointer upon release

[PATCH] drm/i915: Clear execbuf's vma backpointer upon release

[Chris Wilson]

commit 2889caa92321 ("drm/i915: Eliminate lots of iterations over the
execobjects array") jiggled around the error handling and replace a test
that we cleaned up properly after ourselves with an assertion. That
assertion failed because in the release function (moments after the
assertion) we were indeed forgetting to mark the vma as cleared. The
consequence was when testing an invalid relocation address, we would try
to release the vma twice (following the couple of attempts to verify the
address) and on the second release notice that the first release was
incomplete.

Testcase: igt/gem_reloc_overflow/invalid-address
Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index b2457556591c..ec33b358fba9 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -879,6 +879,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
 
 		GEM_BUG_ON(vma->exec_entry != entry);
 		vma->exec_entry = NULL;
+		__exec_to_vma(entry) = 0;
 
 		if (entry->flags & __EXEC_OBJECT_HAS_PIN)
 			__eb_unreserve_vma(vma, entry);
-- 
2.11.0

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

✓ Fi.CI.BAT: success for drm/i915: Clear execbuf's vma backpointer upon release

[Patchwork]

== Series Details ==

Series: drm/i915: Clear execbuf's vma backpointer upon release
URL   : https://patchwork.freedesktop.org/series/26210/
State : success

== Summary ==

Series 26210v1 drm/i915: Clear execbuf's vma backpointer upon release
https://patchwork.freedesktop.org/api/1.0/series/26210/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test prime_busy:
        Subgroup basic-wait-after-default:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#101515 +3

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101515 https://bugs.freedesktop.org/show_bug.cgi?id=101515

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:437s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:431s
fi-bsw-n3050     total:278  pass:241  dwarn:1   dfail:0   fail:0   skip:36  time:529s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:506s
fi-byt-j1900     total:278  pass:252  dwarn:2   dfail:0   fail:0   skip:24  time:487s
fi-byt-n2820     total:278  pass:248  dwarn:2   dfail:0   fail:0   skip:28  time:481s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:582s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:436s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:416s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:424s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:497s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:571s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:583s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:457s
fi-skl-6700hq    total:278  pass:220  dwarn:3   dfail:0   fail:30  skip:24  time:341s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:477s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:479s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:402s

7f2c92652872fbbe37c8bca92eccb7218100c21d drm-tip: 2017y-06m-22d-08h-47m-32s UTC integration manifest
249f38e drm/i915: Clear execbuf's vma backpointer upon release

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5022/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Clear execbuf's vma backpointer upon release

[Tvrtko Ursulin]

On 22/06/2017 11:47, Chris Wilson wrote:
> commit 2889caa92321 ("drm/i915: Eliminate lots of iterations over the
> execobjects array") jiggled around the error handling and replace a test
> that we cleaned up properly after ourselves with an assertion. That
> assertion failed because in the release function (moments after the
> assertion) we were indeed forgetting to mark the vma as cleared. The
> consequence was when testing an invalid relocation address, we would try
> to release the vma twice (following the couple of attempts to verify the
> address) and on the second release notice that the first release was
> incomplete.
> 
> Testcase: igt/gem_reloc_overflow/invalid-address
> Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index b2457556591c..ec33b358fba9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -879,6 +879,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
>   
>   		GEM_BUG_ON(vma->exec_entry != entry);
>   		vma->exec_entry = NULL;
> +		__exec_to_vma(entry) = 0;
>   
>   		if (entry->flags & __EXEC_OBJECT_HAS_PIN)
>   			__eb_unreserve_vma(vma, entry);
> 

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

Re: [PATCH] drm/i915: Clear execbuf's vma backpointer upon release

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-06-22 12:36:37)
> 
> On 22/06/2017 11:47, Chris Wilson wrote:
> > commit 2889caa92321 ("drm/i915: Eliminate lots of iterations over the
> > execobjects array") jiggled around the error handling and replace a test
> > that we cleaned up properly after ourselves with an assertion. That
> > assertion failed because in the release function (moments after the
> > assertion) we were indeed forgetting to mark the vma as cleared. The
> > consequence was when testing an invalid relocation address, we would try
> > to release the vma twice (following the couple of attempts to verify the
> > address) and on the second release notice that the first release was
> > incomplete.
> > 
> > Testcase: igt/gem_reloc_overflow/invalid-address
> > Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index b2457556591c..ec33b358fba9 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -879,6 +879,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
> >   
> >               GEM_BUG_ON(vma->exec_entry != entry);
> >               vma->exec_entry = NULL;
> > +             __exec_to_vma(entry) = 0;
> >   
> >               if (entry->flags & __EXEC_OBJECT_HAS_PIN)
> >                       __eb_unreserve_vma(vma, entry);
> > 
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks for the quick review, pushed for Tomi's sake.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx