| Message ID | 20190508100958.32637-10-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,01/16] i915/gem_exec_schedule: Semaphore priority fixups | expand |
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: if (flags & ALL) { for (i = 0; i < nchild; i++) { engines_copy = engines; nengines_copy = nengine; negines_child = 1; engines[0] = engines[i]; igt_fork(child, 1) { ... } if (in_parent) { engines = engines_copy; nengine = nengines_copy; } else { break; } } } ? Regards, Tvrtko > + nengine = 1; > + } > + > memset(&scratch, 0, sizeof(scratch)); > scratch.handle = gem_create(fd, 4096); > scratch.flags = EXEC_OBJECT_WRITE; > @@ -341,7 +355,7 @@ static void whisper(int fd, unsigned engine, unsigned flags) > igt_until_timeout(150) { > uint64_t offset; > > - if (!(flags & FORKED)) > + if (nchild == 1) > write_seqno(debugfs, pass); > > if (flags & HANG) > @@ -382,8 +396,8 @@ static void whisper(int fd, unsigned engine, unsigned flags) > > gem_write(fd, batches[1023].handle, loc, &pass, sizeof(pass)); > for (n = 1024; --n >= 1; ) { > + uint32_t handle[2] = {}; > int this_fd = fd; > - uint32_t handle[2]; > > execbuf.buffers_ptr = to_user_pointer(&batches[n-1]); > reloc_migrations += batches[n-1].offset != inter[n].presumed_offset; > @@ -550,7 +564,7 @@ igt_main > { "queues-sync", QUEUES | SYNC }, > { NULL } > }; > - int fd; > + int fd = -1; > > igt_fixture { > fd = drm_open_driver_master(DRIVER_INTEL); > @@ -561,9 +575,12 @@ igt_main > igt_fork_hang_detector(fd); > } > > - for (const struct mode *m = modes; m->name; m++) > + for (const struct mode *m = modes; m->name; m++) { > igt_subtest_f("%s", m->name) > whisper(fd, ALL_ENGINES, m->flags); > + igt_subtest_f("%s-all", m->name) > + whisper(fd, ALL_ENGINES, m->flags | ALL); > + } > > for (const struct intel_execution_engine *e = intel_execution_engines; > e->name; e++) { >
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). -Chris
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
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
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@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@intel.com> Regards, Tvrtko
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]; + nengine = 1; + } + memset(&scratch, 0, sizeof(scratch)); scratch.handle = gem_create(fd, 4096); scratch.flags = EXEC_OBJECT_WRITE; @@ -341,7 +355,7 @@ static void whisper(int fd, unsigned engine, unsigned flags) igt_until_timeout(150) { uint64_t offset; - if (!(flags & FORKED)) + if (nchild == 1) write_seqno(debugfs, pass); if (flags & HANG) @@ -382,8 +396,8 @@ static void whisper(int fd, unsigned engine, unsigned flags) gem_write(fd, batches[1023].handle, loc, &pass, sizeof(pass)); for (n = 1024; --n >= 1; ) { + uint32_t handle[2] = {}; int this_fd = fd; - uint32_t handle[2]; execbuf.buffers_ptr = to_user_pointer(&batches[n-1]); reloc_migrations += batches[n-1].offset != inter[n].presumed_offset; @@ -550,7 +564,7 @@ igt_main { "queues-sync", QUEUES | SYNC }, { NULL } }; - int fd; + int fd = -1; igt_fixture { fd = drm_open_driver_master(DRIVER_INTEL); @@ -561,9 +575,12 @@ igt_main igt_fork_hang_detector(fd); } - for (const struct mode *m = modes; m->name; m++) + for (const struct mode *m = modes; m->name; m++) { igt_subtest_f("%s", m->name) whisper(fd, ALL_ENGINES, m->flags); + igt_subtest_f("%s-all", m->name) + whisper(fd, ALL_ENGINES, m->flags | ALL); + } for (const struct intel_execution_engine *e = intel_execution_engines; e->name; e++) {
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(-)