[igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Katarzyna Dec]

On simulation environment these tests were failing. During debug
we decided that whole binary should be skipped, because there is
no additional coverage in simulation.

Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 tests/i915/gem_exec_parallel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/i915/gem_exec_parallel.c b/tests/i915/gem_exec_parallel.c
index a6fa698e..c6a20c4f 100644
--- a/tests/i915/gem_exec_parallel.c
+++ b/tests/i915/gem_exec_parallel.c
@@ -238,6 +238,8 @@ igt_main
 	};
 	int fd;
 
+	igt_skip_on_simulation();
+
 	igt_fixture {
 		fd = drm_open_driver_master(DRIVER_INTEL);
 		igt_require_gem(fd);
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Chris Wilson]

Quoting Katarzyna Dec (2019-02-08 13:10:37)
> On simulation environment these tests were failing. During debug
> we decided that whole binary should be skipped, because there is
> no additional coverage in simulation.

How did they fail? There are some paths here that are rarely touched
elsewhere.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Katarzyna Dec]

On Fri, Feb 08, 2019 at 01:14:26PM +0000, Chris Wilson wrote:
> Quoting Katarzyna Dec (2019-02-08 13:10:37)
> > On simulation environment these tests were failing. During debug
> > we decided that whole binary should be skipped, because there is
> > no additional coverage in simulation.
> 
> How did they fail? There are some paths here that are rarely touched
> elsewhere.
> -Chris
I like you challenges Chris :)

gem_exec_parallel@*context* were failing with error:

(gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Test assertion failure
function gem_execbuf, file ../lib/ioctl_wrappers.c:605:
(gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Failed assertion:
__gem_execbuf(fd, execbuf) == 0
(gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: error: -28 != 0

and @*fds* with:

(gem_exec_parallel:1315) igt_debugfs-CRITICAL: Last errno: 24, Too many open
files

After discussion with Ursulin Tvrtko we decided that it is redundant to run it
on simulation.
Probably my commit msg was lacking some information.

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

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Chris Wilson]

Quoting Katarzyna Dec (2019-02-08 13:23:59)
> On Fri, Feb 08, 2019 at 01:14:26PM +0000, Chris Wilson wrote:
> > Quoting Katarzyna Dec (2019-02-08 13:10:37)
> > > On simulation environment these tests were failing. During debug
> > > we decided that whole binary should be skipped, because there is
> > > no additional coverage in simulation.
> > 
> > How did they fail? There are some paths here that are rarely touched
> > elsewhere.
> > -Chris
> I like you challenges Chris :)
> 
> gem_exec_parallel@*context* were failing with error:
> 
> (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Test assertion failure
> function gem_execbuf, file ../lib/ioctl_wrappers.c:605:
> (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Failed assertion:
> __gem_execbuf(fd, execbuf) == 0
> (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: error: -28 != 0

Should not happen.
 
> and @*fds* with:
> 
> (gem_exec_parallel:1315) igt_debugfs-CRITICAL: Last errno: 24, Too many open
> files

That is boring, but more indicative of system setup failure. We should
be configuring the system to allow ourselves, as root, to use as many fd
as can fit in memory.

I'd recommend a pass through the basic stress test at least. It won't
take a lot to exercise some unusual conditions.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Katarzyna Dec]

On Fri, Feb 08, 2019 at 01:27:51PM +0000, Chris Wilson wrote:
> Quoting Katarzyna Dec (2019-02-08 13:23:59)
> > On Fri, Feb 08, 2019 at 01:14:26PM +0000, Chris Wilson wrote:
> > > Quoting Katarzyna Dec (2019-02-08 13:10:37)
> > > > On simulation environment these tests were failing. During debug
> > > > we decided that whole binary should be skipped, because there is
> > > > no additional coverage in simulation.
> > > 
> > > How did they fail? There are some paths here that are rarely touched
> > > elsewhere.
> > > -Chris
> > I like you challenges Chris :)
> > 
> > gem_exec_parallel@*context* were failing with error:
> > 
> > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Test assertion failure
> > function gem_execbuf, file ../lib/ioctl_wrappers.c:605:
> > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Failed assertion:
> > __gem_execbuf(fd, execbuf) == 0
> > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: error: -28 != 0
> 
> Should not happen.
But happends and is reproducible on some CI runs.
>  
> > and @*fds* with:
> > 
> > (gem_exec_parallel:1315) igt_debugfs-CRITICAL: Last errno: 24, Too many open
> > files
> 
> That is boring, but more indicative of system setup failure. We should
> be configuring the system to allow ourselves, as root, to use as many fd
> as can fit in memory.
> 
> I'd recommend a pass through the basic stress test at least. It won't
> take a lot to exercise some unusual conditions.
> -Chris

What if we skip all *context* and all *fds* and leave rest as it is?
Will it be resonable in this shape?

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

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Chris Wilson]

Quoting Katarzyna Dec (2019-02-08 13:32:38)
> On Fri, Feb 08, 2019 at 01:27:51PM +0000, Chris Wilson wrote:
> > Quoting Katarzyna Dec (2019-02-08 13:23:59)
> > > On Fri, Feb 08, 2019 at 01:14:26PM +0000, Chris Wilson wrote:
> > > > Quoting Katarzyna Dec (2019-02-08 13:10:37)
> > > > > On simulation environment these tests were failing. During debug
> > > > > we decided that whole binary should be skipped, because there is
> > > > > no additional coverage in simulation.
> > > > 
> > > > How did they fail? There are some paths here that are rarely touched
> > > > elsewhere.
> > > > -Chris
> > > I like you challenges Chris :)
> > > 
> > > gem_exec_parallel@*context* were failing with error:
> > > 
> > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Test assertion failure
> > > function gem_execbuf, file ../lib/ioctl_wrappers.c:605:
> > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Failed assertion:
> > > __gem_execbuf(fd, execbuf) == 0
> > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: error: -28 != 0
> > 
> > Should not happen.
> But happends and is reproducible on some CI runs.

