On 6/5/2017 3:57 PM, Chris Wilson wrote:
Quoting Michel Thierry (2017-06-05 22:21:48)
On 6/5/2017 5:13 AM, Chris Wilson wrote:
The hang detector relies on a uevent for notification and aborting the
test. As proposed, fine-grained resets may not produce a global uevent
and so this hang detection becomes void. As we don't expect any hang, we
can just reduce the reset to only a global + uevent and so maintain
functionality, and switch back to fine-grained resets afterwards.
Note that any test that requires testing fine-grained resets should
ensure that they are enabled first as igt may leave the global
parameters in an inconsistent state.
v2: Restore fine-grained resets for explict igt_allow_hang()
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
---
lib/igt_aux.c | 10 ++++++++
lib/igt_gt.c | 4 ++++
lib/igt_sysfs.c | 72 ++++++++++++++++++++++++++++++++++++++++++---------------
lib/igt_sysfs.h | 6 +++++
4 files changed, 74 insertions(+), 18 deletions(-)
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 1222806c..b1b63db5 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -60,6 +60,7 @@
#include "igt_debugfs.h"
#include "igt_gt.h"
#include "igt_rand.h"
+#include "igt_sysfs.h"
#include "config.h"
#include "intel_reg.h"
#include "ioctl_wrappers.h"
@@ -449,6 +450,15 @@ void igt_fork_hang_detector(int fd)
igt_assert(fstat(fd, &st) == 0);
+ /*
+ * Disable per-engine reset to force an error uevent. We don't
+ * expect to get any hangs whilst the detector is enabled (if we do
+ * they are a test failure!) and so the loss of per-engine reset
+ * functionality is not an issue.
+ */
+ igt_assert(igt_sysfs_set_parameter
+ (fd, "reset", "%d", 1 /* only global reset */));
+
signal(SIGIO, sig_abort);
igt_fork_helper(&hang_detector)
hang_detector_process(getppid(), st.st_rdev);
I think the stop_hang_detector needs to restore the 'reset' value too,
or subsequent tests (using igt_hang_ctx) would only have global reset;
There's nothing fundamentally wrong with that... The problem is that we
are introducing a second reset path, and so anything that is testing
resets should ideally exercise both. :| That's where I was going with
my comments about this leaving igt in an inconsistent state and that we
should specify which path we want to test when we do the tests.
Ok, I agree "reset tests" should either exercise both or explicitly set
if they want engine or global reset. Other tests that just require a
hang subtest can use either one.
The caveat would be display's hung subtests, but I noticed they write -1
to i915_wedged, and that will trigger a global reset anyway.
Reviewed-by: Michel Thierry <michel.thierry@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
