[igt-dev] [PATCH i-g-t 10/16] i915/gem_exec_whisper: Fork all-engine tests one-per-engine

[Tvrtko Ursulin]

On 22/05/2019 11:59, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-16 09:57:08)
>>
>> On 15/05/2019 20:35, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-05-14 13:57:26)
>>>>
>>>> On 08/05/2019 11:09, Chris Wilson wrote:
>>>>> Add a new mode for some more stress, submit the all-engines tests
>>>>> simultaneously, a stream per engine.
>>>>>
>>>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>>> ---
>>>>>     tests/i915/gem_exec_whisper.c | 27 ++++++++++++++++++++++-----
>>>>>     1 file changed, 22 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/tests/i915/gem_exec_whisper.c b/tests/i915/gem_exec_whisper.c
>>>>> index d3e0b0ba2..d5afc8119 100644
>>>>> --- a/tests/i915/gem_exec_whisper.c
>>>>> +++ b/tests/i915/gem_exec_whisper.c
>>>>> @@ -88,6 +88,7 @@ static void verify_reloc(int fd, uint32_t handle,
>>>>>     #define SYNC 0x40
>>>>>     #define PRIORITY 0x80
>>>>>     #define QUEUES 0x100
>>>>> +#define ALL 0x200
>>>>>     
>>>>>     struct hang {
>>>>>         struct drm_i915_gem_exec_object2 obj;
>>>>> @@ -199,6 +200,7 @@ static void whisper(int fd, unsigned engine, unsigned flags)
>>>>>         uint64_t old_offset;
>>>>>         int i, n, loc;
>>>>>         int debugfs;
>>>>> +     int nchild;
>>>>>     
>>>>>         if (flags & PRIORITY) {
>>>>>                 igt_require(gem_scheduler_enabled(fd));
>>>>> @@ -215,6 +217,7 @@ static void whisper(int fd, unsigned engine, unsigned flags)
>>>>>                                 engines[nengine++] = engine;
>>>>>                 }
>>>>>         } else {
>>>>> +             igt_assert(!(flags & ALL));
>>>>>                 igt_require(gem_has_ring(fd, engine));
>>>>>                 igt_require(gem_can_store_dword(fd, engine));
>>>>>                 engines[nengine++] = engine;
>>>>> @@ -233,11 +236,22 @@ static void whisper(int fd, unsigned engine, unsigned flags)
>>>>>         if (flags & HANG)
>>>>>                 init_hang(&hang);
>>>>>     
>>>>> +     nchild = 1;
>>>>> +     if (flags & FORKED)
>>>>> +             nchild *= sysconf(_SC_NPROCESSORS_ONLN);
>>>>> +     if (flags & ALL)
>>>>> +             nchild *= nengine;
>>>>> +
>>>>>         intel_detect_and_clear_missed_interrupts(fd);
>>>>>         gpu_power_read(&power, &sample[0]);
>>>>> -     igt_fork(child, flags & FORKED ? sysconf(_SC_NPROCESSORS_ONLN) : 1)  {
>>>>> +     igt_fork(child, nchild) {
>>>>>                 unsigned int pass;
>>>>>     
>>>>> +             if (flags & ALL) {
>>>>> +                     engines[0] = engines[child % nengine];
>>>>
>>>> Relying on PIDs being sequential feels fragile but suggesting pipes or
>>>> shared memory would be overkill. How about another loop:
>>>
>>> Where are you getting pid_t from? child is an integer [0, nchild).
>>
>> Add a core helper to get it?
>>
>> I am coming from an angle that I remember some time in the past there
>> was a security thing which randomized pid allocation. TBH I am not sure
>> if that still exists, but if it does then it would not be good for this
>> test. May be moot point to think such security hardening measures would
>> be active on a machine running IGT tests.. hm.. not sure. But it is
>> still a quite hidden assumption.
> 
> But we are not using pid_t here. It is just an array of child processes,
> with each child getting its own engine, using the child index as an
> index.

Oh right.. both is the same. Sorry, context-switching fail on my part..

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko