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

Tvrtko Ursulin
2019-05-16T08:57:08Z

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@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.

Regards,

Tvrtko