All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
@ 2017-12-08 16:27 Antonio Argenziano
  2017-12-08 16:46 ` Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Antonio Argenziano @ 2017-12-08 16:27 UTC (permalink / raw)
  To: intel-gfx

The test expected IOCTL 'I915_GET_RESET_STATS' would return an error
when not root. That is no longer true in the driver and therefore
the test was incorrectly failing.

Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
---
 tests/gem_reset_stats.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index edc40767..83c91f0f 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,16 @@ 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);
+		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);
-- 
2.14.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
  2017-12-08 16:27 [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation Antonio Argenziano
@ 2017-12-08 16:46 ` Chris Wilson
  2017-12-08 17:07   ` Antonio Argenziano
  2017-12-08 16:53 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-12-08 18:19 ` ✗ Fi.CI.IGT: failure " Patchwork
  2 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2017-12-08 16:46 UTC (permalink / raw)
  To: Antonio Argenziano, intel-gfx

Quoting Antonio Argenziano (2017-12-08 16:27:17)
> The test expected IOCTL 'I915_GET_RESET_STATS' would return an error
> when not root. That is no longer true in the driver and therefore
> the test was incorrectly failing.
> 
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> ---
>  tests/gem_reset_stats.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
> index edc40767..83c91f0f 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,16 @@ 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);

Spurious indenting leftover.

