[igt-dev] [PATCH i-g-t 2/2] tests/perf: add test for disabled preemption stream

[Lionel Landwerlin]

On 06/06/18 17:52, Antonio Argenziano wrote:
>
>
> On 05/06/18 09:25, Lionel Landwerlin wrote:
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> ---
>
>> +
>
> Maybe a comment on what busy loop is trying to do would help the lazy 
> reader :).

Sure.

>
>> +static uint32_t
>> +busy_loop(uint32_t context, uint32_t duration_ns)
>> +{
>> +    struct drm_i915_gem_execbuffer2 execbuf;
>> +    struct drm_i915_gem_exec_object2 obj[4];
>
>> +
>> +    for (i = 1 /* skip data_handle */; i < ARRAY_SIZE(obj); i++)
>> +        gem_close(drm_fd, obj[i].handle);
>> +
>> +    return data_handle;
>> +}
>> +
>
>> +
>> +/*
>> + * Verify that holding preemption is not available for normal users
>> + * unless they perf_stream_paranoid is off.
>> + */
>> +static void
>> +test_unprivileged_single_ctx_counters_disabled_preemption(void)
>> +{
>> +    igt_fork(child, 1) {
>> + write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 1);
>> +
>> +        igt_drop_root();
>> +
>> + igt_assert(!has_i915_perf_disable_preemption_support(drm_fd));
>
> Shouldn't this assert the return code from the IOCTL is -EPERM?

Yes indeed, they could use some more precise checking.

>
>> +    }
>> +
>> +    igt_waitchildren();
>> +
>> +    igt_fork(child, 1) {
>> + write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 0);
>> +
>> +        igt_drop_root();
>> +
>> + igt_assert(has_i915_perf_disable_preemption_support(drm_fd));
>> +    }
>> +
>> +    igt_waitchildren();
>
> I think this could be split into two sub-tests. Do you plan to add 
> more API tests?

Yeah... I guess the tests is small enough that it doesn't require splitting.
It's basically testing the behavior of perf_stream_paranoid.

Are you think of something in particular for additional tests?

Thanks,

-
Lionel

>
> Thanks,
> Antonio
>
>> +}
>> +
>