[Intel-gfx] [PATCH] tests: add gem_reset_stats

[Daniel Vetter]

On Wed, Nov 13, 2013 at 3:13 PM, Mika Kuoppala
<mika.kuoppala at linux.intel.com> wrote:
> Daniel Vetter <daniel at ffwll.ch> writes:
>
>> On Tue, Nov 12, 2013 at 07:58:16PM +0200, Mika Kuoppala wrote:
>>> v2: check the ioctl pad and flag parameters
>>>
>>> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
>>
>> I've merged this to igt, but there are a few small fixups to do on top:
>> - We now have the igt_main macro to cut down a bit on boilerplate.
>> - I haven't tested it, but I guess inject_hang will cause some *ERROR*
>>   noise in dmesg. Rule is that igt testcases should only cause info/debug
>>   level messages, everything else is considered a failure. I think we
>>   could fix this by setting the stop_rings debugfs value right _after_ the
>>   hang is injected, to tell the kernel that the hang it'll see is actually
>>   fake.
>
> I will take a look what stop_rings do. I just have a feeling
> that with this kind of trickery we shoot ourselves in foot some day.

Yeah, it's a bit trickery. But as long as we only have on special
testcase I think it's ok. If we grow more injected hang tests which
aren't simulated with the stop_ring stuff then maybe a new debugfs
variable to tell the kernel to expect a fake hang could be useful. But
smells like not worth it right now.

> Would be good that we get a proper error messages when we submit a real
> hanging batches. Would be also good that our test checks that
> the ERROR msg was really emitted.

I think the check for the reset count is good enough to make sure the
hangcheck code works. If we start to check for specific dmesg lines I
fear we'll implicitly make them abi. We have uevents as a general
signal to userspace that a hang happened (and now the reset_stat
ioctl).

> Having whitelist of expected '*ERROR*' messages for these kind of tests
> is not an option?

Thus far we've just tuned down the message to info level for fake
hangs. It does make things a bit easier for the test runner since we
can just filter for any dmesg noise with a level >= warn and then fail
the test. This is what QA does, and with the latest piglit patches
I've just pushed also what the piglit testrunner does.

>> - The userspace interface checking has two missing spots: a) checking that
>>   lookup for an invalid ctx id fails with ENOENT b) checking that non-root
>>   can't read out the default context. For the later it's probably simples
>>   to fork a 2nd process and drop the CAP_SYS_ADMIN capability in there.
>
> I check for ENOENT on submitting bad context id there. Did you miss it
> or it is not enough?

Oops, missed that. I didn't reread the latest version too carefully :(

> I will resubmit when I have b) in place.

Thanks. Btw I've already pushed your current patch to igt, so just a
follow-up patch is required.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch