[igt-dev] [PATCH i-g-t] igt/gem_eio: Preserve batch between reset-stress iterations

[igt-dev] [PATCH i-g-t] igt/gem_eio: Preserve batch between reset-stress iterations

[Chris Wilson]

We can keep the original batch around and avoid recreating it between
reset iterations to focus on the impact of resets.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 tests/gem_eio.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/tests/gem_eio.c b/tests/gem_eio.c
index de161332d..5250a414c 100644
--- a/tests/gem_eio.c
+++ b/tests/gem_eio.c
@@ -650,35 +650,38 @@ static void reset_stress(int fd,
 			 uint32_t ctx0, unsigned int engine,
 			 unsigned int flags)
 {
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	struct drm_i915_gem_exec_object2 obj = {
+		.handle = gem_create(fd, 4096)
+	};
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+		.flags = engine,
+	};
+	gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
+
 	igt_until_timeout(5) {
-		struct drm_i915_gem_execbuffer2 execbuf = { };
-		struct drm_i915_gem_exec_object2 obj = { };
-		uint32_t bbe = MI_BATCH_BUFFER_END;
+		uint32_t ctx = context_create_safe(fd);
 		igt_spin_t *hang;
 		unsigned int i;
-		uint32_t ctx;
 
 		gem_quiescent_gpu(fd);
 
 		igt_require(i915_reset_control(flags & TEST_WEDGE ?
 					       false : true));
 
-		ctx = context_create_safe(fd);
-
 		/*
 		 * Start executing a spin batch with some queued batches
 		 * against a different context after it.
 		 */
 		hang = spin_sync(fd, ctx0, engine);
 
-		obj.handle = gem_create(fd, 4096);
-		gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
+		execbuf.rsvd1 = ctx;
+		for (i = 0; i < 10; i++)
+			gem_execbuf(fd, &execbuf);
 
-		execbuf.buffers_ptr = to_user_pointer(&obj);
-		execbuf.buffer_count = 1;
 		execbuf.rsvd1 = ctx0;
-		execbuf.flags = engine;
-
 		for (i = 0; i < 10; i++)
 			gem_execbuf(fd, &execbuf);
 
@@ -706,8 +709,9 @@ static void reset_stress(int fd,
 		gem_sync(fd, obj.handle);
 		igt_spin_batch_free(fd, hang);
 		gem_context_destroy(fd, ctx);
-		gem_close(fd, obj.handle);
 	}
+
+	gem_close(fd, obj.handle);
 }
 
 /*
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] igt/gem_eio: Preserve batch between reset-stress iterations

[Tvrtko Ursulin]

On 08/08/2018 12:31, Chris Wilson wrote:
> We can keep the original batch around and avoid recreating it between
> reset iterations to focus on the impact of resets.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> ---
>   tests/gem_eio.c | 30 +++++++++++++++++-------------
>   1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/gem_eio.c b/tests/gem_eio.c
> index de161332d..5250a414c 100644
> --- a/tests/gem_eio.c
> +++ b/tests/gem_eio.c
> @@ -650,35 +650,38 @@ static void reset_stress(int fd,
>   			 uint32_t ctx0, unsigned int engine,
>   			 unsigned int flags)
>   {
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
> +	struct drm_i915_gem_exec_object2 obj = {
> +		.handle = gem_create(fd, 4096)
> +	};
> +	struct drm_i915_gem_execbuffer2 execbuf = {
> +		.buffers_ptr = to_user_pointer(&obj),
> +		.buffer_count = 1,
> +		.flags = engine,
> +	};
> +	gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
> +
>   	igt_until_timeout(5) {
> -		struct drm_i915_gem_execbuffer2 execbuf = { };
> -		struct drm_i915_gem_exec_object2 obj = { };
> -		uint32_t bbe = MI_BATCH_BUFFER_END;
> +		uint32_t ctx = context_create_safe(fd);

There is still this per loop...

>   		igt_spin_t *hang;
>   		unsigned int i;
> -		uint32_t ctx;
>   
>   		gem_quiescent_gpu(fd);
>   
>   		igt_require(i915_reset_control(flags & TEST_WEDGE ?
>   					       false : true));
>   
> -		ctx = context_create_safe(fd);
> -
>   		/*
>   		 * Start executing a spin batch with some queued batches
>   		 * against a different context after it.
>   		 */
>   		hang = spin_sync(fd, ctx0, engine);