Well fix it. It is not a valid error in this case. ENOSPC is only valid
in the case the user submitted more than could be fitted into the GGTT
(or had an overlap in their softpin). In this instance, it means that
one _context_ impacted another client, and that is simply not allowed.

> > > and @*fds* with:
> > > 
> > > (gem_exec_parallel:1315) igt_debugfs-CRITICAL: Last errno: 24, Too many open
> > > files
> > 
> > That is boring, but more indicative of system setup failure. We should
> > be configuring the system to allow ourselves, as root, to use as many fd
> > as can fit in memory.
> > 
> > I'd recommend a pass through the basic stress test at least. It won't
> > take a lot to exercise some unusual conditions.
> > -Chris
> 
> What if we skip all *context* and all *fds* and leave rest as it is?
> Will it be resonable in this shape?

I would rewrite it such that the threads ran for a certain period of
time rather than iterations; set basic to something small (say 2s) and
left everything else to run for 150s (tends to be the common value I've
used for random stress tests). Then skip everything but the basic-all
test.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Katarzyna Dec]

On Fri, Feb 08, 2019 at 01:42:42PM +0000, Chris Wilson wrote:
> Quoting Katarzyna Dec (2019-02-08 13:32:38)
> > On Fri, Feb 08, 2019 at 01:27:51PM +0000, Chris Wilson wrote:
> > > Quoting Katarzyna Dec (2019-02-08 13:23:59)
> > > > On Fri, Feb 08, 2019 at 01:14:26PM +0000, Chris Wilson wrote:
> > > > > Quoting Katarzyna Dec (2019-02-08 13:10:37)
> > > > > > On simulation environment these tests were failing. During debug
> > > > > > we decided that whole binary should be skipped, because there is
> > > > > > no additional coverage in simulation.
> > > > > 
> > > > > How did they fail? There are some paths here that are rarely touched
> > > > > elsewhere.
> > > > > -Chris
> > > > I like you challenges Chris :)
> > > > 
> > > > gem_exec_parallel@*context* were failing with error:
> > > > 
> > > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Test assertion failure
> > > > function gem_execbuf, file ../lib/ioctl_wrappers.c:605:
> > > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Failed assertion:
> > > > __gem_execbuf(fd, execbuf) == 0
> > > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: error: -28 != 0
> > > 
> > > Should not happen.
> > But happends and is reproducible on some CI runs.
> 
> Well fix it. It is not a valid error in this case. ENOSPC is only valid
> in the case the user submitted more than could be fitted into the GGTT
> (or had an overlap in their softpin). In this instance, it means that
> one _context_ impacted another client, and that is simply not allowed.
>
According to what you are saying we need to debug and fix the issue.
> > > > and @*fds* with:
> > > > 
> > > > (gem_exec_parallel:1315) igt_debugfs-CRITICAL: Last errno: 24, Too many open
> > > > files
> > > 
> > > That is boring, but more indicative of system setup failure. We should
> > > be configuring the system to allow ourselves, as root, to use as many fd
> > > as can fit in memory.
> > > 
> > > I'd recommend a pass through the basic stress test at least. It won't
> > > take a lot to exercise some unusual conditions.
> > > -Chris
> > 
> > What if we skip all *context* and all *fds* and leave rest as it is?
> > Will it be resonable in this shape?
> 
> I would rewrite it such that the threads ran for a certain period of
> time rather than iterations; set basic to something small (say 2s) and
> left everything else to run for 150s (tends to be the common value I've
Is this comment to *fds* testcases or general to whole binary?
Kasia :)
> used for random stress tests). Then skip everything but the basic-all
> test.
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Patchwork]

