* [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.