* [igt-dev] [PATCH i-g-t 1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork
@ 2019-02-07 14:57 Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 2/4] lib/tests: make sure igt_skip in igt_fork is forbidden Daniel Vetter
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Daniel Vetter @ 2019-02-07 14:57 UTC (permalink / raw)
To: IGT development; +Cc: Daniel Vetter
Note that without the igt_waitchildren nothing at all gets forwarded,
maybe we should check for left-behind children somewhere on subtest
exit.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
lib/tests/igt_fork.c | 87 +++++++++++++++++++++++++++++++++++++++++++
lib/tests/meson.build | 1 +
2 files changed, 88 insertions(+)
create mode 100644 lib/tests/igt_fork.c
diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
new file mode 100644
index 000000000000..d495c7815e06
--- /dev/null
+++ b/lib/tests/igt_fork.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "igt_core.h"
+
+/*
+ * We need to hide assert from the cocci igt test refactor spatch.
+ *
+ * IMPORTANT: Test infrastructure tests are the only valid places where using
+ * assert is allowed.
+ */
+#define internal_assert assert
+
+char test[] = "test";
+char *argv_run[] = { test };
+
+static void igt_fork_vs_assert(void)
+{
+ igt_fork(i, 1) {
+ igt_assert(0);
+ }
+
+ igt_waitchildren();
+}
+
+static int do_fork(void (*test_to_run)(void))
+{
+ int pid, status;
+ int argc;
+
+ switch (pid = fork()) {
+ case -1:
+ internal_assert(0);
+ case 0:
+ argc = 1;
+ igt_simple_init(argc, argv_run);
+ test_to_run();
+ igt_exit();
+ default:
+ while (waitpid(pid, &status, 0) == -1 &&
+ errno == EINTR)
+ ;
+
+ if(WIFSIGNALED(status))
+ return WTERMSIG(status) + 128;
+
+ return WEXITSTATUS(status);
+ }
+}
+
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ ret = do_fork(igt_fork_vs_assert);
+ internal_assert(ret == IGT_EXIT_FAILURE);
+}
diff --git a/lib/tests/meson.build b/lib/tests/meson.build
index 55ab2b3d2dff..665ad4a0fbcc 100644
--- a/lib/tests/meson.build
+++ b/lib/tests/meson.build
@@ -1,5 +1,6 @@
lib_tests = [
'igt_fork_helper',
+ 'igt_fork',
'igt_list_only',
'igt_simulation',
'igt_stats',
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [igt-dev] [PATCH i-g-t 2/4] lib/tests: make sure igt_skip in igt_fork is forbidden
2019-02-07 14:57 [igt-dev] [PATCH i-g-t 1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork Daniel Vetter
@ 2019-02-07 14:57 ` Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout Daniel Vetter
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2019-02-07 14:57 UTC (permalink / raw)
To: IGT development; +Cc: Daniel Vetter
Another corner case to check.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
lib/tests/igt_fork.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
index d495c7815e06..5ff2956dd0ab 100644
--- a/lib/tests/igt_fork.c
+++ b/lib/tests/igt_fork.c
@@ -43,6 +43,15 @@
char test[] = "test";
char *argv_run[] = { test };
+static void igt_fork_vs_skip(void)
+{
+ igt_fork(i, 1) {
+ igt_skip("skipping");
+ }
+
+ igt_waitchildren();
+}
+
static void igt_fork_vs_assert(void)
{
igt_fork(i, 1) {
@@ -82,6 +91,11 @@ int main(int argc, char **argv)
{
int ret;
+ /* check that igt_assert is forwarded */
ret = do_fork(igt_fork_vs_assert);
internal_assert(ret == IGT_EXIT_FAILURE);
+
+ /* check that igt_skip within a fork blows up */
+ ret = do_fork(igt_fork_vs_skip);
+ internal_assert(ret == SIGABRT + 128);
}
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout
2019-02-07 14:57 [igt-dev] [PATCH i-g-t 1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 2/4] lib/tests: make sure igt_skip in igt_fork is forbidden Daniel Vetter
@ 2019-02-07 14:57 ` Daniel Vetter
2019-02-07 15:20 ` Chris Wilson
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 4/4] lib/tests: make sure we catch igt_fork leaks Daniel Vetter
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2019-02-07 14:57 UTC (permalink / raw)
To: IGT development; +Cc: Daniel Vetter
Instead of cleaning up the mess in igt_exit make sure we don't even
let it out of the container. See also
commit 754876378d6c9b2775e8c07b4d16f9878c55949f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri Feb 26 22:11:10 2016 +0000
igt/gem_sync: Enforce a timeout of 20s
which added this helper.
To make sure that everyone follows the rules, add an assert.
We're keeping the cleanup code as a failsafe, and because it speeds
up the testcase I'm following up with.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
lib/igt_core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 49fbf70deb06..a7105f0591fc 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1525,6 +1525,7 @@ void igt_exit(void)
for (int c = 0; c < num_test_children; c++)
kill(test_children[c], SIGKILL);
+ assert(!num_test_children);
if (!test_with_subtests) {
struct timespec now;
@@ -1842,6 +1843,11 @@ void igt_waitchildren_timeout(int seconds, const char *reason)
igt_set_timeout(seconds, reason);
igt_waitchildren();
igt_reset_timeout();
+
+ for (int c = 0; c < num_test_children; c++)
+ kill(test_children[c], SIGKILL);
+
+ __igt_waitchildren();
}
/* exit handler code */
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [igt-dev] [PATCH i-g-t 4/4] lib/tests: make sure we catch igt_fork leaks
2019-02-07 14:57 [igt-dev] [PATCH i-g-t 1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 2/4] lib/tests: make sure igt_skip in igt_fork is forbidden Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout Daniel Vetter
@ 2019-02-07 14:57 ` Daniel Vetter
2019-02-07 15:07 ` [igt-dev] [PATCH i-g-t] " Daniel Vetter
2019-02-07 15:51 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork (rev2) Patchwork
2019-02-07 17:06 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
4 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2019-02-07 14:57 UTC (permalink / raw)
To: IGT development; +Cc: Daniel Vetter
Tests have to call igt_waitchildren(_timeout) before they finish
(either with success or an igt_fail/assert/skip/whatever).
Make sure we catch this.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
lib/tests/igt_fork.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
index 5ff2956dd0ab..94b937d1d0fe 100644
--- a/lib/tests/igt_fork.c
+++ b/lib/tests/igt_fork.c
@@ -61,6 +61,13 @@ static void igt_fork_vs_assert(void)
igt_waitchildren();
}
+static void igt_fork_leak(void)
+{
+ igt_fork(i, 1) {
+ sleep(10);
+ }
+}
+
static int do_fork(void (*test_to_run)(void))
{
int pid, status;
@@ -98,4 +105,8 @@ int main(int argc, char **argv)
/* check that igt_skip within a fork blows up */
ret = do_fork(igt_fork_vs_skip);
internal_assert(ret == SIGABRT + 128);
+
+ /* check that failure to clean up fails */
+ ret = do_fork(igt_fork_leak);
+ internal_assert(ret == SIGABRT + 128);
}
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [igt-dev] [PATCH i-g-t] lib/tests: make sure we catch igt_fork leaks
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 4/4] lib/tests: make sure we catch igt_fork leaks Daniel Vetter
@ 2019-02-07 15:07 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2019-02-07 15:07 UTC (permalink / raw)
To: IGT development; +Cc: Daniel Vetter
Tests have to call igt_waitchildren(_timeout) before they finish
(either with success or an igt_fail/assert/skip/whatever).
Make sure we catch this.
v2: Another testcase to make sure igt_waitchildren_timeout cleans up.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
lib/tests/igt_fork.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
index 5ff2956dd0ab..0ea22ba30bb9 100644
--- a/lib/tests/igt_fork.c
+++ b/lib/tests/igt_fork.c
@@ -61,6 +61,22 @@ static void igt_fork_vs_assert(void)
igt_waitchildren();
}
+static void igt_fork_leak(void)
+{
+ igt_fork(i, 1) {
+ sleep(10);
+ }
+}
+
+static void igt_fork_timeout_leak(void)
+{
+ igt_fork(i, 1) {
+ sleep(10);
+ }
+
+ igt_waitchildren_timeout(1, "library test");
+}
+
static int do_fork(void (*test_to_run)(void))
{
int pid, status;
@@ -98,4 +114,12 @@ int main(int argc, char **argv)
/* check that igt_skip within a fork blows up */
ret = do_fork(igt_fork_vs_skip);
internal_assert(ret == SIGABRT + 128);
+
+ /* check that failure to clean up fails */
+ ret = do_fork(igt_fork_leak);
+ internal_assert(ret == SIGABRT + 128);
+
+ /* check that igt_waitchildren_timeout cleans up*/
+ ret = do_fork(igt_fork_timeout_leak);
+ internal_assert(ret == SIGABRT + 128);
}
--
2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout Daniel Vetter
@ 2019-02-07 15:20 ` Chris Wilson
2019-02-07 16:06 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2019-02-07 15:20 UTC (permalink / raw)
To: Daniel Vetter, IGT development; +Cc: Daniel Vetter
Quoting Daniel Vetter (2019-02-07 14:57:06)
> Instead of cleaning up the mess in igt_exit make sure we don't even
> let it out of the container. See also
>
> commit 754876378d6c9b2775e8c07b4d16f9878c55949f
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Fri Feb 26 22:11:10 2016 +0000
>
> igt/gem_sync: Enforce a timeout of 20s
>
> which added this helper.
>
> To make sure that everyone follows the rules, add an assert.
>
> We're keeping the cleanup code as a failsafe, and because it speeds
> up the testcase I'm following up with.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> lib/igt_core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 49fbf70deb06..a7105f0591fc 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1525,6 +1525,7 @@ void igt_exit(void)
>
> for (int c = 0; c < num_test_children; c++)
> kill(test_children[c], SIGKILL);
> + assert(!num_test_children);
>
> if (!test_with_subtests) {
> struct timespec now;
> @@ -1842,6 +1843,11 @@ void igt_waitchildren_timeout(int seconds, const char *reason)
> igt_set_timeout(seconds, reason);
> igt_waitchildren();
> igt_reset_timeout();
> +
> + for (int c = 0; c < num_test_children; c++)
> + kill(test_children[c], SIGKILL);
> +
> + __igt_waitchildren();
How did we escape?
If the SIGALRM fired, we hit igt_fail() in the parent with children in
tow, otherwise we completed igt_waitchildren() successfully and cleaned
everything up.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 10+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork (rev2)
2019-02-07 14:57 [igt-dev] [PATCH i-g-t 1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork Daniel Vetter
` (2 preceding siblings ...)
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 4/4] lib/tests: make sure we catch igt_fork leaks Daniel Vetter
@ 2019-02-07 15:51 ` Patchwork
2019-02-07 17:06 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-02-07 15:51 UTC (permalink / raw)
To: Daniel Vetter; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork (rev2)
URL : https://patchwork.freedesktop.org/series/56344/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5560 -> IGTPW_2352
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/56344/revisions/2/mbox/
Known issues
------------
Here are the changes found in IGTPW_2352 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_execlists:
- fi-apl-guc: PASS -> INCOMPLETE [fdo#103927]
* igt@i915_selftest@live_hangcheck:
- fi-kbl-7560u: PASS -> INCOMPLETE [fdo#108044] / [fdo#108744]
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7500u: NOTRUN -> DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105079] / [fdo#105602]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@pm_rpm@basic-rte:
- fi-bsw-kefka: PASS -> FAIL [fdo#108800]
* igt@prime_vgem@basic-fence-flip:
- fi-gdg-551: PASS -> FAIL [fdo#103182]
#### Possible fixes ####
* igt@i915_selftest@live_hangcheck:
- fi-skl-gvtdvm: INCOMPLETE [fdo#105600] / [fdo#108744] -> PASS
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
- fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS
* igt@pm_rpm@basic-pci-d3-state:
- fi-byt-j1900: {SKIP} [fdo#109271] -> PASS
* igt@pm_rpm@basic-rte:
- fi-byt-j1900: FAIL [fdo#108800] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
[fdo#105600]: https://bugs.freedesktop.org/show_bug.cgi?id=105600
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108044]: https://bugs.freedesktop.org/show_bug.cgi?id=108044
[fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109527]: https://bugs.freedesktop.org/show_bug.cgi?id=109527
Participating hosts (48 -> 43)
------------------------------
Additional (1): fi-kbl-7500u
Missing (6): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ivb-3770 fi-pnv-d510 fi-bdw-samus
Build changes
-------------
* IGT: IGT_4813 -> IGTPW_2352
CI_DRM_5560: b3cc6ae9211bbf4e40195469147a884b74c0ec8e @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2352: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2352/
IGT_4813: 09f506726d0e115ee7f4a1604ae71adcf9f12690 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2352/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout
2019-02-07 15:20 ` Chris Wilson
@ 2019-02-07 16:06 ` Daniel Vetter
2019-02-07 16:34 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2019-02-07 16:06 UTC (permalink / raw)
To: Chris Wilson; +Cc: IGT development, Daniel Vetter
On Thu, Feb 07, 2019 at 03:20:37PM +0000, Chris Wilson wrote:
> Quoting Daniel Vetter (2019-02-07 14:57:06)
> > Instead of cleaning up the mess in igt_exit make sure we don't even
> > let it out of the container. See also
> >
> > commit 754876378d6c9b2775e8c07b4d16f9878c55949f
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date: Fri Feb 26 22:11:10 2016 +0000
> >
> > igt/gem_sync: Enforce a timeout of 20s
> >
> > which added this helper.
> >
> > To make sure that everyone follows the rules, add an assert.
> >
> > We're keeping the cleanup code as a failsafe, and because it speeds
> > up the testcase I'm following up with.
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > lib/igt_core.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index 49fbf70deb06..a7105f0591fc 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -1525,6 +1525,7 @@ void igt_exit(void)
> >
> > for (int c = 0; c < num_test_children; c++)
> > kill(test_children[c], SIGKILL);
> > + assert(!num_test_children);
> >
> > if (!test_with_subtests) {
> > struct timespec now;
> > @@ -1842,6 +1843,11 @@ void igt_waitchildren_timeout(int seconds, const char *reason)
> > igt_set_timeout(seconds, reason);
> > igt_waitchildren();
> > igt_reset_timeout();
> > +
> > + for (int c = 0; c < num_test_children; c++)
> > + kill(test_children[c], SIGKILL);
> > +
> > + __igt_waitchildren();
>
> How did we escape?
>
> If the SIGALRM fired, we hit igt_fail() in the parent with children in
> tow, otherwise we completed igt_waitchildren() successfully and cleaned
> everything up.
Hm right, this does nothing.
The motivation for this is that I noticed that without an explicit
igt_waitchildren() the igt_assert forwarding doesn't work. So I've tried
to catch that kind of leaking.
The trouble that does happen is if you run multiple tests, then the
children do leak into the next subtest (since igt_exit is only called at
the very end). That's the usual trouble with our exit handlers not
properly stacking. In the end I wanted to put a wait() into igt_exit and
check that it gives us ECHILD, to make sure everything is cleaned up.
So not sure what to do ... any ideas?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout
2019-02-07 16:06 ` Daniel Vetter
@ 2019-02-07 16:34 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2019-02-07 16:34 UTC (permalink / raw)
To: Chris Wilson; +Cc: IGT development, Daniel Vetter
On Thu, Feb 07, 2019 at 05:06:55PM +0100, Daniel Vetter wrote:
> On Thu, Feb 07, 2019 at 03:20:37PM +0000, Chris Wilson wrote:
> > Quoting Daniel Vetter (2019-02-07 14:57:06)
> > > Instead of cleaning up the mess in igt_exit make sure we don't even
> > > let it out of the container. See also
> > >
> > > commit 754876378d6c9b2775e8c07b4d16f9878c55949f
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date: Fri Feb 26 22:11:10 2016 +0000
> > >
> > > igt/gem_sync: Enforce a timeout of 20s
> > >
> > > which added this helper.
> > >
> > > To make sure that everyone follows the rules, add an assert.
> > >
> > > We're keeping the cleanup code as a failsafe, and because it speeds
> > > up the testcase I'm following up with.
> > >
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > > lib/igt_core.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > index 49fbf70deb06..a7105f0591fc 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -1525,6 +1525,7 @@ void igt_exit(void)
> > >
> > > for (int c = 0; c < num_test_children; c++)
> > > kill(test_children[c], SIGKILL);
> > > + assert(!num_test_children);
> > >
> > > if (!test_with_subtests) {
> > > struct timespec now;
> > > @@ -1842,6 +1843,11 @@ void igt_waitchildren_timeout(int seconds, const char *reason)
> > > igt_set_timeout(seconds, reason);
> > > igt_waitchildren();
> > > igt_reset_timeout();
> > > +
> > > + for (int c = 0; c < num_test_children; c++)
> > > + kill(test_children[c], SIGKILL);
> > > +
> > > + __igt_waitchildren();
> >
> > How did we escape?
> >
> > If the SIGALRM fired, we hit igt_fail() in the parent with children in
> > tow, otherwise we completed igt_waitchildren() successfully and cleaned
> > everything up.
>
> Hm right, this does nothing.
>
> The motivation for this is that I noticed that without an explicit
> igt_waitchildren() the igt_assert forwarding doesn't work. So I've tried
> to catch that kind of leaking.
>
> The trouble that does happen is if you run multiple tests, then the
> children do leak into the next subtest (since igt_exit is only called at
> the very end). That's the usual trouble with our exit handlers not
> properly stacking. In the end I wanted to put a wait() into igt_exit and
> check that it gives us ECHILD, to make sure everything is cleaned up.
>
> So not sure what to do ... any ideas?
I realized my testcase was also testing for the wrong thing, so resending
both.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 10+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork (rev2)
2019-02-07 14:57 [igt-dev] [PATCH i-g-t 1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork Daniel Vetter
` (3 preceding siblings ...)
2019-02-07 15:51 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork (rev2) Patchwork
@ 2019-02-07 17:06 ` Patchwork
4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-02-07 17:06 UTC (permalink / raw)
To: Daniel Vetter; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork (rev2)
URL : https://patchwork.freedesktop.org/series/56344/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5560_full -> IGTPW_2352_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/56344/revisions/2/mbox/
Known issues
------------
Here are the changes found in IGTPW_2352_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_available_modes_crc@available_mode_test_crc:
- shard-kbl: PASS -> FAIL [fdo#106641]
* igt@kms_color@pipe-c-ctm-max:
- shard-apl: PASS -> FAIL [fdo#108147]
* igt@kms_cursor_crc@cursor-256x256-sliding:
- shard-glk: PASS -> FAIL [fdo#103232] +5
* igt@kms_cursor_crc@cursor-256x85-onscreen:
- shard-apl: PASS -> FAIL [fdo#103232] +4
* igt@kms_cursor_crc@cursor-alpha-opaque:
- shard-glk: PASS -> FAIL [fdo#109350]
* igt@kms_cursor_crc@cursor-size-change:
- shard-kbl: PASS -> FAIL [fdo#103232] +1
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
- shard-hsw: PASS -> FAIL [fdo#105767]
* igt@kms_flip@flip-vs-fences:
- shard-snb: PASS -> INCOMPLETE [fdo#105411] +1
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
- shard-apl: PASS -> FAIL [fdo#103167] +2
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
- shard-glk: PASS -> FAIL [fdo#103167] +7
* igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
- shard-apl: PASS -> FAIL [fdo#108948] +1
* igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
- shard-glk: PASS -> FAIL [fdo#103166] +6
* igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
- shard-apl: PASS -> FAIL [fdo#103166] +2
* igt@kms_setmode@basic:
- shard-hsw: PASS -> FAIL [fdo#99912]
- shard-snb: NOTRUN -> FAIL [fdo#99912]
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-glk: PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]
#### Possible fixes ####
* igt@gem_ctx_isolation@vcs0-s3:
- shard-kbl: INCOMPLETE [fdo#103665] -> PASS
* igt@gem_mmap_gtt@hang:
- shard-glk: FAIL [fdo#109469] -> PASS
* igt@kms_cursor_crc@cursor-256x256-suspend:
- shard-apl: FAIL [fdo#103191] / [fdo#103232] -> PASS
* igt@kms_cursor_crc@cursor-256x85-onscreen:
- shard-glk: FAIL [fdo#103232] -> PASS +2
* igt@kms_cursor_crc@cursor-64x64-sliding:
- shard-apl: FAIL [fdo#103232] -> PASS +2
- shard-kbl: FAIL [fdo#103232] -> PASS
* igt@kms_cursor_crc@cursor-alpha-opaque:
- shard-apl: FAIL [fdo#109350] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt:
- shard-snb: {SKIP} [fdo#109271] -> PASS +1
* igt@kms_plane@plane-position-covered-pipe-c-planes:
- shard-apl: FAIL [fdo#103166] -> PASS +1
- shard-glk: FAIL [fdo#103166] -> PASS
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-glk: FAIL [fdo#108145] -> PASS +2
* igt@kms_setmode@basic:
- shard-glk: FAIL [fdo#99912] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
[fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
[fdo#109469]: https://bugs.freedesktop.org/show_bug.cgi?id=109469
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (7 -> 5)
------------------------------
Missing (2): shard-skl shard-iclb
Build changes
-------------
* IGT: IGT_4813 -> IGTPW_2352
* Piglit: piglit_4509 -> None
CI_DRM_5560: b3cc6ae9211bbf4e40195469147a884b74c0ec8e @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2352: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2352/
IGT_4813: 09f506726d0e115ee7f4a1604ae71adcf9f12690 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2352/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-02-07 17:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 14:57 [igt-dev] [PATCH i-g-t 1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 2/4] lib/tests: make sure igt_skip in igt_fork is forbidden Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 3/4] lib: Don't leak children in igt_waitchildren_timeout Daniel Vetter
2019-02-07 15:20 ` Chris Wilson
2019-02-07 16:06 ` Daniel Vetter
2019-02-07 16:34 ` Daniel Vetter
2019-02-07 14:57 ` [igt-dev] [PATCH i-g-t 4/4] lib/tests: make sure we catch igt_fork leaks Daniel Vetter
2019-02-07 15:07 ` [igt-dev] [PATCH i-g-t] " Daniel Vetter
2019-02-07 15:51 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/4] lib/tests: Check that igt_assert forwards correctly through igt_fork (rev2) Patchwork
2019-02-07 17:06 ` [igt-dev] ✓ 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.