== Series Details ==

Series: i915/gem_exec_parallel: Add skip_on_simulation to whole binary
URL   : https://patchwork.freedesktop.org/series/56396/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5567 -> IGTPW_2363
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2363 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2363, 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/56396/revisions/1/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@pm_rpm@module-reload:
    - fi-kbl-7500u:       PASS -> DMESG-FAIL

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       PASS -> DMESG-WARN [fdo#107709]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  
#### Possible fixes ####

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

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

  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109383]: https://bugs.freedesktop.org/show_bug.cgi?id=109383


Participating hosts (49 -> 45)
------------------------------

  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


Build changes
-------------

    * IGT: IGT_4815 -> IGTPW_2363

  CI_DRM_5567: 5725867972123d46f4a4d382ed5fa2eb0348b00c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2363: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2363/
  IGT_4815: 947301563259726b65ce47d3a3fe37931ed42efe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t] i915/gem_exec_parallel: Add skip_on_simulation to whole binary

[Chris Wilson]

Quoting Katarzyna Dec (2019-02-08 13:49:28)
> On Fri, Feb 08, 2019 at 01:42:42PM +0000, Chris Wilson wrote:
> > Quoting Katarzyna Dec (2019-02-08 13:32:38)
> > > On Fri, Feb 08, 2019 at 01:27:51PM +0000, Chris Wilson wrote:
> > > > Quoting Katarzyna Dec (2019-02-08 13:23:59)
> > > > > On Fri, Feb 08, 2019 at 01:14:26PM +0000, Chris Wilson wrote:
> > > > > > Quoting Katarzyna Dec (2019-02-08 13:10:37)
> > > > > > > On simulation environment these tests were failing. During debug
> > > > > > > we decided that whole binary should be skipped, because there is
> > > > > > > no additional coverage in simulation.
> > > > > > 
> > > > > > How did they fail? There are some paths here that are rarely touched
> > > > > > elsewhere.
> > > > > > -Chris
> > > > > I like you challenges Chris :)
> > > > > 
> > > > > gem_exec_parallel@*context* were failing with error:
> > > > > 
> > > > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Test assertion failure
> > > > > function gem_execbuf, file ../lib/ioctl_wrappers.c:605:
> > > > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: Failed assertion:
> > > > > __gem_execbuf(fd, execbuf) == 0
> > > > > (gem_exec_parallel:1245) ioctl_wrappers-CRITICAL: error: -28 != 0
> > > > 
> > > > Should not happen.
> > > But happends and is reproducible on some CI runs.
> > 
> > Well fix it. It is not a valid error in this case. ENOSPC is only valid
> > in the case the user submitted more than could be fitted into the GGTT
> > (or had an overlap in their softpin). In this instance, it means that
> > one _context_ impacted another client, and that is simply not allowed.
> >
> According to what you are saying we need to debug and fix the issue.

Yes, it'll be either unable to pin the context image or allocate the id.
Both of which should invoke a GC to recover resources (no client should
have a permanent allocation, mere temporarily leases) rather than fail
with ENOSPC, 

> > > > > and @*fds* with:
> > > > > 
> > > > > (gem_exec_parallel:1315) igt_debugfs-CRITICAL: Last errno: 24, Too many open
> > > > > files
> > > > 
> > > > That is boring, but more indicative of system setup failure. We should
> > > > be configuring the system to allow ourselves, as root, to use as many fd
> > > > as can fit in memory.
> > > > 
> > > > I'd recommend a pass through the basic stress test at least. It won't
> > > > take a lot to exercise some unusual conditions.
> > > > -Chris
> > > 
> > > What if we skip all *context* and all *fds* and leave rest as it is?
> > > Will it be resonable in this shape?
> > 
> > I would rewrite it such that the threads ran for a certain period of
> > time rather than iterations; set basic to something small (say 2s) and
> > left everything else to run for 150s (tends to be the common value I've
> Is this comment to *fds* testcases or general to whole binary?

Whole binary; I think it's useful enough to run at least 2 passes with a
1024 or more competing threads (more!) because this test has proven
itself to be an early warning sign of problems that have escaped the
"purpose built" tests. It doesn't have to be in the first round of
testing, and it doesn't have to be extensive, just enough to load the
system up to as much as it can handle and check it doesn't fall over.

It would be nice to extend the normal test out to huge number of threads
(even say 16,000 but there are realistic limits to both the number of fd
and contexts that can fit in memory that need to be determined to find
the ideal number).
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev