| Message ID | 20171214004810.23305-1-antonio.argenziano@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/12/17 16:48, Antonio Argenziano wrote: > The test expected IOCTL 'I915_GET_RESET_STATS' would return an error > when not root. That is no longer true in the driver since commit > 4c9c0d09741d ("drm/i915: Fix retrieval of hangcheck stats") and therefore > the test was incorrectly failing. > > v2: > - Add the commit that changed the behaviour in the Driver to the > commit message (Michel) > > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com> Ping. > --- > tests/gem_reset_stats.c | 25 +++++++++++++++++-------- > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c > index edc40767..0c36a7eb 100644 > --- a/tests/gem_reset_stats.c > +++ b/tests/gem_reset_stats.c > @@ -605,10 +605,7 @@ static void test_reset_count(const struct intel_execution_engine *e, > > c2 = get_reset_count(fd, ctx); > > - if (ctx == 0) > - igt_assert(c2 == -EPERM); > - else > - igt_assert(c2 == 0); > + igt_assert(c2 == 0); > } > > igt_waitchildren(); > @@ -619,6 +616,11 @@ static void test_reset_count(const struct intel_execution_engine *e, > close(fd); > } > > +static int __get_reset_stats(int fd, struct local_drm_i915_reset_stats *rs) > +{ > + return drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs); > +} > + > static int _test_params(int fd, int ctx, uint32_t flags, uint32_t pad) > { > struct local_drm_i915_reset_stats rs; > @@ -644,10 +646,17 @@ static void _check_param_ctx(const int fd, const int ctx, const cap_t cap) > const uint32_t bad = rand() + 1; > > if (ctx == 0) { > - if (cap == root) > - igt_assert_eq(_test_params(fd, ctx, 0, 0), 0); > - else > - igt_assert_eq(_test_params(fd, ctx, 0, 0), -EPERM); > + igt_assert_eq(_test_params(fd, ctx, 0, 0), 0); > + > + if (cap != root) { > + struct local_drm_i915_reset_stats rs; > + rs.ctx_id = ctx; > + rs.reset_count = rand(); > + rs.batch_active = rand(); > + rs.batch_pending = rand(); > + igt_assert(__get_reset_stats(fd, &rs)); > + igt_assert(rs.reset_count != 0); > + } > } > > igt_assert_eq(_test_params(fd, ctx, 0, bad), -EINVAL); >
On 1/16/2018 10:44 AM, Antonio Argenziano wrote: > On 13/12/17 16:48, Antonio Argenziano wrote: >> The test expected IOCTL 'I915_GET_RESET_STATS' would return an error >> when not root. That is no longer true in the driver since commit >> 4c9c0d09741d ("drm/i915: Fix retrieval of hangcheck stats") and therefore >> the test was incorrectly failing. >> >> v2: >> - Add the commit that changed the behaviour in the Driver to the >> commit message (Michel) >> >> Cc: Michel Thierry <michel.thierry@intel.com> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com> > > Ping. > >> --- >> tests/gem_reset_stats.c | 25 +++++++++++++++++-------- >> 1 file changed, 17 insertions(+), 8 deletions(-) >> >> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c >> index edc40767..0c36a7eb 100644 >> --- a/tests/gem_reset_stats.c >> +++ b/tests/gem_reset_stats.c >> @@ -605,10 +605,7 @@ static void test_reset_count(const struct >> intel_execution_engine *e, >> c2 = get_reset_count(fd, ctx); >> - if (ctx == 0) >> - igt_assert(c2 == -EPERM); >> - else >> - igt_assert(c2 == 0); >> + igt_assert(c2 == 0); >> } >> igt_waitchildren(); >> @@ -619,6 +616,11 @@ static void test_reset_count(const struct >> intel_execution_engine *e, >> close(fd); >> } >> +static int __get_reset_stats(int fd, struct >> local_drm_i915_reset_stats *rs) >> +{ >> + return drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs); >> +} >> + >> static int _test_params(int fd, int ctx, uint32_t flags, uint32_t pad) >> { >> struct local_drm_i915_reset_stats rs; >> @@ -644,10 +646,17 @@ static void _check_param_ctx(const int fd, const >> int ctx, const cap_t cap) >> const uint32_t bad = rand() + 1; >> if (ctx == 0) { >> - if (cap == root) >> - igt_assert_eq(_test_params(fd, ctx, 0, 0), 0); >> - else >> - igt_assert_eq(_test_params(fd, ctx, 0, 0), -EPERM); >> + igt_assert_eq(_test_params(fd, ctx, 0, 0), 0); >> + >> + if (cap != root) { >> + struct local_drm_i915_reset_stats rs; >> + rs.ctx_id = ctx; >> + rs.reset_count = rand(); >> + rs.batch_active = rand(); >> + rs.batch_pending = rand(); >> + igt_assert(__get_reset_stats(fd, &rs)); >> + igt_assert(rs.reset_count != 0); >> + } Probably you only care about reset_count == 0, so I would just reuse existing code like this: if (cap != root) igt_assert(get_reset_count(fd, cxt) == 0); >> } >> igt_assert_eq(_test_params(fd, ctx, 0, bad), -EINVAL); >>
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c index edc40767..0c36a7eb 100644 --- a/tests/gem_reset_stats.c +++ b/tests/gem_reset_stats.c @@ -605,10 +605,7 @@ static void test_reset_count(const struct intel_execution_engine *e, c2 = get_reset_count(fd, ctx); - if (ctx == 0) - igt_assert(c2 == -EPERM); - else - igt_assert(c2 == 0); + igt_assert(c2 == 0); } igt_waitchildren(); @@ -619,6 +616,11 @@ static void test_reset_count(const struct intel_execution_engine *e, close(fd); } +static int __get_reset_stats(int fd, struct local_drm_i915_reset_stats *rs) +{ + return drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs); +} + static int _test_params(int fd, int ctx, uint32_t flags, uint32_t pad) { struct local_drm_i915_reset_stats rs; @@ -644,10 +646,17 @@ static void _check_param_ctx(const int fd, const int ctx, const cap_t cap) const uint32_t bad = rand() + 1; if (ctx == 0) { - if (cap == root) - igt_assert_eq(_test_params(fd, ctx, 0, 0), 0); - else - igt_assert_eq(_test_params(fd, ctx, 0, 0), -EPERM); + igt_assert_eq(_test_params(fd, ctx, 0, 0), 0); + + if (cap != root) { + struct local_drm_i915_reset_stats rs; + rs.ctx_id = ctx; + rs.reset_count = rand(); + rs.batch_active = rand(); + rs.batch_pending = rand(); + igt_assert(__get_reset_stats(fd, &rs)); + igt_assert(rs.reset_count != 0); + } } igt_assert_eq(_test_params(fd, ctx, 0, bad), -EINVAL);
The test expected IOCTL 'I915_GET_RESET_STATS' would return an error when not root. That is no longer true in the driver since commit 4c9c0d09741d ("drm/i915: Fix retrieval of hangcheck stats") and therefore the test was incorrectly failing. v2: - Add the commit that changed the behaviour in the Driver to the commit message (Michel) Cc: Michel Thierry <michel.thierry@intel.com> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com> --- tests/gem_reset_stats.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)