... and a ton of operations in this one, so I wonder why bother with one 
batch?

>   
> -		obj.handle = gem_create(fd, 4096);
> -		gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
> +		execbuf.rsvd1 = ctx;
> +		for (i = 0; i < 10; i++)
> +			gem_execbuf(fd, &execbuf);
>   
> -		execbuf.buffers_ptr = to_user_pointer(&obj);
> -		execbuf.buffer_count = 1;
>   		execbuf.rsvd1 = ctx0;
> -		execbuf.flags = engine;
> -
>   		for (i = 0; i < 10; i++)
>   			gem_execbuf(fd, &execbuf);
>   
> @@ -706,8 +709,9 @@ static void reset_stress(int fd,
>   		gem_sync(fd, obj.handle);
>   		igt_spin_batch_free(fd, hang);
>   		gem_context_destroy(fd, ctx);
> -		gem_close(fd, obj.handle);
>   	}
> +
> +	gem_close(fd, obj.handle);
>   }
>   
>   /*
> 

But anyway - no technical complaints:

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

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] igt/gem_eio: Preserve batch between reset-stress iterations

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-08-08 13:38:53)
> 
> On 08/08/2018 12:31, Chris Wilson wrote:
> > We can keep the original batch around and avoid recreating it between
> > reset iterations to focus on the impact of resets.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > ---
> >   tests/gem_eio.c | 30 +++++++++++++++++-------------
> >   1 file changed, 17 insertions(+), 13 deletions(-)
> > 
> > diff --git a/tests/gem_eio.c b/tests/gem_eio.c
> > index de161332d..5250a414c 100644
> > --- a/tests/gem_eio.c
> > +++ b/tests/gem_eio.c
> > @@ -650,35 +650,38 @@ static void reset_stress(int fd,
> >                        uint32_t ctx0, unsigned int engine,
> >                        unsigned int flags)
> >   {
> > +     const uint32_t bbe = MI_BATCH_BUFFER_END;
> > +     struct drm_i915_gem_exec_object2 obj = {
> > +             .handle = gem_create(fd, 4096)
> > +     };
> > +     struct drm_i915_gem_execbuffer2 execbuf = {
> > +             .buffers_ptr = to_user_pointer(&obj),
> > +             .buffer_count = 1,
> > +             .flags = engine,
> > +     };
> > +     gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
> > +
> >       igt_until_timeout(5) {
> > -             struct drm_i915_gem_execbuffer2 execbuf = { };
> > -             struct drm_i915_gem_exec_object2 obj = { };
> > -             uint32_t bbe = MI_BATCH_BUFFER_END;
> > +             uint32_t ctx = context_create_safe(fd);
> 
> There is still this per loop...

I thought that was intentional :)

It felt like the spirit of the test to try and mix up the contexts as
much as possible; one constant, one fresh.

> >               igt_spin_t *hang;
> >               unsigned int i;
> > -             uint32_t ctx;
> >   
> >               gem_quiescent_gpu(fd);
> >   
> >               igt_require(i915_reset_control(flags & TEST_WEDGE ?
> >                                              false : true));
> >   
> > -             ctx = context_create_safe(fd);
> > -
> >               /*
> >                * Start executing a spin batch with some queued batches
> >                * against a different context after it.
> >                */
> >               hang = spin_sync(fd, ctx0, engine);
> 
> ... and a ton of operations in this one, so I wonder why bother with one 
> batch?

Because I don't have spin_sync() in my pattern recognition matrix yet.
One excuse is that it doesn't have any create verb in its name, so easy
to forget its hidden costs.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [Intel-gfx] [PATCH i-g-t] igt/gem_eio: Preserve batch between reset-stress iterations

[Tvrtko Ursulin]

