* [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice
@ 2019-07-09 12:23 Arkadiusz Hiler
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 2/3] runner: Warn when watchdogs are being closed from the exit handler Arkadiusz Hiler
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Arkadiusz Hiler @ 2019-07-09 12:23 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala
Setting the watchdog fd lists to NULL for extra fireworks if accessed
unintentionally.
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
runner/executor.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/runner/executor.c b/runner/executor.c
index 7a7e29d9..af02c5a8 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -46,6 +46,10 @@ static void close_watchdogs(struct settings *settings)
for (i = 0; i < watchdogs.num_dogs; i++) {
__close_watchdog(watchdogs.fds[i]);
}
+
+ free(watchdogs.fds);
+ watchdogs.num_dogs = 0;
+ watchdogs.fds = NULL;
}
static void close_watchdogs_atexit(void)
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [igt-dev] [PATCH i-g-t 2/3] runner: Warn when watchdogs are being closed from the exit handler
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
@ 2019-07-09 12:23 ` Arkadiusz Hiler
2019-07-10 8:20 ` Ser, Simon
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals Arkadiusz Hiler
` (6 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Arkadiusz Hiler @ 2019-07-09 12:23 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala
instead of being closed normally on a graceful code path
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
runner/executor.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/runner/executor.c b/runner/executor.c
index af02c5a8..6463ab96 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -43,6 +43,9 @@ static void close_watchdogs(struct settings *settings)
if (settings && settings->log_level >= LOG_LEVEL_VERBOSE)
printf("Closing watchdogs\n");
+ if (settings == NULL && watchdogs.num_dogs != 0)
+ fprintf(stderr, "Closing watchdogs from exit handler!\n");
+
for (i = 0; i < watchdogs.num_dogs; i++) {
__close_watchdog(watchdogs.fds[i]);
}
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 2/3] runner: Warn when watchdogs are being closed from the exit handler Arkadiusz Hiler
@ 2019-07-09 12:23 ` Arkadiusz Hiler
2019-07-18 10:57 ` Ser, Simon
2019-07-19 12:01 ` [igt-dev] [PATCH v2 " Arkadiusz Hiler
2019-07-09 13:04 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice Patchwork
` (5 subsequent siblings)
7 siblings, 2 replies; 15+ messages in thread
From: Arkadiusz Hiler @ 2019-07-09 12:23 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala
There are few short windows of opportunity when watchdogs are primed but
there is no signal handling in place, so the process may exit without
proper shutdown sequence.
This patch rearranges the existing code so that we set up the signalfd
and BLOCK the signals before setting up watchdogs and UNBLOCK only after
the watchdogs are closed properly.
If igt_runner exits due to signal, non-zero status code is returned.
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
runner/executor.c | 100 +++++++++++++++++++++++++++++++++-------------
1 file changed, 73 insertions(+), 27 deletions(-)
diff --git a/runner/executor.c b/runner/executor.c
index 6463ab96..62303ff8 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -7,6 +7,7 @@
#include <string.h>
#include <sys/ioctl.h>
#include <sys/select.h>
+#include <sys/poll.h>
#include <sys/signalfd.h>
#include <sys/stat.h>
#include <sys/time.h>
@@ -604,7 +605,6 @@ static int monitor_output(pid_t child,
close(outfd);
close(errfd);
close(kmsgfd);
- close(sigfd);
return -1;
}
@@ -776,9 +776,8 @@ static int monitor_output(pid_t child,
*time_spent = time;
}
- close(sigfd);
- sigfd = -1;
child = 0;
+ sigfd = -1; /* we are dying, no signal handling for now */
}
}
@@ -790,7 +789,6 @@ static int monitor_output(pid_t child,
close(outfd);
close(errfd);
close(kmsgfd);
- close(sigfd);
if (aborting)
return -1;
@@ -908,13 +906,12 @@ static int execute_next_entry(struct execute_state *state,
double *time_spent,
struct settings *settings,
struct job_list_entry *entry,
- int testdirfd, int resdirfd)
+ int testdirfd, int resdirfd,
+ int sigfd, sigset_t *sigmask)
{
int dirfd;
int outputs[_F_LAST];
int kmsgfd;
- int sigfd;
- sigset_t mask;
int outpipe[2] = { -1, -1 };
int errpipe[2] = { -1, -1 };
int outfd, errfd;
@@ -954,21 +951,6 @@ static int execute_next_entry(struct execute_state *state,
lseek(kmsgfd, 0, SEEK_END);
}
- sigemptyset(&mask);
- sigaddset(&mask, SIGCHLD);
- sigaddset(&mask, SIGINT);
- sigaddset(&mask, SIGTERM);
- sigaddset(&mask, SIGQUIT);
- sigaddset(&mask, SIGHUP);
- sigprocmask(SIG_BLOCK, &mask, NULL);
- sigfd = signalfd(-1, &mask, O_CLOEXEC);
-
- if (sigfd < 0) {
- /* TODO: Handle better */
- fprintf(stderr, "Cannot monitor child process with signalfd\n");
- result = -1;
- goto out_kmsgfd;
- }
if (settings->log_level >= LOG_LEVEL_NORMAL) {
char *displayname;
@@ -1002,7 +984,7 @@ static int execute_next_entry(struct execute_state *state,
close(outpipe[0]);
close(errpipe[0]);
- sigprocmask(SIG_UNBLOCK, &mask, NULL);
+ sigprocmask(SIG_UNBLOCK, sigmask, NULL);
setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
@@ -1261,12 +1243,41 @@ static void oom_immortal(void)
close(fd);
}
+static bool should_die_because_signal(int sigfd)
+{
+ struct signalfd_siginfo siginfo;
+ int ret;
+
+ struct pollfd sigpoll = { .fd = sigfd, .events = POLLIN | POLLRDBAND };
+
+ if ((ret = poll(&sigpoll, 1, 0)) != 0) {
+ if (ret == -1) {
+ }
+
+ ret = read(sigfd, &siginfo, sizeof(siginfo));
+
+ if (siginfo.ssi_signo == SIGCHLD) {
+ fprintf(stderr, "Runner got stray SIGCHLD while not executing any tests.\n");
+
+ } else {
+ fprintf(stderr, "Runner is being killed by %s\n",
+ strsignal(siginfo.ssi_signo));
+ return true;
+ }
+
+ }
+
+ return false;
+}
+
bool execute(struct execute_state *state,
struct settings *settings,
struct job_list *job_list)
{
struct utsname unamebuf;
int resdirfd, testdirfd, unamefd, timefd;
+ sigset_t sigmask;
+ int sigfd;
double time_spent = 0.0;
bool status = true;
@@ -1310,6 +1321,22 @@ bool execute(struct execute_state *state,
oom_immortal();
+ sigemptyset(&sigmask);
+ sigaddset(&sigmask, SIGCHLD);
+ sigaddset(&sigmask, SIGINT);
+ sigaddset(&sigmask, SIGTERM);
+ sigaddset(&sigmask, SIGQUIT);
+ sigaddset(&sigmask, SIGHUP);
+ sigfd = signalfd(-1, &sigmask, O_CLOEXEC);
+ sigprocmask(SIG_BLOCK, &sigmask, NULL);
+
+ if (sigfd < 0) {
+ /* TODO: Handle better */
+ fprintf(stderr, "Cannot mask signals\n");
+ status = -1;
+ goto end;
+ }
+
init_watchdogs(settings);
if (!uname(&unamebuf)) {
@@ -1345,12 +1372,18 @@ bool execute(struct execute_state *state,
char *reason;
int result;
+ if (should_die_because_signal(sigfd)) {
+ status = false;
+ goto end;
+ }
+
result = execute_next_entry(state,
job_list->size,
&time_spent,
settings,
&job_list->entries[state->next],
- testdirfd, resdirfd);
+ testdirfd, resdirfd,
+ sigfd, &sigmask);
if (result < 0) {
status = false;
@@ -1383,8 +1416,15 @@ bool execute(struct execute_state *state,
if (result > 0) {
double time_left = state->time_left;
- close(testdirfd);
close_watchdogs(settings);
+ sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
+ /* make sure that we do not leave any signals unhandled */
+ if (should_die_because_signal(sigfd)){
+ status = false;
+ goto end_post_signal_restore;
+ }
+ close(sigfd);
+ close(testdirfd);
initialize_execute_state_from_resume(resdirfd, state, settings, job_list);
state->time_left = time_left;
return execute(state, settings, job_list);
@@ -1397,8 +1437,14 @@ bool execute(struct execute_state *state,
}
end:
- close(testdirfd);
- close(resdirfd);
close_watchdogs(settings);
+ sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
+ /* make sure that we do not leave any signals unhandled */
+ if (should_die_because_signal(sigfd))
+ status = false;
+ end_post_signal_restore:
+ close(sigfd);
+ close(testdirfd);
+ close(resdirfd);
return status;
}
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 2/3] runner: Warn when watchdogs are being closed from the exit handler Arkadiusz Hiler
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals Arkadiusz Hiler
@ 2019-07-09 13:04 ` Patchwork
2019-07-10 0:24 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-07-09 13:04 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice
URL : https://patchwork.freedesktop.org/series/63439/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6440 -> IGTPW_3251
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/63439/revisions/1/mbox/
Known issues
------------
Here are the changes found in IGTPW_3251 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_mmap_gtt@basic-small-bo-tiledx:
- fi-icl-u3: [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledx.html
* igt@i915_module_load@reload:
- fi-blb-e6850: [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-blb-e6850/igt@i915_module_load@reload.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-blb-e6850/igt@i915_module_load@reload.html
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: [PASS][5] -> [FAIL][6] ([fdo#108511])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [PASS][7] -> [FAIL][8] ([fdo#109485])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
#### Possible fixes ####
* igt@gem_mmap_gtt@basic-short:
- fi-icl-u3: [DMESG-WARN][9] ([fdo#107724]) -> [PASS][10] +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-icl-u3/igt@gem_mmap_gtt@basic-short.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-icl-u3/igt@gem_mmap_gtt@basic-short.html
* igt@i915_selftest@live_contexts:
- fi-skl-iommu: [INCOMPLETE][11] ([fdo#111050]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-skl-iommu/igt@i915_selftest@live_contexts.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-skl-iommu/igt@i915_selftest@live_contexts.html
* igt@kms_chamelium@hdmi-edid-read:
- {fi-icl-u4}: [FAIL][13] ([fdo#111046 ]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html
#### Warnings ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-kbl-guc: [FAIL][15] ([fdo#110829]) -> [SKIP][16] ([fdo#109271])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
{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#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
[fdo#110829]: https://bugs.freedesktop.org/show_bug.cgi?id=110829
[fdo#111046 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111046
[fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
[fdo#111050]: https://bugs.freedesktop.org/show_bug.cgi?id=111050
Participating hosts (51 -> 47)
------------------------------
Additional (2): fi-icl-dsi fi-pnv-d510
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* IGT: IGT_5092 -> IGTPW_3251
CI_DRM_6440: f3ee9eaf13443e179a5ad263da0abe241ea04172 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3251: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/
IGT_5092: 2a66ae6626d5583240509f84117d1345a799b75a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
` (2 preceding siblings ...)
2019-07-09 13:04 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice Patchwork
@ 2019-07-10 0:24 ` Patchwork
2019-07-10 8:18 ` [igt-dev] [PATCH i-g-t 1/3] " Ser, Simon
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-07-10 0:24 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice
URL : https://patchwork.freedesktop.org/series/63439/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6440_full -> IGTPW_3251_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/63439/revisions/1/mbox/
Known issues
------------
Here are the changes found in IGTPW_3251_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_isolation@rcs0-s3:
- shard-glk: [PASS][1] -> [INCOMPLETE][2] ([fdo#103359] / [k.org#198133])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-glk6/igt@gem_ctx_isolation@rcs0-s3.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-glk7/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_eio@in-flight-suspend:
- shard-kbl: [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-kbl2/igt@gem_eio@in-flight-suspend.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-kbl4/igt@gem_eio@in-flight-suspend.html
* igt@gem_exec_balancer@smoke:
- shard-iclb: [PASS][5] -> [SKIP][6] ([fdo#110854])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb1/igt@gem_exec_balancer@smoke.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb8/igt@gem_exec_balancer@smoke.html
* igt@gem_workarounds@suspend-resume:
- shard-apl: [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +2 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-apl6/igt@gem_workarounds@suspend-resume.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-apl1/igt@gem_workarounds@suspend-resume.html
* igt@i915_pm_lpsp@screens-disabled:
- shard-hsw: [PASS][9] -> [FAIL][10] ([fdo#110383])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-hsw2/igt@i915_pm_lpsp@screens-disabled.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-hsw7/igt@i915_pm_lpsp@screens-disabled.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
- shard-glk: [PASS][11] -> [FAIL][12] ([fdo#103167])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-glk9/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-glk4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-render:
- shard-iclb: [PASS][13] -> [INCOMPLETE][14] ([fdo#107713])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-render.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-render.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
- shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103167]) +1 similar issue
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103166])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-y.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109441]) +2 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html
* igt@kms_setmode@basic:
- shard-kbl: [PASS][21] -> [FAIL][22] ([fdo#99912])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-kbl1/igt@kms_setmode@basic.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-kbl4/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-c-ts-continuation-suspend:
- shard-kbl: [PASS][23] -> [INCOMPLETE][24] ([fdo#103665])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-kbl2/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-kbl2/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
#### Possible fixes ####
* igt@gem_exec_nop@basic-series:
- shard-iclb: [INCOMPLETE][25] ([fdo#107713] / [fdo#109100]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb1/igt@gem_exec_nop@basic-series.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb2/igt@gem_exec_nop@basic-series.html
* igt@i915_pm_rc6_residency@rc6-accuracy:
- shard-snb: [SKIP][27] ([fdo#109271]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-snb4/igt@i915_pm_rc6_residency@rc6-accuracy.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-snb1/igt@i915_pm_rc6_residency@rc6-accuracy.html
* igt@i915_suspend@fence-restore-untiled:
- shard-kbl: [DMESG-WARN][29] ([fdo#108566]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-kbl4/igt@i915_suspend@fence-restore-untiled.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-kbl3/igt@i915_suspend@fence-restore-untiled.html
* igt@kms_cursor_crc@pipe-c-cursor-suspend:
- shard-apl: [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +1 similar issue
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-apl8/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
- shard-hsw: [FAIL][33] ([fdo#105767]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-hsw5/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-hsw8/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
* igt@kms_flip@2x-flip-vs-wf_vblank-interruptible:
- shard-hsw: [INCOMPLETE][35] ([fdo#103540]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-hsw7/igt@kms_flip@2x-flip-vs-wf_vblank-interruptible.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-hsw7/igt@kms_flip@2x-flip-vs-wf_vblank-interruptible.html
* igt@kms_flip_tiling@flip-y-tiled:
- shard-iclb: [FAIL][37] ([fdo#108303]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb6/igt@kms_flip_tiling@flip-y-tiled.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb2/igt@kms_flip_tiling@flip-y-tiled.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
- shard-iclb: [FAIL][39] ([fdo#103167]) -> [PASS][40] +5 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html
* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [FAIL][41] ([fdo#103166]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-x.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-x.html
* igt@kms_psr@psr2_primary_render:
- shard-iclb: [SKIP][43] ([fdo#109441]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-iclb4/igt@kms_psr@psr2_primary_render.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-iclb2/igt@kms_psr@psr2_primary_render.html
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-kbl: [FAIL][45] ([fdo#109016]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6440/shard-kbl6/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/shard-kbl1/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html
[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#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110383]: https://bugs.freedesktop.org/show_bug.cgi?id=110383
[fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
[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 (10 -> 6)
------------------------------
Missing (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005
Build changes
-------------
* IGT: IGT_5092 -> IGTPW_3251
* Piglit: piglit_4509 -> None
CI_DRM_6440: f3ee9eaf13443e179a5ad263da0abe241ea04172 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3251: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3251/
IGT_5092: 2a66ae6626d5583240509f84117d1345a799b75a @ 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_3251/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
` (3 preceding siblings ...)
2019-07-10 0:24 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2019-07-10 8:18 ` Ser, Simon
2019-07-19 13:31 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice (rev2) Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Ser, Simon @ 2019-07-10 8:18 UTC (permalink / raw)
To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri
On Tue, 2019-07-09 at 15:23 +0300, Arkadiusz Hiler wrote:
> Setting the watchdog fd lists to NULL for extra fireworks if accessed
> unintentionally.
>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
> ---
> runner/executor.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/runner/executor.c b/runner/executor.c
> index 7a7e29d9..af02c5a8 100644
> --- a/runner/executor.c
> +++ b/runner/executor.c
> @@ -46,6 +46,10 @@ static void close_watchdogs(struct settings *settings)
> for (i = 0; i < watchdogs.num_dogs; i++) {
> __close_watchdog(watchdogs.fds[i]);
> }
> +
> + free(watchdogs.fds);
> + watchdogs.num_dogs = 0;
> + watchdogs.fds = NULL;
> }
>
> static void close_watchdogs_atexit(void)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 2/3] runner: Warn when watchdogs are being closed from the exit handler
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 2/3] runner: Warn when watchdogs are being closed from the exit handler Arkadiusz Hiler
@ 2019-07-10 8:20 ` Ser, Simon
0 siblings, 0 replies; 15+ messages in thread
From: Ser, Simon @ 2019-07-10 8:20 UTC (permalink / raw)
To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri
On Tue, 2019-07-09 at 15:23 +0300, Arkadiusz Hiler wrote:
> instead of being closed normally on a graceful code path
>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
> ---
> runner/executor.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/runner/executor.c b/runner/executor.c
> index af02c5a8..6463ab96 100644
> --- a/runner/executor.c
> +++ b/runner/executor.c
> @@ -43,6 +43,9 @@ static void close_watchdogs(struct settings *settings)
> if (settings && settings->log_level >= LOG_LEVEL_VERBOSE)
> printf("Closing watchdogs\n");
>
> + if (settings == NULL && watchdogs.num_dogs != 0)
> + fprintf(stderr, "Closing watchdogs from exit handler!\n");
> +
> for (i = 0; i < watchdogs.num_dogs; i++) {
> __close_watchdog(watchdogs.fds[i]);
> }
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals Arkadiusz Hiler
@ 2019-07-18 10:57 ` Ser, Simon
2019-07-19 11:25 ` Arkadiusz Hiler
2019-07-19 12:01 ` [igt-dev] [PATCH v2 " Arkadiusz Hiler
1 sibling, 1 reply; 15+ messages in thread
From: Ser, Simon @ 2019-07-18 10:57 UTC (permalink / raw)
To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri
On Tue, 2019-07-09 at 15:23 +0300, Arkadiusz Hiler wrote:
> There are few short windows of opportunity when watchdogs are primed but
> there is no signal handling in place, so the process may exit without
> proper shutdown sequence.
>
> This patch rearranges the existing code so that we set up the signalfd
> and BLOCK the signals before setting up watchdogs and UNBLOCK only after
> the watchdogs are closed properly.
>
> If igt_runner exits due to signal, non-zero status code is returned.
>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
> runner/executor.c | 100 +++++++++++++++++++++++++++++++++-------------
> 1 file changed, 73 insertions(+), 27 deletions(-)
>
> diff --git a/runner/executor.c b/runner/executor.c
> index 6463ab96..62303ff8 100644
> --- a/runner/executor.c
> +++ b/runner/executor.c
> @@ -7,6 +7,7 @@
> #include <string.h>
> #include <sys/ioctl.h>
> #include <sys/select.h>
> +#include <sys/poll.h>
> #include <sys/signalfd.h>
> #include <sys/stat.h>
> #include <sys/time.h>
> @@ -604,7 +605,6 @@ static int monitor_output(pid_t child,
> close(outfd);
> close(errfd);
> close(kmsgfd);
> - close(sigfd);
> return -1;
> }
>
> @@ -776,9 +776,8 @@ static int monitor_output(pid_t child,
> *time_spent = time;
> }
>
> - close(sigfd);
> - sigfd = -1;
> child = 0;
> + sigfd = -1; /* we are dying, no signal handling for now */
> }
> }
>
> @@ -790,7 +789,6 @@ static int monitor_output(pid_t child,
> close(outfd);
> close(errfd);
> close(kmsgfd);
> - close(sigfd);
>
> if (aborting)
> return -1;
> @@ -908,13 +906,12 @@ static int execute_next_entry(struct execute_state *state,
> double *time_spent,
> struct settings *settings,
> struct job_list_entry *entry,
> - int testdirfd, int resdirfd)
> + int testdirfd, int resdirfd,
> + int sigfd, sigset_t *sigmask)
> {
> int dirfd;
> int outputs[_F_LAST];
> int kmsgfd;
> - int sigfd;
> - sigset_t mask;
> int outpipe[2] = { -1, -1 };
> int errpipe[2] = { -1, -1 };
> int outfd, errfd;
> @@ -954,21 +951,6 @@ static int execute_next_entry(struct execute_state *state,
> lseek(kmsgfd, 0, SEEK_END);
> }
>
> - sigemptyset(&mask);
> - sigaddset(&mask, SIGCHLD);
> - sigaddset(&mask, SIGINT);
> - sigaddset(&mask, SIGTERM);
> - sigaddset(&mask, SIGQUIT);
> - sigaddset(&mask, SIGHUP);
> - sigprocmask(SIG_BLOCK, &mask, NULL);
> - sigfd = signalfd(-1, &mask, O_CLOEXEC);
> -
> - if (sigfd < 0) {
> - /* TODO: Handle better */
> - fprintf(stderr, "Cannot monitor child process with signalfd\n");
> - result = -1;
> - goto out_kmsgfd;
> - }
>
> if (settings->log_level >= LOG_LEVEL_NORMAL) {
> char *displayname;
> @@ -1002,7 +984,7 @@ static int execute_next_entry(struct execute_state *state,
> close(outpipe[0]);
> close(errpipe[0]);
>
> - sigprocmask(SIG_UNBLOCK, &mask, NULL);
> + sigprocmask(SIG_UNBLOCK, sigmask, NULL);
>
> setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
>
> @@ -1261,12 +1243,41 @@ static void oom_immortal(void)
> close(fd);
> }
>
> +static bool should_die_because_signal(int sigfd)
> +{
> + struct signalfd_siginfo siginfo;
> + int ret;
> +
> + struct pollfd sigpoll = { .fd = sigfd, .events = POLLIN | POLLRDBAND };
> +
> + if ((ret = poll(&sigpoll, 1, 0)) != 0) {
> + if (ret == -1) {
Seems like this is unintentionally left blank.
> + }
> +
> + ret = read(sigfd, &siginfo, sizeof(siginfo));
Error handling is missing (ret == sizeof(siginfo)).
> + if (siginfo.ssi_signo == SIGCHLD) {
> + fprintf(stderr, "Runner got stray SIGCHLD while not executing any tests.\n");
> +
Nit: extra blank line here
> + } else {
> + fprintf(stderr, "Runner is being killed by %s\n",
> + strsignal(siginfo.ssi_signo));
> + return true;
> + }
> +
> + }
> +
> + return false;
> +}
> +
> bool execute(struct execute_state *state,
> struct settings *settings,
> struct job_list *job_list)
> {
> struct utsname unamebuf;
> int resdirfd, testdirfd, unamefd, timefd;
> + sigset_t sigmask;
> + int sigfd;
> double time_spent = 0.0;
> bool status = true;
>
> @@ -1310,6 +1321,22 @@ bool execute(struct execute_state *state,
>
> oom_immortal();
>
> + sigemptyset(&sigmask);
> + sigaddset(&sigmask, SIGCHLD);
> + sigaddset(&sigmask, SIGINT);
> + sigaddset(&sigmask, SIGTERM);
> + sigaddset(&sigmask, SIGQUIT);
> + sigaddset(&sigmask, SIGHUP);
> + sigfd = signalfd(-1, &sigmask, O_CLOEXEC);
> + sigprocmask(SIG_BLOCK, &sigmask, NULL);
> +
> + if (sigfd < 0) {
> + /* TODO: Handle better */
> + fprintf(stderr, "Cannot mask signals\n");
> + status = -1;
This should probably be `status = false`. -1 is a truthy value.
> + goto end;
> + }
> +
> init_watchdogs(settings);
>
> if (!uname(&unamebuf)) {
> @@ -1345,12 +1372,18 @@ bool execute(struct execute_state *state,
> char *reason;
> int result;
>
> + if (should_die_because_signal(sigfd)) {
> + status = false;
Should we close_watchdogs at this point?
> + goto end;
> + }
> +
> result = execute_next_entry(state,
> job_list->size,
> &time_spent,
> settings,
> &job_list->entries[state->next],
> - testdirfd, resdirfd);
> + testdirfd, resdirfd,
> + sigfd, &sigmask);
The argument list is getting quite large. At some point it may be worth
it to put everything (or part of these) in a struct.
(In general I feel like this file could be improved a lot, these long
functions are hard to read.)
>
> if (result < 0) {
> status = false;
> @@ -1383,8 +1416,15 @@ bool execute(struct execute_state *state,
> if (result > 0) {
> double time_left = state->time_left;
>
> - close(testdirfd);
> close_watchdogs(settings);
> + sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
> + /* make sure that we do not leave any signals unhandled */
> + if (should_die_because_signal(sigfd)){
Style nit: missing space between `)` and `{`.
> + status = false;
> + goto end_post_signal_restore;
> + }
> + close(sigfd);
> + close(testdirfd);
> initialize_execute_state_from_resume(resdirfd, state, settings, job_list);
> state->time_left = time_left;
> return execute(state, settings, job_list);
> @@ -1397,8 +1437,14 @@ bool execute(struct execute_state *state,
> }
>
> end:
> - close(testdirfd);
> - close(resdirfd);
> close_watchdogs(settings);
> + sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
> + /* make sure that we do not leave any signals unhandled */
> + if (should_die_because_signal(sigfd))
> + status = false;
> + end_post_signal_restore:
> + close(sigfd);
> + close(testdirfd);
> + close(resdirfd);
> return status;
> }
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals
2019-07-18 10:57 ` Ser, Simon
@ 2019-07-19 11:25 ` Arkadiusz Hiler
2019-07-19 11:31 ` Ser, Simon
0 siblings, 1 reply; 15+ messages in thread
From: Arkadiusz Hiler @ 2019-07-19 11:25 UTC (permalink / raw)
To: Ser, Simon; +Cc: igt-dev, Latvala, Petri
On Thu, Jul 18, 2019 at 01:57:20PM +0300, Ser, Simon wrote:
> On Tue, 2019-07-09 at 15:23 +0300, Arkadiusz Hiler wrote:
> > There are few short windows of opportunity when watchdogs are primed but
> > there is no signal handling in place, so the process may exit without
> > proper shutdown sequence.
> >
> > This patch rearranges the existing code so that we set up the signalfd
> > and BLOCK the signals before setting up watchdogs and UNBLOCK only after
> > the watchdogs are closed properly.
> >
> > If igt_runner exits due to signal, non-zero status code is returned.
> >
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> > runner/executor.c | 100 +++++++++++++++++++++++++++++++++-------------
> > 1 file changed, 73 insertions(+), 27 deletions(-)
> >
> > diff --git a/runner/executor.c b/runner/executor.c
> > index 6463ab96..62303ff8 100644
> > --- a/runner/executor.c
> > +++ b/runner/executor.c
> > @@ -7,6 +7,7 @@
> > #include <string.h>
> > #include <sys/ioctl.h>
> > #include <sys/select.h>
> > +#include <sys/poll.h>
> > #include <sys/signalfd.h>
> > #include <sys/stat.h>
> > #include <sys/time.h>
> > @@ -604,7 +605,6 @@ static int monitor_output(pid_t child,
> > close(outfd);
> > close(errfd);
> > close(kmsgfd);
> > - close(sigfd);
> > return -1;
> > }
> >
> > @@ -776,9 +776,8 @@ static int monitor_output(pid_t child,
> > *time_spent = time;
> > }
> >
> > - close(sigfd);
> > - sigfd = -1;
> > child = 0;
> > + sigfd = -1; /* we are dying, no signal handling for now */
> > }
> > }
> >
> > @@ -790,7 +789,6 @@ static int monitor_output(pid_t child,
> > close(outfd);
> > close(errfd);
> > close(kmsgfd);
> > - close(sigfd);
> >
> > if (aborting)
> > return -1;
> > @@ -908,13 +906,12 @@ static int execute_next_entry(struct execute_state *state,
> > double *time_spent,
> > struct settings *settings,
> > struct job_list_entry *entry,
> > - int testdirfd, int resdirfd)
> > + int testdirfd, int resdirfd,
> > + int sigfd, sigset_t *sigmask)
> > {
> > int dirfd;
> > int outputs[_F_LAST];
> > int kmsgfd;
> > - int sigfd;
> > - sigset_t mask;
> > int outpipe[2] = { -1, -1 };
> > int errpipe[2] = { -1, -1 };
> > int outfd, errfd;
> > @@ -954,21 +951,6 @@ static int execute_next_entry(struct execute_state *state,
> > lseek(kmsgfd, 0, SEEK_END);
> > }
> >
> > - sigemptyset(&mask);
> > - sigaddset(&mask, SIGCHLD);
> > - sigaddset(&mask, SIGINT);
> > - sigaddset(&mask, SIGTERM);
> > - sigaddset(&mask, SIGQUIT);
> > - sigaddset(&mask, SIGHUP);
> > - sigprocmask(SIG_BLOCK, &mask, NULL);
> > - sigfd = signalfd(-1, &mask, O_CLOEXEC);
> > -
> > - if (sigfd < 0) {
> > - /* TODO: Handle better */
> > - fprintf(stderr, "Cannot monitor child process with signalfd\n");
> > - result = -1;
> > - goto out_kmsgfd;
> > - }
> >
> > if (settings->log_level >= LOG_LEVEL_NORMAL) {
> > char *displayname;
> > @@ -1002,7 +984,7 @@ static int execute_next_entry(struct execute_state *state,
> > close(outpipe[0]);
> > close(errpipe[0]);
> >
> > - sigprocmask(SIG_UNBLOCK, &mask, NULL);
> > + sigprocmask(SIG_UNBLOCK, sigmask, NULL);
> >
> > setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
> >
> > @@ -1261,12 +1243,41 @@ static void oom_immortal(void)
> > close(fd);
> > }
> >
> > +static bool should_die_because_signal(int sigfd)
> > +{
> > + struct signalfd_siginfo siginfo;
> > + int ret;
> > +
> > + struct pollfd sigpoll = { .fd = sigfd, .events = POLLIN | POLLRDBAND };
> > +
> > + if ((ret = poll(&sigpoll, 1, 0)) != 0) {
> > + if (ret == -1) {
>
> Seems like this is unintentionally left blank.
do {
ret = poll(&sigpoll, 1, 0);
} while (ret == -1 && (errno == EAGAIN || errno == EINTR));
if (ret != 0) {
if (ret == -1) {
fprintf(stderr, "Poll on signalfd failed with %s\n", strerror(errno));
return true; /* something is wrong, let's die */
}
Although this do-while is probalby an overkill for special fds.
> > + }
> > +
> > + ret = read(sigfd, &siginfo, sizeof(siginfo));
>
> Error handling is missing (ret == sizeof(siginfo)).
I am not sure about this check. This is not a normal fd and I partial
reads should not not be possible.
The other place we read it in is:
s = read(sigfd, &siginfo, sizeof(siginfo));
if (s < 0) {
fprintf(stderr, "Error reading from signalfd: %s\n",
strerror(errno));
continue;
I can add:
if (ret == -1) {
fprintf(stderr, "Error reading from signalfd: %s\n", strerror(errno));
return false; /* we may want to retry later */
}
> > + if (siginfo.ssi_signo == SIGCHLD) {
> > + fprintf(stderr, "Runner got stray SIGCHLD while not executing any tests.\n");
> > +
>
> Nit: extra blank line here
>
> > + } else {
> > + fprintf(stderr, "Runner is being killed by %s\n",
> > + strsignal(siginfo.ssi_signo));
> > + return true;
> > + }
> > +
> > + }
> > +
> > + return false;
> > +}
> > +
> > bool execute(struct execute_state *state,
> > struct settings *settings,
> > struct job_list *job_list)
> > {
> > struct utsname unamebuf;
> > int resdirfd, testdirfd, unamefd, timefd;
> > + sigset_t sigmask;
> > + int sigfd;
> > double time_spent = 0.0;
> > bool status = true;
> >
> > @@ -1310,6 +1321,22 @@ bool execute(struct execute_state *state,
> >
> > oom_immortal();
> >
> > + sigemptyset(&sigmask);
> > + sigaddset(&sigmask, SIGCHLD);
> > + sigaddset(&sigmask, SIGINT);
> > + sigaddset(&sigmask, SIGTERM);
> > + sigaddset(&sigmask, SIGQUIT);
> > + sigaddset(&sigmask, SIGHUP);
> > + sigfd = signalfd(-1, &sigmask, O_CLOEXEC);
> > + sigprocmask(SIG_BLOCK, &sigmask, NULL);
> > +
> > + if (sigfd < 0) {
> > + /* TODO: Handle better */
> > + fprintf(stderr, "Cannot mask signals\n");
> > + status = -1;
>
> This should probably be `status = false`. -1 is a truthy value.
>
> > + goto end;
> > + }
> > +
> > init_watchdogs(settings);
> >
> > if (!uname(&unamebuf)) {
> > @@ -1345,12 +1372,18 @@ bool execute(struct execute_state *state,
> > char *reason;
> > int result;
> >
> > + if (should_die_because_signal(sigfd)) {
> > + status = false;
>
> Should we close_watchdogs at this point?
"end" closes watchdgos:
end:
close_watchdogs(settings);
>
> > + goto end;
> > + }
> > +
> > result = execute_next_entry(state,
> > job_list->size,
> > &time_spent,
> > settings,
> > &job_list->entries[state->next],
> > - testdirfd, resdirfd);
> > + testdirfd, resdirfd,
> > + sigfd, &sigmask);
>
> The argument list is getting quite large. At some point it may be worth
> it to put everything (or part of these) in a struct.
>
> (In general I feel like this file could be improved a lot, these long
> functions are hard to read.)
Agreed. If I am going to do another substantial change to this file (or
another iteration of this series) it will get its own patch.
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals
2019-07-19 11:25 ` Arkadiusz Hiler
@ 2019-07-19 11:31 ` Ser, Simon
0 siblings, 0 replies; 15+ messages in thread
From: Ser, Simon @ 2019-07-19 11:31 UTC (permalink / raw)
To: Hiler, Arkadiusz; +Cc: igt-dev, Latvala, Petri
On Fri, 2019-07-19 at 14:25 +0300, Arkadiusz Hiler wrote:
> On Thu, Jul 18, 2019 at 01:57:20PM +0300, Ser, Simon wrote:
> > On Tue, 2019-07-09 at 15:23 +0300, Arkadiusz Hiler wrote:
> > > There are few short windows of opportunity when watchdogs are primed but
> > > there is no signal handling in place, so the process may exit without
> > > proper shutdown sequence.
> > >
> > > This patch rearranges the existing code so that we set up the signalfd
> > > and BLOCK the signals before setting up watchdogs and UNBLOCK only after
> > > the watchdogs are closed properly.
> > >
> > > If igt_runner exits due to signal, non-zero status code is returned.
> > >
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > ---
> > > runner/executor.c | 100 +++++++++++++++++++++++++++++++++-------------
> > > 1 file changed, 73 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/runner/executor.c b/runner/executor.c
> > > index 6463ab96..62303ff8 100644
> > > --- a/runner/executor.c
> > > +++ b/runner/executor.c
> > > @@ -7,6 +7,7 @@
> > > #include <string.h>
> > > #include <sys/ioctl.h>
> > > #include <sys/select.h>
> > > +#include <sys/poll.h>
> > > #include <sys/signalfd.h>
> > > #include <sys/stat.h>
> > > #include <sys/time.h>
> > > @@ -604,7 +605,6 @@ static int monitor_output(pid_t child,
> > > close(outfd);
> > > close(errfd);
> > > close(kmsgfd);
> > > - close(sigfd);
> > > return -1;
> > > }
> > >
> > > @@ -776,9 +776,8 @@ static int monitor_output(pid_t child,
> > > *time_spent = time;
> > > }
> > >
> > > - close(sigfd);
> > > - sigfd = -1;
> > > child = 0;
> > > + sigfd = -1; /* we are dying, no signal handling for now */
> > > }
> > > }
> > >
> > > @@ -790,7 +789,6 @@ static int monitor_output(pid_t child,
> > > close(outfd);
> > > close(errfd);
> > > close(kmsgfd);
> > > - close(sigfd);
> > >
> > > if (aborting)
> > > return -1;
> > > @@ -908,13 +906,12 @@ static int execute_next_entry(struct execute_state *state,
> > > double *time_spent,
> > > struct settings *settings,
> > > struct job_list_entry *entry,
> > > - int testdirfd, int resdirfd)
> > > + int testdirfd, int resdirfd,
> > > + int sigfd, sigset_t *sigmask)
> > > {
> > > int dirfd;
> > > int outputs[_F_LAST];
> > > int kmsgfd;
> > > - int sigfd;
> > > - sigset_t mask;
> > > int outpipe[2] = { -1, -1 };
> > > int errpipe[2] = { -1, -1 };
> > > int outfd, errfd;
> > > @@ -954,21 +951,6 @@ static int execute_next_entry(struct execute_state *state,
> > > lseek(kmsgfd, 0, SEEK_END);
> > > }
> > >
> > > - sigemptyset(&mask);
> > > - sigaddset(&mask, SIGCHLD);
> > > - sigaddset(&mask, SIGINT);
> > > - sigaddset(&mask, SIGTERM);
> > > - sigaddset(&mask, SIGQUIT);
> > > - sigaddset(&mask, SIGHUP);
> > > - sigprocmask(SIG_BLOCK, &mask, NULL);
> > > - sigfd = signalfd(-1, &mask, O_CLOEXEC);
> > > -
> > > - if (sigfd < 0) {
> > > - /* TODO: Handle better */
> > > - fprintf(stderr, "Cannot monitor child process with signalfd\n");
> > > - result = -1;
> > > - goto out_kmsgfd;
> > > - }
> > >
> > > if (settings->log_level >= LOG_LEVEL_NORMAL) {
> > > char *displayname;
> > > @@ -1002,7 +984,7 @@ static int execute_next_entry(struct execute_state *state,
> > > close(outpipe[0]);
> > > close(errpipe[0]);
> > >
> > > - sigprocmask(SIG_UNBLOCK, &mask, NULL);
> > > + sigprocmask(SIG_UNBLOCK, sigmask, NULL);
> > >
> > > setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
> > >
> > > @@ -1261,12 +1243,41 @@ static void oom_immortal(void)
> > > close(fd);
> > > }
> > >
> > > +static bool should_die_because_signal(int sigfd)
> > > +{
> > > + struct signalfd_siginfo siginfo;
> > > + int ret;
> > > +
> > > + struct pollfd sigpoll = { .fd = sigfd, .events = POLLIN | POLLRDBAND };
> > > +
> > > + if ((ret = poll(&sigpoll, 1, 0)) != 0) {
> > > + if (ret == -1) {
> >
> > Seems like this is unintentionally left blank.
>
> do {
> ret = poll(&sigpoll, 1, 0);
> } while (ret == -1 && (errno == EAGAIN || errno == EINTR));
>
> if (ret != 0) {
> if (ret == -1) {
> fprintf(stderr, "Poll on signalfd failed with %s\n", strerror(errno));
> return true; /* something is wrong, let's die */
> }
>
> Although this do-while is probalby an overkill for special fds.
Yeah, I agree. The `ret == -1` check is probably enough.
> > > + }
> > > +
> > > + ret = read(sigfd, &siginfo, sizeof(siginfo));
> >
> > Error handling is missing (ret == sizeof(siginfo)).
>
> I am not sure about this check. This is not a normal fd and I partial
> reads should not not be possible.
>
> The other place we read it in is:
>
> s = read(sigfd, &siginfo, sizeof(siginfo));
> if (s < 0) {
> fprintf(stderr, "Error reading from signalfd: %s\n",
> strerror(errno));
> continue;
>
>
> I can add:
>
> if (ret == -1) {
> fprintf(stderr, "Error reading from signalfd: %s\n", strerror(errno));
> return false; /* we may want to retry later */
> }
>
+1
As a side note, this:
fprintf(stderr, "Error reading from signalfd: %s\n", strerror(errno));
can be simplified to:
perror("Error reading from signalfd");
But it's up to you :P
> > > + if (siginfo.ssi_signo == SIGCHLD) {
> > > + fprintf(stderr, "Runner got stray SIGCHLD while not executing any tests.\n");
> > > +
> >
> > Nit: extra blank line here
> >
> > > + } else {
> > > + fprintf(stderr, "Runner is being killed by %s\n",
> > > + strsignal(siginfo.ssi_signo));
> > > + return true;
> > > + }
> > > +
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > bool execute(struct execute_state *state,
> > > struct settings *settings,
> > > struct job_list *job_list)
> > > {
> > > struct utsname unamebuf;
> > > int resdirfd, testdirfd, unamefd, timefd;
> > > + sigset_t sigmask;
> > > + int sigfd;
> > > double time_spent = 0.0;
> > > bool status = true;
> > >
> > > @@ -1310,6 +1321,22 @@ bool execute(struct execute_state *state,
> > >
> > > oom_immortal();
> > >
> > > + sigemptyset(&sigmask);
> > > + sigaddset(&sigmask, SIGCHLD);
> > > + sigaddset(&sigmask, SIGINT);
> > > + sigaddset(&sigmask, SIGTERM);
> > > + sigaddset(&sigmask, SIGQUIT);
> > > + sigaddset(&sigmask, SIGHUP);
> > > + sigfd = signalfd(-1, &sigmask, O_CLOEXEC);
> > > + sigprocmask(SIG_BLOCK, &sigmask, NULL);
> > > +
> > > + if (sigfd < 0) {
> > > + /* TODO: Handle better */
> > > + fprintf(stderr, "Cannot mask signals\n");
> > > + status = -1;
> >
> > This should probably be `status = false`. -1 is a truthy value.
> >
> > > + goto end;
> > > + }
> > > +
> > > init_watchdogs(settings);
> > >
> > > if (!uname(&unamebuf)) {
> > > @@ -1345,12 +1372,18 @@ bool execute(struct execute_state *state,
> > > char *reason;
> > > int result;
> > >
> > > + if (should_die_because_signal(sigfd)) {
> > > + status = false;
> >
> > Should we close_watchdogs at this point?
>
> "end" closes watchdgos:
>
> end:
> close_watchdogs(settings);
Oh right
> > > + goto end;
> > > + }
> > > +
> > > result = execute_next_entry(state,
> > > job_list->size,
> > > &time_spent,
> > > settings,
> > > &job_list->entries[state->next],
> > > - testdirfd, resdirfd);
> > > + testdirfd, resdirfd,
> > > + sigfd, &sigmask);
> >
> > The argument list is getting quite large. At some point it may be worth
> > it to put everything (or part of these) in a struct.
> >
> > (In general I feel like this file could be improved a lot, these long
> > functions are hard to read.)
>
> Agreed. If I am going to do another substantial change to this file (or
> another iteration of this series) it will get its own patch.
>
> Cheers,
> Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* [igt-dev] [PATCH v2 i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals Arkadiusz Hiler
2019-07-18 10:57 ` Ser, Simon
@ 2019-07-19 12:01 ` Arkadiusz Hiler
2019-07-19 12:04 ` Ser, Simon
1 sibling, 1 reply; 15+ messages in thread
From: Arkadiusz Hiler @ 2019-07-19 12:01 UTC (permalink / raw)
To: igt-dev; +Cc: Petri Latvala
There are few short windows of opportunity when watchdogs are primed but
there is no signal handling in place, so the process may exit without
proper shutdown sequence.
This patch rearranges the existing code so that we set up the signalfd
and BLOCK the signals before setting up watchdogs and UNBLOCK only after
the watchdogs are closed properly.
If igt_runner exits due to signal, non-zero status code is returned.
v2: more error handling and minor touch ups (Simon)
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
delta with v1: https://hiler.eu/p/9d79f730.txt
runner/executor.c | 107 ++++++++++++++++++++++++++++++++++------------
1 file changed, 80 insertions(+), 27 deletions(-)
diff --git a/runner/executor.c b/runner/executor.c
index 6463ab96..52fee7d1 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -7,6 +7,7 @@
#include <string.h>
#include <sys/ioctl.h>
#include <sys/select.h>
+#include <sys/poll.h>
#include <sys/signalfd.h>
#include <sys/stat.h>
#include <sys/time.h>
@@ -604,7 +605,6 @@ static int monitor_output(pid_t child,
close(outfd);
close(errfd);
close(kmsgfd);
- close(sigfd);
return -1;
}
@@ -776,9 +776,8 @@ static int monitor_output(pid_t child,
*time_spent = time;
}
- close(sigfd);
- sigfd = -1;
child = 0;
+ sigfd = -1; /* we are dying, no signal handling for now */
}
}
@@ -790,7 +789,6 @@ static int monitor_output(pid_t child,
close(outfd);
close(errfd);
close(kmsgfd);
- close(sigfd);
if (aborting)
return -1;
@@ -908,13 +906,12 @@ static int execute_next_entry(struct execute_state *state,
double *time_spent,
struct settings *settings,
struct job_list_entry *entry,
- int testdirfd, int resdirfd)
+ int testdirfd, int resdirfd,
+ int sigfd, sigset_t *sigmask)
{
int dirfd;
int outputs[_F_LAST];
int kmsgfd;
- int sigfd;
- sigset_t mask;
int outpipe[2] = { -1, -1 };
int errpipe[2] = { -1, -1 };
int outfd, errfd;
@@ -954,21 +951,6 @@ static int execute_next_entry(struct execute_state *state,
lseek(kmsgfd, 0, SEEK_END);
}
- sigemptyset(&mask);
- sigaddset(&mask, SIGCHLD);
- sigaddset(&mask, SIGINT);
- sigaddset(&mask, SIGTERM);
- sigaddset(&mask, SIGQUIT);
- sigaddset(&mask, SIGHUP);
- sigprocmask(SIG_BLOCK, &mask, NULL);
- sigfd = signalfd(-1, &mask, O_CLOEXEC);
-
- if (sigfd < 0) {
- /* TODO: Handle better */
- fprintf(stderr, "Cannot monitor child process with signalfd\n");
- result = -1;
- goto out_kmsgfd;
- }
if (settings->log_level >= LOG_LEVEL_NORMAL) {
char *displayname;
@@ -1002,7 +984,7 @@ static int execute_next_entry(struct execute_state *state,
close(outpipe[0]);
close(errpipe[0]);
- sigprocmask(SIG_UNBLOCK, &mask, NULL);
+ sigprocmask(SIG_UNBLOCK, sigmask, NULL);
setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
@@ -1261,12 +1243,48 @@ static void oom_immortal(void)
close(fd);
}
+static bool should_die_because_signal(int sigfd)
+{
+ struct signalfd_siginfo siginfo;
+ int ret;
+ struct pollfd sigpoll = { .fd = sigfd, .events = POLLIN | POLLRDBAND };
+
+ ret = poll(&sigpoll, 1, 0);
+
+ if (ret != 0) {
+ if (ret == -1) {
+ fprintf(stderr, "Poll on signalfd failed with %s\n", strerror(errno));
+ return true; /* something is wrong, let's die */
+ }
+
+ ret = read(sigfd, &siginfo, sizeof(siginfo));
+
+ if (ret == -1) {
+ fprintf(stderr, "Error reading from signalfd: %s\n", strerror(errno));
+ return false; /* we may want to retry later */
+ }
+
+ if (siginfo.ssi_signo == SIGCHLD) {
+ fprintf(stderr, "Runner got stray SIGCHLD while not executing any tests.\n");
+ } else {
+ fprintf(stderr, "Runner is being killed by %s\n",
+ strsignal(siginfo.ssi_signo));
+ return true;
+ }
+
+ }
+
+ return false;
+}
+
bool execute(struct execute_state *state,
struct settings *settings,
struct job_list *job_list)
{
struct utsname unamebuf;
int resdirfd, testdirfd, unamefd, timefd;
+ sigset_t sigmask;
+ int sigfd;
double time_spent = 0.0;
bool status = true;
@@ -1310,6 +1328,22 @@ bool execute(struct execute_state *state,
oom_immortal();
+ sigemptyset(&sigmask);
+ sigaddset(&sigmask, SIGCHLD);
+ sigaddset(&sigmask, SIGINT);
+ sigaddset(&sigmask, SIGTERM);
+ sigaddset(&sigmask, SIGQUIT);
+ sigaddset(&sigmask, SIGHUP);
+ sigfd = signalfd(-1, &sigmask, O_CLOEXEC);
+ sigprocmask(SIG_BLOCK, &sigmask, NULL);
+
+ if (sigfd < 0) {
+ /* TODO: Handle better */
+ fprintf(stderr, "Cannot mask signals\n");
+ status = false;
+ goto end;
+ }
+
init_watchdogs(settings);
if (!uname(&unamebuf)) {
@@ -1345,12 +1379,18 @@ bool execute(struct execute_state *state,
char *reason;
int result;
+ if (should_die_because_signal(sigfd)) {
+ status = false;
+ goto end;
+ }
+
result = execute_next_entry(state,
job_list->size,
&time_spent,
settings,
&job_list->entries[state->next],
- testdirfd, resdirfd);
+ testdirfd, resdirfd,
+ sigfd, &sigmask);
if (result < 0) {
status = false;
@@ -1383,8 +1423,15 @@ bool execute(struct execute_state *state,
if (result > 0) {
double time_left = state->time_left;
- close(testdirfd);
close_watchdogs(settings);
+ sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
+ /* make sure that we do not leave any signals unhandled */
+ if (should_die_because_signal(sigfd)) {
+ status = false;
+ goto end_post_signal_restore;
+ }
+ close(sigfd);
+ close(testdirfd);
initialize_execute_state_from_resume(resdirfd, state, settings, job_list);
state->time_left = time_left;
return execute(state, settings, job_list);
@@ -1397,8 +1444,14 @@ bool execute(struct execute_state *state,
}
end:
- close(testdirfd);
- close(resdirfd);
close_watchdogs(settings);
+ sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
+ /* make sure that we do not leave any signals unhandled */
+ if (should_die_because_signal(sigfd))
+ status = false;
+ end_post_signal_restore:
+ close(sigfd);
+ close(testdirfd);
+ close(resdirfd);
return status;
}
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH v2 i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals
2019-07-19 12:01 ` [igt-dev] [PATCH v2 " Arkadiusz Hiler
@ 2019-07-19 12:04 ` Ser, Simon
0 siblings, 0 replies; 15+ messages in thread
From: Ser, Simon @ 2019-07-19 12:04 UTC (permalink / raw)
To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri
On Fri, 2019-07-19 at 15:01 +0300, Arkadiusz Hiler wrote:
> There are few short windows of opportunity when watchdogs are primed but
> there is no signal handling in place, so the process may exit without
> proper shutdown sequence.
>
> This patch rearranges the existing code so that we set up the signalfd
> and BLOCK the signals before setting up watchdogs and UNBLOCK only after
> the watchdogs are closed properly.
>
> If igt_runner exits due to signal, non-zero status code is returned.
>
> v2: more error handling and minor touch ups (Simon)
>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Simon Ser <simon.ser@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
LGTM, thanks for the delta!
Reviewed-by: Simon Ser <simon.ser@intel.com>
> ---
>
> delta with v1: https://hiler.eu/p/9d79f730.txt
>
> runner/executor.c | 107 ++++++++++++++++++++++++++++++++++------------
> 1 file changed, 80 insertions(+), 27 deletions(-)
>
> diff --git a/runner/executor.c b/runner/executor.c
> index 6463ab96..52fee7d1 100644
> --- a/runner/executor.c
> +++ b/runner/executor.c
> @@ -7,6 +7,7 @@
> #include <string.h>
> #include <sys/ioctl.h>
> #include <sys/select.h>
> +#include <sys/poll.h>
> #include <sys/signalfd.h>
> #include <sys/stat.h>
> #include <sys/time.h>
> @@ -604,7 +605,6 @@ static int monitor_output(pid_t child,
> close(outfd);
> close(errfd);
> close(kmsgfd);
> - close(sigfd);
> return -1;
> }
>
> @@ -776,9 +776,8 @@ static int monitor_output(pid_t child,
> *time_spent = time;
> }
>
> - close(sigfd);
> - sigfd = -1;
> child = 0;
> + sigfd = -1; /* we are dying, no signal handling for now */
> }
> }
>
> @@ -790,7 +789,6 @@ static int monitor_output(pid_t child,
> close(outfd);
> close(errfd);
> close(kmsgfd);
> - close(sigfd);
>
> if (aborting)
> return -1;
> @@ -908,13 +906,12 @@ static int execute_next_entry(struct execute_state *state,
> double *time_spent,
> struct settings *settings,
> struct job_list_entry *entry,
> - int testdirfd, int resdirfd)
> + int testdirfd, int resdirfd,
> + int sigfd, sigset_t *sigmask)
> {
> int dirfd;
> int outputs[_F_LAST];
> int kmsgfd;
> - int sigfd;
> - sigset_t mask;
> int outpipe[2] = { -1, -1 };
> int errpipe[2] = { -1, -1 };
> int outfd, errfd;
> @@ -954,21 +951,6 @@ static int execute_next_entry(struct execute_state *state,
> lseek(kmsgfd, 0, SEEK_END);
> }
>
> - sigemptyset(&mask);
> - sigaddset(&mask, SIGCHLD);
> - sigaddset(&mask, SIGINT);
> - sigaddset(&mask, SIGTERM);
> - sigaddset(&mask, SIGQUIT);
> - sigaddset(&mask, SIGHUP);
> - sigprocmask(SIG_BLOCK, &mask, NULL);
> - sigfd = signalfd(-1, &mask, O_CLOEXEC);
> -
> - if (sigfd < 0) {
> - /* TODO: Handle better */
> - fprintf(stderr, "Cannot monitor child process with signalfd\n");
> - result = -1;
> - goto out_kmsgfd;
> - }
>
> if (settings->log_level >= LOG_LEVEL_NORMAL) {
> char *displayname;
> @@ -1002,7 +984,7 @@ static int execute_next_entry(struct execute_state *state,
> close(outpipe[0]);
> close(errpipe[0]);
>
> - sigprocmask(SIG_UNBLOCK, &mask, NULL);
> + sigprocmask(SIG_UNBLOCK, sigmask, NULL);
>
> setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
>
> @@ -1261,12 +1243,48 @@ static void oom_immortal(void)
> close(fd);
> }
>
> +static bool should_die_because_signal(int sigfd)
> +{
> + struct signalfd_siginfo siginfo;
> + int ret;
> + struct pollfd sigpoll = { .fd = sigfd, .events = POLLIN | POLLRDBAND };
> +
> + ret = poll(&sigpoll, 1, 0);
> +
> + if (ret != 0) {
> + if (ret == -1) {
> + fprintf(stderr, "Poll on signalfd failed with %s\n", strerror(errno));
> + return true; /* something is wrong, let's die */
> + }
> +
> + ret = read(sigfd, &siginfo, sizeof(siginfo));
> +
> + if (ret == -1) {
> + fprintf(stderr, "Error reading from signalfd: %s\n", strerror(errno));
> + return false; /* we may want to retry later */
> + }
> +
> + if (siginfo.ssi_signo == SIGCHLD) {
> + fprintf(stderr, "Runner got stray SIGCHLD while not executing any tests.\n");
> + } else {
> + fprintf(stderr, "Runner is being killed by %s\n",
> + strsignal(siginfo.ssi_signo));
> + return true;
> + }
> +
> + }
> +
> + return false;
> +}
> +
> bool execute(struct execute_state *state,
> struct settings *settings,
> struct job_list *job_list)
> {
> struct utsname unamebuf;
> int resdirfd, testdirfd, unamefd, timefd;
> + sigset_t sigmask;
> + int sigfd;
> double time_spent = 0.0;
> bool status = true;
>
> @@ -1310,6 +1328,22 @@ bool execute(struct execute_state *state,
>
> oom_immortal();
>
> + sigemptyset(&sigmask);
> + sigaddset(&sigmask, SIGCHLD);
> + sigaddset(&sigmask, SIGINT);
> + sigaddset(&sigmask, SIGTERM);
> + sigaddset(&sigmask, SIGQUIT);
> + sigaddset(&sigmask, SIGHUP);
> + sigfd = signalfd(-1, &sigmask, O_CLOEXEC);
> + sigprocmask(SIG_BLOCK, &sigmask, NULL);
> +
> + if (sigfd < 0) {
> + /* TODO: Handle better */
> + fprintf(stderr, "Cannot mask signals\n");
> + status = false;
> + goto end;
> + }
> +
> init_watchdogs(settings);
>
> if (!uname(&unamebuf)) {
> @@ -1345,12 +1379,18 @@ bool execute(struct execute_state *state,
> char *reason;
> int result;
>
> + if (should_die_because_signal(sigfd)) {
> + status = false;
> + goto end;
> + }
> +
> result = execute_next_entry(state,
> job_list->size,
> &time_spent,
> settings,
> &job_list->entries[state->next],
> - testdirfd, resdirfd);
> + testdirfd, resdirfd,
> + sigfd, &sigmask);
>
> if (result < 0) {
> status = false;
> @@ -1383,8 +1423,15 @@ bool execute(struct execute_state *state,
> if (result > 0) {
> double time_left = state->time_left;
>
> - close(testdirfd);
> close_watchdogs(settings);
> + sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
> + /* make sure that we do not leave any signals unhandled */
> + if (should_die_because_signal(sigfd)) {
> + status = false;
> + goto end_post_signal_restore;
> + }
> + close(sigfd);
> + close(testdirfd);
> initialize_execute_state_from_resume(resdirfd, state, settings, job_list);
> state->time_left = time_left;
> return execute(state, settings, job_list);
> @@ -1397,8 +1444,14 @@ bool execute(struct execute_state *state,
> }
>
> end:
> - close(testdirfd);
> - close(resdirfd);
> close_watchdogs(settings);
> + sigprocmask(SIG_UNBLOCK, &sigmask, NULL);
> + /* make sure that we do not leave any signals unhandled */
> + if (should_die_because_signal(sigfd))
> + status = false;
> + end_post_signal_restore:
> + close(sigfd);
> + close(testdirfd);
> + close(resdirfd);
> return status;
> }
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice (rev2)
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
` (4 preceding siblings ...)
2019-07-10 8:18 ` [igt-dev] [PATCH i-g-t 1/3] " Ser, Simon
@ 2019-07-19 13:31 ` Patchwork
2019-07-19 16:26 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-07-22 10:24 ` [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Petri Latvala
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-07-19 13:31 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice (rev2)
URL : https://patchwork.freedesktop.org/series/63439/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6511 -> IGTPW_3280
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/63439/revisions/2/mbox/
Known issues
------------
Here are the changes found in IGTPW_3280 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_exec@basic:
- fi-icl-u3: [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-icl-u3/igt@gem_ctx_exec@basic.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-icl-u3/igt@gem_ctx_exec@basic.html
* igt@gem_exec_create@basic:
- fi-pnv-d510: [PASS][3] -> [INCOMPLETE][4] ([fdo#110740])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-pnv-d510/igt@gem_exec_create@basic.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-pnv-d510/igt@gem_exec_create@basic.html
* igt@i915_selftest@live_contexts:
- fi-skl-iommu: [PASS][5] -> [INCOMPLETE][6] ([fdo#111050])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-skl-iommu/igt@i915_selftest@live_contexts.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-skl-iommu/igt@i915_selftest@live_contexts.html
* igt@i915_selftest@live_execlists:
- fi-skl-gvtdvm: [PASS][7] -> [DMESG-FAIL][8] ([fdo#111108])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
* igt@kms_chamelium@dp-crc-fast:
- fi-kbl-7500u: [PASS][9] -> [WARN][10] ([fdo#111156])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
#### Possible fixes ####
* igt@gem_basic@bad-close:
- fi-icl-u3: [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-icl-u3/igt@gem_basic@bad-close.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-icl-u3/igt@gem_basic@bad-close.html
* igt@gem_mmap_gtt@basic-copy:
- fi-glk-dsi: [INCOMPLETE][13] ([fdo#103359] / [k.org#198133]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-glk-dsi/igt@gem_mmap_gtt@basic-copy.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-glk-dsi/igt@gem_mmap_gtt@basic-copy.html
* igt@i915_selftest@live_client:
- fi-icl-dsi: [INCOMPLETE][15] ([fdo#107713]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-icl-dsi/igt@i915_selftest@live_client.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-icl-dsi/igt@i915_selftest@live_client.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7567u: [FAIL][17] ([fdo#109485]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
[fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740
[fdo#111050]: https://bugs.freedesktop.org/show_bug.cgi?id=111050
[fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108
[fdo#111156]: https://bugs.freedesktop.org/show_bug.cgi?id=111156
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (52 -> 47)
------------------------------
Additional (1): fi-cml-u2
Missing (6): fi-kbl-soraka fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* IGT: IGT_5103 -> IGTPW_3280
CI_DRM_6511: dbedd493118204a194fbc480f86866ddebbc4723 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3280: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/
IGT_5103: 45c31e294b9d7874a9a21860f8a89c64bc853df2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice (rev2)
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
` (5 preceding siblings ...)
2019-07-19 13:31 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice (rev2) Patchwork
@ 2019-07-19 16:26 ` Patchwork
2019-07-22 10:24 ` [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Petri Latvala
7 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-07-19 16:26 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice (rev2)
URL : https://patchwork.freedesktop.org/series/63439/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6511_full -> IGTPW_3280_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/63439/revisions/2/mbox/
Known issues
------------
Here are the changes found in IGTPW_3280_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_suspend@debugfs-reader:
- shard-apl: [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +5 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-apl5/igt@i915_suspend@debugfs-reader.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-apl7/igt@i915_suspend@debugfs-reader.html
* igt@kms_busy@extended-modeset-hang-oldfb-render-c:
- shard-iclb: [PASS][3] -> [INCOMPLETE][4] ([fdo#107713])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-iclb4/igt@kms_busy@extended-modeset-hang-oldfb-render-c.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-iclb7/igt@kms_busy@extended-modeset-hang-oldfb-render-c.html
* igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen:
- shard-apl: [PASS][5] -> [FAIL][6] ([fdo#103232])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-apl5/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html
- shard-kbl: [PASS][7] -> [FAIL][8] ([fdo#103232])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html
* igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions-varying-size:
- shard-snb: [PASS][9] -> [INCOMPLETE][10] ([fdo#105411])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-snb5/igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions-varying-size.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-snb7/igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions-varying-size.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render:
- shard-iclb: [PASS][11] -> [FAIL][12] ([fdo#103167]) +5 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render.html
* igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu:
- shard-glk: [PASS][13] -> [FAIL][14] ([fdo#103167])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-glk8/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu.html
* igt@kms_psr@psr2_cursor_mmap_cpu:
- shard-iclb: [PASS][15] -> [SKIP][16] ([fdo#109441]) +2 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-iclb6/igt@kms_psr@psr2_cursor_mmap_cpu.html
* igt@perf@rc6-disable:
- shard-kbl: [PASS][17] -> [FAIL][18] ([fdo#103179])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-kbl7/igt@perf@rc6-disable.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-kbl3/igt@perf@rc6-disable.html
#### Possible fixes ####
* igt@gem_exec_balancer@smoke:
- shard-iclb: [SKIP][19] ([fdo#110854]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-iclb3/igt@gem_exec_balancer@smoke.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-iclb2/igt@gem_exec_balancer@smoke.html
* igt@gem_tiled_swapping@non-threaded:
- shard-glk: [DMESG-WARN][21] ([fdo#108686]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-glk2/igt@gem_tiled_swapping@non-threaded.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-glk7/igt@gem_tiled_swapping@non-threaded.html
* igt@i915_pm_rc6_residency@rc6-accuracy:
- shard-iclb: [SKIP][23] ([fdo#110933]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-iclb4/igt@i915_pm_rc6_residency@rc6-accuracy.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-iclb4/igt@i915_pm_rc6_residency@rc6-accuracy.html
* igt@i915_suspend@sysfs-reader:
- shard-apl: [DMESG-WARN][25] ([fdo#108566]) -> [PASS][26] +4 similar issues
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-apl6/igt@i915_suspend@sysfs-reader.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-apl1/igt@i915_suspend@sysfs-reader.html
* igt@kms_flip@flip-vs-suspend:
- shard-hsw: [INCOMPLETE][27] ([fdo#103540]) -> [PASS][28] +1 similar issue
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-hsw5/igt@kms_flip@flip-vs-suspend.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-hsw8/igt@kms_flip@flip-vs-suspend.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
- shard-apl: [FAIL][29] ([fdo#103167]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-apl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-apl3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html
- shard-kbl: [FAIL][31] ([fdo#103167]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbc-stridechange:
- shard-iclb: [FAIL][33] ([fdo#103167]) -> [PASS][34] +4 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-stridechange.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-stridechange.html
* igt@kms_psr@psr2_basic:
- shard-iclb: [SKIP][35] ([fdo#109441]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-iclb7/igt@kms_psr@psr2_basic.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-iclb2/igt@kms_psr@psr2_basic.html
* igt@kms_setmode@basic:
- shard-apl: [FAIL][37] ([fdo#99912]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-apl2/igt@kms_setmode@basic.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-apl7/igt@kms_setmode@basic.html
#### Warnings ####
* igt@kms_vblank@pipe-c-ts-continuation-modeset-rpm:
- shard-snb: [SKIP][39] ([fdo#109271] / [fdo#109278]) -> [SKIP][40] ([fdo#109271])
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6511/shard-snb6/igt@kms_vblank@pipe-c-ts-continuation-modeset-rpm.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/shard-snb6/igt@kms_vblank@pipe-c-ts-continuation-modeset-rpm.html
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103179]: https://bugs.freedesktop.org/show_bug.cgi?id=103179
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
[fdo#110933]: https://bugs.freedesktop.org/show_bug.cgi?id=110933
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 6)
------------------------------
Missing (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005
Build changes
-------------
* IGT: IGT_5103 -> IGTPW_3280
* Piglit: piglit_4509 -> None
CI_DRM_6511: dbedd493118204a194fbc480f86866ddebbc4723 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3280: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3280/
IGT_5103: 45c31e294b9d7874a9a21860f8a89c64bc853df2 @ 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_3280/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
` (6 preceding siblings ...)
2019-07-19 16:26 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2019-07-22 10:24 ` Petri Latvala
7 siblings, 0 replies; 15+ messages in thread
From: Petri Latvala @ 2019-07-22 10:24 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev
On Tue, Jul 09, 2019 at 03:23:43PM +0300, Arkadiusz Hiler wrote:
> Setting the watchdog fd lists to NULL for extra fireworks if accessed
> unintentionally.
>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
For the series:
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-07-22 10:24 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 12:23 [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Arkadiusz Hiler
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 2/3] runner: Warn when watchdogs are being closed from the exit handler Arkadiusz Hiler
2019-07-10 8:20 ` Ser, Simon
2019-07-09 12:23 ` [igt-dev] [PATCH i-g-t 3/3] runner: Make sure that we are closing watchdogs on signals Arkadiusz Hiler
2019-07-18 10:57 ` Ser, Simon
2019-07-19 11:25 ` Arkadiusz Hiler
2019-07-19 11:31 ` Ser, Simon
2019-07-19 12:01 ` [igt-dev] [PATCH v2 " Arkadiusz Hiler
2019-07-19 12:04 ` Ser, Simon
2019-07-09 13:04 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice Patchwork
2019-07-10 0:24 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-07-10 8:18 ` [igt-dev] [PATCH i-g-t 1/3] " Ser, Simon
2019-07-19 13:31 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] runner: Make sure we don't close watchdogs twice (rev2) Patchwork
2019-07-19 16:26 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-07-22 10:24 ` [igt-dev] [PATCH i-g-t 1/3] runner: Make sure we don't close watchdogs twice Petri Latvala
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.