> -               else
> -                       igt_assert_eq(_test_params(fd, ctx, 0, 0), -EPERM);
> +               if (cap != root) {

So what are you expecting to happen if you do happen to be rot? Is this
test redundant, which is why you skipped it?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
  2017-12-08 16:27 [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation Antonio Argenziano
  2017-12-08 16:46 ` Chris Wilson
@ 2017-12-08 16:53 ` Patchwork
  2017-12-08 18:19 ` ✗ Fi.CI.IGT: failure " Patchwork
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-12-08 16:53 UTC (permalink / raw)
  To: Antonio Argenziano; +Cc: intel-gfx

== Series Details ==

Series: tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
URL   : https://patchwork.freedesktop.org/series/35101/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
2fc64acf8a4465d5eab3d6cfec9b3c1b5df30d73 igt/perf_pmu: Tweak wait_for_rc6, yet again

with latest DRM-Tip kernel build CI_DRM_3483
b5f297e08432 drm-tip: 2017y-12m-08d-13h-53m-36s UTC integration manifest

No testlist changes.

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> FAIL       (fi-elk-e7500) fdo#103989
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-gdg-551) fdo#102618
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:440s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:442s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:389s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:521s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:284s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:508s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:504s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:492s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:478s
fi-elk-e7500     total:224  pass:163  dwarn:14  dfail:0   fail:1   skip:45 
fi-gdg-551       total:288  pass:177  dwarn:1   dfail:0   fail:2   skip:108 time:280s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:375s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:261s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:400s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:449s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:485s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:533s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:472s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:537s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:590s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:459s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:549s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:572s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:526s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:505s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:452s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:417s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:611s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:639s
fi-glk-dsi       total:288  pass:175  dwarn:1   dfail:4   fail:0   skip:108 time:343s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_630/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
  2017-12-08 16:46 ` Chris Wilson
@ 2017-12-08 17:07   ` Antonio Argenziano
  2017-12-08 18:33     ` Michel Thierry
  2018-01-10 22:04     ` Antonio Argenziano
  0 siblings, 2 replies; 7+ messages in thread
From: Antonio Argenziano @ 2017-12-08 17:07 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx



On 08/12/17 08:46, Chris Wilson wrote:
> Quoting Antonio Argenziano (2017-12-08 16:27:17)
>> The test expected IOCTL 'I915_GET_RESET_STATS' would return an error
>> when not root. That is no longer true in the driver and therefore
>> the test was incorrectly failing.
>>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>> ---
>>   tests/gem_reset_stats.c | 22 +++++++++++++++-------
>>   1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
>> index edc40767..83c91f0f 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,16 @@ 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);
> 
> Spurious indenting leftover.
> 
>> -               else
>> -                       igt_assert_eq(_test_params(fd, ctx, 0, 0), -EPERM);
>> +               if (cap != root) {
> 
> So what are you expecting to happen if you do happen to be rot? Is this
> test redundant, which is why you skipped it?

Yes, I think it is redundant because the only expectation for root is 
for the IOCTL to be successful as it is for non root users (that is why 
I left the first assert to be run unconditionally), and, even if root is 
supposed to get the correct reset_count value, unless I am missing 
something, that test is not in the scope of this subtest.

-Antonio

> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✗ Fi.CI.IGT: failure for tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
  2017-12-08 16:27 [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation Antonio Argenziano
  2017-12-08 16:46 ` Chris Wilson
  2017-12-08 16:53 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-12-08 18:19 ` Patchwork
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-12-08 18:19 UTC (permalink / raw)
  To: Antonio Argenziano; +Cc: intel-gfx

== Series Details ==

Series: tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
URL   : https://patchwork.freedesktop.org/series/35101/
State : failure

== Summary ==

Test kms_cursor_legacy:
        Subgroup all-pipes-forked-bo:
                pass       -> INCOMPLETE (shard-snb)
        Subgroup pipe-c-torture-bo:
                pass       -> INCOMPLETE (shard-hsw)
        Subgroup cursor-vs-flip-atomic-transitions-varying-size:
                pass       -> SKIP       (shard-snb) fdo#103172
Test drv_suspend:
        Subgroup fence-restore-tiled2untiled-hibernate:
                skip       -> FAIL       (shard-hsw) fdo#103375
        Subgroup forcewake:
                skip       -> PASS       (shard-hsw)
        Subgroup debugfs-reader:
                skip       -> PASS       (shard-hsw)
Test drv_module_reload:
        Subgroup basic-reload:
                dmesg-warn -> PASS       (shard-snb) fdo#102848
                dmesg-warn -> PASS       (shard-hsw) fdo#102707
Test kms_flip:
        Subgroup plain-flip-fb-recreate-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-snb) fdo#101623
Test kms_atomic_transition:
        Subgroup plane-all-transition:
                pass       -> SKIP       (shard-snb)

fdo#103172 https://bugs.freedesktop.org/show_bug.cgi?id=103172
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#102848 https://bugs.freedesktop.org/show_bug.cgi?id=102848
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-hsw        total:2606 pass:1488 dwarn:1   dfail:0   fail:11  skip:1105 time:9343s
shard-snb        total:2639 pass:1285 dwarn:1   dfail:0   fail:12  skip:1340 time:7883s
Blacklisted hosts:
shard-apl        total:2679 pass:1679 dwarn:1   dfail:0   fail:21  skip:977 time:13779s
shard-kbl        total:2679 pass:1797 dwarn:1   dfail:0   fail:25  skip:856 time:10859s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_630/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
  2017-12-08 17:07   ` Antonio Argenziano
@ 2017-12-08 18:33     ` Michel Thierry
  2018-01-10 22:04     ` Antonio Argenziano
  1 sibling, 0 replies; 7+ messages in thread
From: Michel Thierry @ 2017-12-08 18:33 UTC (permalink / raw)
  To: Antonio Argenziano, Chris Wilson, intel-gfx

On 08/12/17 09:07, Antonio Argenziano wrote:
> 
> 
> On 08/12/17 08:46, Chris Wilson wrote:
>> Quoting Antonio Argenziano (2017-12-08 16:27:17)
>>> The test expected IOCTL 'I915_GET_RESET_STATS' would return an error
>>> when not root. That is no longer true in the driver and therefore
I would add the commit that changed the behaviour,

...when not root. This is no longer true in the driver since commit 
4c9c0d09741d ("drm/i915: Fix retrieval of hangcheck stats") and therefore...

>>> the test was incorrectly failing.
>>>
>>> Cc: Michel Thierry <michel.thierry@intel.com>
>>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>> ---
>>>    tests/gem_reset_stats.c | 22 +++++++++++++++-------
>>>    1 file changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
>>> index edc40767..83c91f0f 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,16 @@ 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);
>>
>> Spurious indenting leftover.
>>
>>> -               else
>>> -                       igt_assert_eq(_test_params(fd, ctx, 0, 0), -EPERM);
>>> +               if (cap != root) {
>>
>> So what are you expecting to happen if you do happen to be rot? Is this
>> test redundant, which is why you skipped it?
> 
> Yes, I think it is redundant because the only expectation for root is
> for the IOCTL to be successful as it is for non root users (that is why
> I left the first assert to be run unconditionally), and, even if root is
> supposed to get the correct reset_count value, unless I am missing
> something, that test is not in the scope of this subtest.
> 
> -Antonio
> 
>> -Chris
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation
  2017-12-08 17:07   ` Antonio Argenziano
  2017-12-08 18:33     ` Michel Thierry
@ 2018-01-10 22:04     ` Antonio Argenziano
  1 sibling, 0 replies; 7+ messages in thread
From: Antonio Argenziano @ 2018-01-10 22:04 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Michel Thierry



On 08/12/17 09:07, Antonio Argenziano wrote:
> 
> 
> On 08/12/17 08:46, Chris Wilson wrote:
>> Quoting Antonio Argenziano (2017-12-08 16:27:17)
>>> The test expected IOCTL 'I915_GET_RESET_STATS' would return an error
>>> when not root. That is no longer true in the driver and therefore
>>> the test was incorrectly failing.
>>>
>>> Cc: Michel Thierry <michel.thierry@intel.com>
>>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>> ---
>>>   tests/gem_reset_stats.c | 22 +++++++++++++++-------
>>>   1 file changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
>>> index edc40767..83c91f0f 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,16 @@ 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);
>>
>> Spurious indenting leftover.
>>
>>> -               else
>>> -                       igt_assert_eq(_test_params(fd, ctx, 0, 0), 
>>> -EPERM);
>>> +               if (cap != root) {
>>
>> So what are you expecting to happen if you do happen to be rot? Is this
>> test redundant, which is why you skipped it?
> 
> Yes, I think it is redundant because the only expectation for root is 
> for the IOCTL to be successful as it is for non root users (that is why 
> I left the first assert to be run unconditionally), and, even if root is 
> supposed to get the correct reset_count value, unless I am missing 

After looking at this again I disagree with myself :). I (now) think 
that if the interface doesn't allow a non privileged user to access some 
information it should return an error (EPERM) instead of returning a 
returning a fixed value after what looks like a a successful IOCTL, it 
might be incorrectly interpreted as the real thing while it should have 
been discarded. What do you think?

BTW, why are non-root users not allowed to read reset_stats?

> something, that test is not in the scope of this subtest.
> 
> -Antonio
> 
>> -Chris
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-10 22:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-08 16:27 [PATCH i-g-t] tests/gem_reset_stats: Fix retrieval of hangcheck stats expectation Antonio Argenziano
2017-12-08 16:46 ` Chris Wilson
2017-12-08 17:07   ` Antonio Argenziano
2017-12-08 18:33     ` Michel Thierry
2018-01-10 22:04     ` Antonio Argenziano
2017-12-08 16:53 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-12-08 18:19 ` ✗ Fi.CI.IGT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.