On 08/08/2018 13:47, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-08-08 13:38:53)
>>
>> On 08/08/2018 12:31, Chris Wilson wrote:
>>> We can keep the original batch around and avoid recreating it between
>>> reset iterations to focus on the impact of resets.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>>> ---
>>>    tests/gem_eio.c | 30 +++++++++++++++++-------------
>>>    1 file changed, 17 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tests/gem_eio.c b/tests/gem_eio.c
>>> index de161332d..5250a414c 100644
>>> --- a/tests/gem_eio.c
>>> +++ b/tests/gem_eio.c
>>> @@ -650,35 +650,38 @@ static void reset_stress(int fd,
>>>                         uint32_t ctx0, unsigned int engine,
>>>                         unsigned int flags)
>>>    {
>>> +     const uint32_t bbe = MI_BATCH_BUFFER_END;
>>> +     struct drm_i915_gem_exec_object2 obj = {
>>> +             .handle = gem_create(fd, 4096)
>>> +     };
>>> +     struct drm_i915_gem_execbuffer2 execbuf = {
>>> +             .buffers_ptr = to_user_pointer(&obj),
>>> +             .buffer_count = 1,
>>> +             .flags = engine,
>>> +     };
>>> +     gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
>>> +
>>>        igt_until_timeout(5) {
>>> -             struct drm_i915_gem_execbuffer2 execbuf = { };
>>> -             struct drm_i915_gem_exec_object2 obj = { };
>>> -             uint32_t bbe = MI_BATCH_BUFFER_END;
>>> +             uint32_t ctx = context_create_safe(fd);
>>
>> There is still this per loop...
> 
> I thought that was intentional :)
> 
> It felt like the spirit of the test to try and mix up the contexts as
> much as possible; one constant, one fresh.

Yes definitely intentional (required), I was just puzzled why you are 
concerned with removing one gem_create when we have more ioctls in the 
loop anyway.

> 
>>>                igt_spin_t *hang;
>>>                unsigned int i;
>>> -             uint32_t ctx;
>>>    
>>>                gem_quiescent_gpu(fd);
>>>    
>>>                igt_require(i915_reset_control(flags & TEST_WEDGE ?
>>>                                               false : true));
>>>    
>>> -             ctx = context_create_safe(fd);
>>> -
>>>                /*
>>>                 * Start executing a spin batch with some queued batches
>>>                 * against a different context after it.
>>>                 */
>>>                hang = spin_sync(fd, ctx0, engine);
>>
>> ... and a ton of operations in this one, so I wonder why bother with one
>> batch?
> 
> Because I don't have spin_sync() in my pattern recognition matrix yet.
> One excuse is that it doesn't have any create verb in its name, so easy
> to forget its hidden costs.

Okay. :) I blame the poor name on it being local. :)


Regards,

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

[igt-dev] ✓ Fi.CI.BAT: success for igt/gem_eio: Preserve batch between reset-stress iterations

[Patchwork]

== Series Details ==

