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

Chris Wilson
2019-05-22T10:59:50Z

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