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. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