Series: igt/gem_eio: Preserve batch between reset-stress iterations
URL   : https://patchwork.freedesktop.org/series/47882/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4633 -> IGTPW_1692 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47882/revisions/1/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1692 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106947)
      {fi-icl-u}:         NOTRUN -> INCOMPLETE (fdo#107399)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         NOTRUN -> INCOMPLETE (fdo#103927)
      {fi-icl-u}:         NOTRUN -> DMESG-WARN (fdo#107382) +4

    {igt@kms_psr@primary_page_flip}:
      {fi-icl-u}:         NOTRUN -> FAIL (fdo#107383) +3

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_workarounds:
      fi-whl-u:           DMESG-FAIL (fdo#107292) -> PASS
      fi-kbl-x1275:       DMESG-FAIL (fdo#107292) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   FAIL (fdo#103167) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
  fdo#107382 https://bugs.freedesktop.org/show_bug.cgi?id=107382
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107399 https://bugs.freedesktop.org/show_bug.cgi?id=107399


== Participating hosts (51 -> 48) ==

  Additional (2): fi-icl-u fi-bxt-dsi 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4588 -> IGTPW_1692

  CI_DRM_4633: ea6e3f703e4d234c9c8eaec6c533355c7454ecb6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1692: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1692/
  IGT_4588: 7e5abbe4d9b2129bbbf02be77a70cad3da2ab941 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1692/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] igt/gem_eio: Preserve batch between reset-stress iterations

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-08-08 13:57:56)
> 
> On 08/08/2018 13:47, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-08-08 13:38:53)
> >>
> >> On 08/08/2018 12:31, Chris Wilson wrote:
> >>>        igt_until_timeout(5) {
> >>> -             struct drm_i915_gem_execbuffer2 execbuf = { };
> >>> -             struct drm_i915_gem_exec_object2 obj = { };
> >>> -             uint32_t bbe = MI_BATCH_BUFFER_END;
> >>> +             uint32_t ctx = context_create_safe(fd);
> >>
> >> There is still this per loop...
> > 
> > I thought that was intentional :)
> > 
> > It felt like the spirit of the test to try and mix up the contexts as
> > much as possible; one constant, one fresh.
> 
> Yes definitely intentional (required), I was just puzzled why you are 
> concerned with removing one gem_create when we have more ioctls in the 
> loop anyway.

Low hanging fruit. Besides, I would cache the context objects if someone
would just turn a blind eye.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for igt/gem_eio: Preserve batch between reset-stress iterations

[Patchwork]

== Series Details ==

Series: igt/gem_eio: Preserve batch between reset-stress iterations
URL   : https://patchwork.freedesktop.org/series/47882/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4588_full -> IGTPW_1692_full =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1692_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1692_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47882/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1692_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_eio@reset-stress:
      shard-snb:          PASS -> FAIL

    igt@gem_eio@wait-10ms:
      shard-hsw:          PASS -> FAIL

    
    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in IGTPW_1692_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_await@wide-contexts:
      shard-apl:          PASS -> FAIL (fdo#105900, fdo#106680)

    igt@gem_workarounds@suspend-resume-fd:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665) +1

    igt@kms_cursor_legacy@pipe-b-torture-move:
      shard-snb:          PASS -> DMESG-WARN (fdo#107122)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-pwrite:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925)

    igt@kms_vblank@pipe-c-ts-continuation-idle-hang:
      shard-apl:          PASS -> DMESG-WARN (fdo#106247)

    igt@pm_rpm@gem-execbuf:
      shard-glk:          PASS -> FAIL (fdo#106539)

    igt@pm_rpm@modeset-lpsp:
      shard-glk:          SKIP -> FAIL (fdo#106539)

    igt@testdisplay:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#107093, fdo#103359)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-snb:          INCOMPLETE (fdo#105411, fdo#106886) -> PASS

    igt@gem_softpin@evict-snoop-interruptible:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-snb:          FAIL (fdo#103166) -> PASS

    igt@kms_vblank@pipe-a-ts-continuation-modeset-rpm:
      shard-apl:          FAIL (fdo#106539) -> PASS +1

    igt@pm_rpm@gem-pread:
      shard-glk:          WARN -> PASS

    igt@pm_rpm@modeset-non-lpsp-stress:
      shard-kbl:          FAIL (fdo#106539) -> PASS +1
      shard-hsw:          FAIL (fdo#106539) -> PASS +1
      shard-glk:          FAIL (fdo#106539) -> PASS

    
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106247 https://bugs.freedesktop.org/show_bug.cgi?id=106247
  fdo#106539 https://bugs.freedesktop.org/show_bug.cgi?id=106539
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107093 https://bugs.freedesktop.org/show_bug.cgi?id=107093
  fdo#107122 https://bugs.freedesktop.org/show_bug.cgi?id=107122
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4588 -> IGTPW_1692
    * Linux: CI_DRM_4632 -> CI_DRM_4633

  CI_DRM_4632: 648e2ff1094eabf43613f41d4d719c1a1f555dbb @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4633: ea6e3f703e4d234c9c8eaec6c533355c7454ecb6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1692: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1692/
  IGT_4588: 7e5abbe4d9b2129bbbf02be77a70cad3da2ab941 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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